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