[libvirt] [PATCH v4 2/4] scsi: Adjust return value for virStorageBackendSCSINewLun

[John Ferlan]

...

>> +    /* Check if the pool is using a stable target path. The call to
>> +     * virStorageBackendStablePath will fail if the pool target path
>> +     * isn't stable and just return the strdup'd 'devpath' anyway.
>> +     * This would be indistinguishable to failing to find the stable
>> +     * path to the device if the virDirRead loop to search the
>> +     * target pool path for our devpath had failed.
>> +     */
>> +    if (!virStorageBackendPoolPathIsStable(pool->def->target.path)) {
>> +        virReportError(VIR_ERR_INVALID_ARG,
>> +                       _("unable to use target path '%s' for dev '%s'"),
>> +                       NULLSTR(pool->def->target.path), dev);
>> +        goto cleanup;
>> +    }
> 
> /dev is a valid non-stable pool target path.
> 
>> +
>>      if (VIR_ALLOC(vol) < 0)
>>          goto cleanup;
>>  
>> @@ -187,13 +211,12 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>>                                                          true)) == NULL)
>>          goto cleanup;
>>  
>> -    if (STREQ(devpath, vol->target.path) &&
>> -        !(STREQ(pool->def->target.path, "/dev") ||
>> -          STREQ(pool->def->target.path, "/dev/"))) {
>> +    if (STREQ(devpath, vol->target.path)) {
>>  
> 
> Before, when virStorageBackendStablePath returned the same devpath because
> the pool path was "/dev", we continued with processing the volume.
> 
> After this patch, we won't even get here because of the first check.
> 
> Failure to stabilize the path should be expected here, if the pool
> target path is not stable.
> 

OK, but because virStorageBackendStablePath won't process the
pool->target.path of "/dev", we'll return the duplicated 'devpath' and
return -2 for every volume in the pool thus making it look empty.

What good is that?

Wouldn't it be better to tell the user that "/dev" is not a valid stable
path name... The path really needs to be more specific... I suppose one
could change virStorageBackendStablePath to accept "/dev" and do the
search, but that "could" take a while depending on the size of the
"/dev" file system.

John