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

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



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 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).


> -    /* 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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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