[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 3/4] scsi: Adjust return value for virStorageBackendSCSINewLun




On 04/16/2015 09:14 AM, Ján Tomko wrote:
> On Tue, Apr 07, 2015 at 04:21:02PM -0400, John Ferlan wrote:
>> Use virStorageBackendPoolUseDevPath API to determine whether creation of
>> stable target path is possible for the volume. If not, then return failure.
>>
>> This will differentiate a failed virStorageBackendStablePath which won't
>> need to be fatal. Thus, we'll add a -2 return value to differentiate that
>> the failure was a result of either the inability to find the symlink for
>> the device or failure to open the target path directory
>>
>> Signed-off-by: John Ferlan <jferlan redhat com>
>> ---
>>  src/storage/storage_backend_scsi.c | 27 ++++++++++++++++++++++++---
>>  1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
>> index b96caec..d3c6470 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, not fatal, caller can try another
>> + */
>>  static int
>>  virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>>                              uint32_t host ATTRIBUTE_UNUSED,
>> @@ -158,6 +168,18 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool,
>>      char *devpath = NULL;
>>      int retval = -1;
>>  
>> +    /* Before we get too far - let's see if the pool is using target path
>> +     * starting with /dev. Attempts to find a stable path not based on a
>> +     * pool target starting with /dev will fail and do lots of unnecessary
>> +     * work - so we'll short circuit here.
> 
> This also rejects the non-stable path '/dev' that was accepted before.
> 

Not quite sure I see the issue - can you be more specific?  Am I missing
something obvious?

Previously if "pool->def->target.path" is "/dev" (or NULL or "/dev/" or
didn't start with "/dev"), then we'd return a strdup'd 'devpath' into
'vol->target.path'.

I see no way 'devpath' could be '/dev' or '/dev/' since it is formatted as :

     if (virAsprintf(&devpath, "/dev/%s", dev) < 0)
         goto cleanup;

where 'dev' is an argument to this function as found by processLU and
cannot be NULL from a call:

    if (getBlockDevice(host, bus, target, lun, &block_device) < 0) {
        VIR_DEBUG("Failed to find block device for this LUN");
        goto out;

which fills 'block_device' from either getNewStyleBlockDevice or
getOldStyleBlockDevice.

John


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]