[libvirt] [PATCH v4 1/4] storage: Split out the valid path check

[John Ferlan]

On 04/20/2015 12:23 PM, Eric Blake wrote:
> On 04/19/2015 06:38 PM, John Ferlan wrote:
>> For virStorageBackendStablePath, in order to make decisions in other code
>> split out the checks regarding whether the pool's target is empty, using /dev,
>> using /dev/, or doesn't start with /dev
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/storage/storage_backend.c | 26 +++++++++++++-------------
>>  src/storage/storage_backend.h |  1 +
>>  2 files changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
>> index 0435983..b07e0d9 100644
>> --- a/src/storage/storage_backend.c
>> +++ b/src/storage/storage_backend.c
>> @@ -1674,6 +1674,17 @@ virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target,
>>      return 0;
>>  }
>>  
>> +bool
>> +virStorageBackendPoolPathIsStable(const char *path)
>> +{
>> +    if (path == NULL || STREQ(path, "/dev") || STREQ(path, "/dev/"))
>> +        return false;
>> +
>> +    if (!STRPREFIX(path, "/dev"))
>> +        return false;
> 
> I think you want "/dev/" here as the prefix to be required; otherwise,
> "/device" would match the prefix.  (This also means that someone using
> "//dev/..." would fail the check, but that's probably something we don't
> need to worry about).
> 
Hmm... Sure I see that... I can make that adjustment. I'll wait a bit
before pushing just so see if there's other feedback...

John
> 
>> -    /* Short circuit if pool has no target, or if its /dev */
>> -    if (pool->def->target.path == NULL ||
>> -        STREQ(pool->def->target.path, "/dev") ||
>> -        STREQ(pool->def->target.path, "/dev/"))
>> -        goto ret_strdup;
>> -
>> -    /* Skip whole thing for a pool which isn't in /dev
>> -     * so we don't mess filesystem/dir based pools
>> -     */
>> -    if (!STRPREFIX(pool->def->target.path, "/dev"))
>> -        goto ret_strdup;
> 
> Of course, the bug was pre-existing in the code you are just refactoring
> from, but now that we've spotted it, we might as well fix it.
>