[libvirt] [PATCH v2 3/6] scsi: Adjust return value for virStorageBackendSCSINewLun

[Ján Tomko]

On Wed, Apr 01, 2015 at 01:29:08PM -0400, John Ferlan wrote:
> Add a return -2 to differentiate that the failure was a result of a non
> stable device path found or some other real error which would be messaged
> in some manner.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/storage/storage_backend_scsi.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
> index 58e7e6d..6def373 100644
> --- a/src/storage/storage_backend_scsi.c
> +++ b/src/storage/storage_backend_scsi.c
> @@ -146,6 +146,16 @@ virStorageBackendSCSISerial(const char *dev)
>  }
>  
>  
> +/*
> + * Attempt to create a new LUN
> + *
> + * Returns:
> + *
> + *  0  => Success
> + *  -1 => Failure due to some sort of OOM or other fatal issue found when
> + *        attempting to get/update information about a found volume
> + *  -2 => Failure to find a stable path
> + */
>  static int
>  virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>                              uint32_t host ATTRIBUTE_UNUSED,
> @@ -202,7 +212,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>          VIR_DEBUG("No stable path found for '%s' in '%s'",
>                    devpath, pool->def->target.path);
>  
> -        retval = -1;
> +        retval = -2;

This mixes two different errors:
* virStorageBackendStablePath short-circuited based on pool target path
  that's just as fatal as OOM and the attempt to find other volumes
  will also fail
* virStorageBackendStablePath fell through to ret_strdup -
  the directory exists, but a symlink to the device did not show up

  This means the device could appear later, so it makes sense to retry
  later.

And it doesn't handle the opendir failure in
virStorageBackendStablePath, which could also mean that the device will
appear later.

Given that this check here has to negate the !STREQ "/dev" checks in
virStorageBackendStablePath, maybe virStorageBackendStablePath should be
split up to two functions - one that returns an error when the device
path can't be stabilized and the other that would strdup the original path?

Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150402/9aba1367/attachment-0001.sig>