pvs in disk.sh

[suedi]

I came across this in disk.sh at line 552

   # is this a physical volume?
   PVS_RC=0
   PVS_DATA=""
   LVM2_PV_UUID=""

   # TODO: don't scan the whole /dev
   if [ -x /sbin/pvs ]; then 
      PVS="/sbin/pvs --nameprefixes --noheadings $VDEV_MOUNTPOINT/$VDEV_PATH"
      PVS_DATA="$($PVS -o pv_uuid 2>"$VDEV_MOUNTPOINT/null")"
      PVS_RC=$?
   else
      vdev_warn "Could not find pvs in /sbin/pvs.  LVM physical volume symlinks in $VDEV_MOUNTPOINT/disk/by-id will not be created."
   fi

   if [ $PVS_RC -eq 0 ]; then 

      eval "$PVS_DATA"
...

In the else statement with vdev_warn shouldn't PVS_RC be set to something other than

0 means that pvs suceeded but in the else statement to "if [ -x /sbin/pvs ]; then " there is no executable pvs on the system ergo PVS_RC should be set to fail and as a consequence the next "if [ $PVS_RC -eq 0 ]; then " would fail.

comment?

also is there a way to log messages with debug priority from bash files that respects loglevel in vdevd.conf

[jcnelson]

You're right. The branch beginning with if [ $PVS_RC -eq 0 ] should only execute if /sbin/pvs was able to generate usable data (i.e. $PVS_DATA should not be empty). The check should be if [ $PVS_RC -eq 0 ] && [ -n "$PVS_DATA" ].

The vdev_warn and vdev_error methods in subr.sh currently do not check vdevd.conf, but they could be extended to do so.

[suedi]

small things but best to handle when you come across them.

I think it would be a good idea to have some logging function that honors vdevd.conf loglevel settings. Perhaps not replacing vdev_warn and vdev_error

[jcnelson]

Hi @suedi,

The PVS probing should be fixed in f367dce7402e073cf78808f633fe17695892c8d8. I have code to fix vdev_warn and vdev_error to honor the loglevel in vdevd.conf, but I can't commit it yet since it interacts with code that addresses #53 (which is still being tested).

[suedi]

Good news man,

I will wait for your commit then

[suedi]

Seems the fix is in now.

closing...