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;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list