[PATCH v2 5/6] scsi: Adjust return values from processLU

[John Ferlan <jferlan@xxxxxxxxxx>]

Adjust the processLU error returns to be a bit more logical. Currently,
the calling code cannot determine the difference between a non disk/lun
volume and a processed/found disk/lun. It can also not differentiate
between perhaps real/fatal error and one that won't necessarily stop
the code from finding other volumes.

After this patch virStorageBackendSCSIFindLUsInternal will stop processing
as soon as a "fatal" message occurs rather than continuting processing
for (well) no apparent reason.  It will also only set the *found value when
at least one of the processLU's was successful.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/storage/storage_backend_scsi.c | 44 +++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index e085da2..8087960 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -375,6 +375,16 @@ getBlockDevice(uint32_t host,
 }
 
 
+/*
+ * Process a Logical Unit entry from the scsi host device directory
+ *
+ * Returns:
+ *
+ *  0  => Found a valid entry
+ *  -1 => Some sort of fatal error
+ *  -2 => A "non-fatal" error such as a non disk/lun entry or an entry
+ *        without a block device found
+ */
 static int
 processLU(virStoragePoolObjPtr pool,
           uint32_t host,
@@ -393,7 +403,7 @@ processLU(virStoragePoolObjPtr pool,
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed to determine if %u:%u:%u:%u is a Direct-Access LUN"),
                        host, bus, target, lun);
-        goto out;
+        return -1;
     }
 
     /* We don't create volumes for devices other than disk and cdrom
@@ -401,32 +411,25 @@ processLU(virStoragePoolObjPtr pool,
      * isn't an error, either. */
     if (!(device_type == VIR_STORAGE_DEVICE_TYPE_DISK ||
           device_type == VIR_STORAGE_DEVICE_TYPE_ROM))
-    {
-        retval = 0;
-        goto out;
-    }
+        return -2;
 
     VIR_DEBUG("%u:%u:%u:%u is a Direct-Access LUN",
               host, bus, target, lun);
 
     if (getBlockDevice(host, bus, target, lun, &block_device) < 0) {
         VIR_DEBUG("Failed to find block device for this LUN");
-        goto out;
+        return -2;
     }
 
-    if (virStorageBackendSCSINewLun(pool,
-                                    host, bus, target, lun,
-                                    block_device) < 0) {
+    retval = virStorageBackendSCSINewLun(pool, host, bus, target, lun,
+                                         block_device);
+    if (retval < 0)
         VIR_DEBUG("Failed to create new storage volume for %u:%u:%u:%u",
                   host, bus, target, lun);
-        goto out;
-    }
-    retval = 0;
-
-    VIR_DEBUG("Created new storage volume for %u:%u:%u:%u successfully",
-              host, bus, target, lun);
+    else
+        VIR_DEBUG("Created new storage volume for %u:%u:%u:%u successfully",
+                  host, bus, target, lun);
 
- out:
     VIR_FREE(block_device);
     return retval;
 }
@@ -460,6 +463,8 @@ virStorageBackendSCSIFindLUsInternal(virStoragePoolObjPtr pool,
 
     *found = false;
     while ((retval = virDirRead(devicedir, &lun_dirent, device_path)) > 0) {
+        int rc;
+
         if (sscanf(lun_dirent->d_name, devicepattern,
                    &bus, &target, &lun) != 3) {
             continue;
@@ -467,7 +472,12 @@ virStorageBackendSCSIFindLUsInternal(virStoragePoolObjPtr pool,
 
         VIR_DEBUG("Found possible LU '%s'", lun_dirent->d_name);
 
-        if (processLU(pool, scanhost, bus, target, lun) == 0)
+        rc = processLU(pool, scanhost, bus, target, lun);
+        if (rc == -1) {
+            retval = -1;
+            break;
+        }
+        if (rc == 0)
             *found = true;
     }
 
-- 
2.1.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list