Re: [libvirt] [PATCH 02/18] Fix error detection in virStorageBackendISCSIGetHostNumber
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;