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