Instead of initializing return value to zero (success) and overwriting it on every failure just before the control jumps onto 'out' label, let's initialize to an error value and set to zero only when we are sure about the success. Just follow the pattern we have in the rest of the code. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/storage/storage_backend_scsi.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index e6c8bb5..3c1bae6 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -255,44 +255,41 @@ getNewStyleBlockDevice(const char *lun_path, char *block_path = NULL; DIR *block_dir = NULL; struct dirent *block_dirent = NULL; - int retval = 0; + int retval = -1; int direrr; if (virAsprintf(&block_path, "%s/block", lun_path) < 0) - goto out; + goto cleanup; VIR_DEBUG("Looking for block device in '%s'", block_path); - block_dir = opendir(block_path); - if (block_dir == NULL) { + if (!(block_dir = opendir(block_path))) { virReportSystemError(errno, _("Failed to opendir sysfs path '%s'"), block_path); - retval = -1; - goto out; + goto cleanup; } while ((direrr = virDirRead(block_dir, &block_dirent, block_path)) > 0) { - if (STREQLEN(block_dirent->d_name, ".", 1)) continue; - if (VIR_STRDUP(*block_device, block_dirent->d_name) < 0) { - closedir(block_dir); - retval = -1; - goto out; - } + if (VIR_STRDUP(*block_device, block_dirent->d_name) < 0) + goto cleanup; VIR_DEBUG("Block device is '%s'", *block_device); break; } + if (direrr < 0) - retval = -1; + goto cleanup; - closedir(block_dir); + retval = 0; - out: + cleanup: + if (block_dir) + closedir(block_dir); VIR_FREE(block_path); return retval; } -- 2.3.6

From: John Ferlan <jferlan(a)redhat.com&gt; https://bugzilla.redhat.com/show_bug.cgi?id=1224233 Currently it's not possible to determine the difference between a fatal memory allocation or failure to open/read the directory error with a perhaps less fatal, I didn't find the "block" device in the directory (which may be a disk entry without a block device). In the case of the latter, we shouldn't cause failure to continue searching in the caller (virStorageBackendSCSIFindLUs), rather we should allow trying reading the next directory entry. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/storage/storage_backend_scsi.c | 48 +++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index ddfbade..b426145 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -324,6 +324,15 @@ getOldStyleBlockDevice(const char *lun_path ATTRIBUTE_UNUSED, } +/* + * Search a device entry for the "block" file + * + * Returns + * + * 0 => Found it + * -1 => Fatal error + * -2 => Didn't find in lun_path directory + */ static int getBlockDevice(uint32_t host, uint32_t bus, @@ -337,36 +346,47 @@ getBlockDevice(uint32_t host, int retval = -1; int direrr; + *block_device = NULL; + if (virAsprintf(&lun_path, "/sys/bus/scsi/devices/%u:%u:%u:%u", host, bus, target, lun) < 0) - goto out; + goto cleanup; - lun_dir = opendir(lun_path); - if (lun_dir == NULL) { + if (!(lun_dir = opendir(lun_path))) { virReportSystemError(errno, _("Failed to opendir sysfs path '%s'"), lun_path); - goto out; + goto cleanup; } while ((direrr = virDirRead(lun_dir, &lun_dirent, lun_path)) > 0) { if (STREQLEN(lun_dirent->d_name, "block", 5)) { if (strlen(lun_dirent->d_name) == 5) { - retval = getNewStyleBlockDevice(lun_path, - lun_dirent->d_name, - block_device); + if (getNewStyleBlockDevice(lun_path, + lun_dirent->d_name, + block_device) < 0) + goto cleanup; } else { - retval = getOldStyleBlockDevice(lun_path, - lun_dirent->d_name, - block_device); + if (getOldStyleBlockDevice(lun_path, + lun_dirent->d_name, + block_device) < 0) + goto cleanup; } break; } } + if (direrr < 0) + goto cleanup; + if (!*block_device) { + retval = -2; + goto cleanup; + } - closedir(lun_dir); + retval = 0; - out: + cleanup: + if (lun_dir) + closedir(lun_dir); VIR_FREE(lun_path); return retval; } @@ -412,9 +432,9 @@ processLU(virStoragePoolObjPtr pool, VIR_DEBUG("%u:%u:%u:%u is a Direct-Access LUN", host, bus, target, lun); - if (getBlockDevice(host, bus, target, lun, &block_device) < 0) { + if ((retval = getBlockDevice(host, bus, target, lun, &block_device)) < 0) { VIR_DEBUG("Failed to find block device for this LUN"); - return -1; + return retval; } retval = virStorageBackendSCSINewLun(pool, host, bus, target, lun, -- 2.3.6