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

[John Ferlan]

On 04/02/2015 07:31 AM, Ján Tomko wrote:
> 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?
> 

True - the real bugger is virStorageBackendStablePath - since it's
shared code, my goal here was to minimize impact to the [i]SCSI paths.

In any case, I've pushed the 3 ACK'd patches and will see what I can
come up with for StablePath checking.

Tks -

John