[libvirt] [PATCH 3/3] scsi: Adjust return status from getBlockDevice

[John Ferlan]

On 06/15/2015 07:24 AM, Michal Privoznik wrote:
> From: John Ferlan <jferlan at redhat.com>
> 
> 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 at redhat.com>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/storage/storage_backend_scsi.c | 48 +++++++++++++++++++++++++++-----------
>  1 file changed, 34 insertions(+), 14 deletions(-)
> 

ACK -

I had assumed only 1 & 2 were adjustments and felt odd acking my own
change, but now I see that there were more changes here as well...

John
> 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,
>