From d8140487d0af54c4017e1d06a7ee5c676fad0a83 Mon Sep 17 00:00:00 2001 Message-Id: From: John Ferlan Date: Thu, 20 Nov 2014 10:42:52 -0500 Subject: [PATCH] storage: Fix issue finding LU's when block doesn't exist https://bugzilla.redhat.com/show_bug.cgi?id=1152382 Fix a problem in the getBlockDevice and processLU where retval initialized to zero causing some failures to erroneously continue through to the virStorageBackendSCSINewLun with an attempt to find a path for "/dev/(null)". This would fail approximately 10 seconds later with debug message: virStorageBackendSCSINewLun:203 : No stable path found for '/dev/(null)' in '/dev/disk/by-path' The root cause of the issue is for many /sys/bus/scsi/devices/ there is no "block*" device found for the vHBA's, where "" are the various paths created for the vHBA, such as "17:0:0:0", "17:0:1:0", "17:0:2:0", "17:0:3:0", etc. If the block device isn't found, then the directory should just be ignored rather than attempting to process it. The bug was that in getBlockDevice the assumption was "block" would exist and either getNewStyleBlockDevice or getOldStyleBlockDevice would fill in @block_device. However, if 'block*' doesn't exist, then the code returned NULL for block_device *and* a good (zero) retval value. This caused the processLU code to attempt the virStorageBackendSCSINewLun which failed "at some point in time" in the future. After this change - on test system the refresh-pool didn't have a noticable pause of about 20 seconds - it completed within a second since no longer was there an attempt/need to find "/dev/(null)". Additionally, the virStorageBackendSCSIFindLU's shouldn't be declaring found unless the processLU actually returns success. This will be important in the followup patch which relies on whether a LU was found. (cherry picked from commit 20870417322f7090d32e31cde864a2f916f55181) Conflicts: src/storage/storage_backend_scsi.c Downstream doesn't have mkletzan's curly brace check, so there was one place in the module where they were removed, but this code adds them back in because it's doing a VIR_DEBUG on getBlockDevice failure (see upstream e7a1da8aeb945e4d54693dd7a580caf94fec8a11) Signed-off-by: John Ferlan Signed-off-by: Jiri Denemark --- src/storage/storage_backend_scsi.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 3f61610..367216a 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -322,7 +322,7 @@ getBlockDevice(uint32_t host, char *lun_path = NULL; DIR *lun_dir = NULL; struct dirent *lun_dirent = NULL; - int retval = 0; + int retval = -1; int direrr; if (virAsprintf(&lun_path, "/sys/bus/scsi/devices/%u:%u:%u:%u", @@ -334,7 +334,6 @@ getBlockDevice(uint32_t host, virReportSystemError(errno, _("Failed to opendir sysfs path '%s'"), lun_path); - retval = -1; goto out; } @@ -369,7 +368,7 @@ processLU(virStoragePoolObjPtr pool, uint32_t lun) { char *type_path = NULL; - int retval = 0; + int retval = -1; int device_type; char *block_device = NULL; @@ -380,7 +379,6 @@ 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); - retval = -1; goto out; } @@ -398,6 +396,7 @@ processLU(virStoragePoolObjPtr pool, 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; } @@ -406,9 +405,9 @@ processLU(virStoragePoolObjPtr pool, block_device) < 0) { VIR_DEBUG("Failed to create new storage volume for %u:%u:%u:%u", host, bus, target, lun); - retval = -1; goto out; } + retval = 0; VIR_DEBUG("Created new storage volume for %u:%u:%u:%u successfully", host, bus, target, lun); @@ -453,10 +452,10 @@ virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, continue; } - found = true; - VIR_DEBUG("Found LU '%s'", lun_dirent->d_name); + VIR_DEBUG("Found possible LU '%s'", lun_dirent->d_name); - processLU(pool, scanhost, bus, target, lun); + if (processLU(pool, scanhost, bus, target, lun) == 0) + found = true; } if (!found) -- 2.1.3