On 06/21/2016 12:05 PM, Ján Tomko wrote:
In the unlikely case the iSCSI session path exists, but does not contain an entry starting with "target", we would silently use an initialized value.
Rewrite the function to correctly report errors. --- src/storage/storage_backend_iscsi.c | 38 +++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index bccfba3..876c14c 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -90,7 +90,7 @@ static int virStorageBackendISCSIGetHostNumber(const char *sysfs_path, uint32_t *host) { - int retval = 0; + int ret = -1; DIR *sysdir = NULL; struct dirent *dirent = NULL; int direrr; @@ -104,26 +104,33 @@ virStorageBackendISCSIGetHostNumber(const char *sysfs_path, if (sysdir == NULL) { virReportSystemError(errno, _("Failed to opendir path '%s'"), sysfs_path); - retval = -1; - goto out; + goto cleanup; }
while ((direrr = virDirRead(sysdir, &dirent, sysfs_path)) > 0) { if (STREQLEN(dirent->d_name, "target", strlen("target"))) {
While we're here and changing, could use STRPREFIX
- if (sscanf(dirent->d_name, - "target%u:", host) != 1) { - VIR_DEBUG("Failed to parse target '%s'", dirent->d_name); - retval = -1; - break; + if (sscanf(dirent->d_name, "target%u:", host) == 1) { + ret = 0; + goto cleanup; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse target '%s'"), dirent->d_name); + goto cleanup; } } } - if (direrr < 0) - retval = -1;
+ if (direrr == 0) {
Should this be <= ? If virDirRead() returns -1, then we don't return with an error... If virDirRead() returns 0, then we've gone through the entire directory without finding an entry that starts with target - seems that would be a system configuration error and I assume errno would be ENOENT ACK with obvious adjustments - John
+ virReportSystemError(errno, + _("Failed to get host number for iSCSI session " + "with path '%s'"), + sysfs_path); + goto cleanup; + } + + cleanup: closedir(sysdir); - out: - return retval; + return ret; }
static int @@ -138,13 +145,8 @@ virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, "/sys/class/iscsi_session/session%s/device", session) < 0) goto cleanup;
- if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0) { - virReportSystemError(errno, - _("Failed to get host number for iSCSI session " - "with path '%s'"), - sysfs_path); + if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0) goto cleanup; - }
if (virStorageBackendSCSIFindLUs(pool, host) < 0) goto cleanup;