On 02/07/2017 09:16 AM, Erik Skultety wrote:
> So rather than comparing 2 paths (strings) as they are, which can very
> easily lead to unnecessary errors (e.g. in storage driver) that the paths
> are not the same when in fact they'd be e.g. just symlinks to the same
> location, we should put our best effort into resolving any symlinks and
> canonicalizing the path and only then compare the 2 paths.
>
> Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
> ---
> src/libvirt_private.syms | 1 +
> src/util/virfile.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> src/util/virfile.h | 2 ++
> 3 files changed, 48 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 8e994c7..06f3737 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1575,6 +1575,7 @@ virFileActivateDirOverride;
> virFileBindMountDevice;
> virFileBuildPath;
> virFileClose;
> +virFileComparePaths;
> virFileCopyACLs;
> virFileDeleteTree;
> virFileDirectFdFlag;
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index bf8099e..b261632 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -3737,3 +3737,48 @@ virFileCopyACLs(const char *src,
> virFileFreeACLs(&acl);
> return ret;
> }
> +
> +/*
> + * virFileComparePaths:
> + * @p1: source path 1
> + * @p2: source path 2
> + *
> + * Compares two paths for equality. To do so, it first canonicalizes both paths
> + * to resolve all symlinks and discard relative path components. If symlinks
> + * resolution or path canonicalization fails, plain string equality of @p1
> + * and @p2 is performed.
> + *
> + * Returns 0 if the paths are equal, 1 if they're not or -1 in case of an
> + * error.
> + */
> +int
> +virFileComparePaths(const char *p1, const char *p2)
I think this be "virFileCompareResolvedPaths" - change all the places...
Better representation than just ComparePaths
> +{
> + int ret = -1;
> + char *res1, *res2;
> +
> + res1 = res2 = NULL;
> +
> + /* Assume p1 and p2 are symlinks, so try to resolve and canonicalize them.
> + * Canonicalization fails for example on file systems names like 'proc' or
> + * 'sysfs', since they're no real paths so fallback to plain string
> + * comparison.
> + */
> + ignore_value(virFileResolveLink(p1, &res1));
> + if (!res1 && VIR_STRDUP(res1, p1) < 0)
> + goto cleanup;
> +
> + ignore_value(virFileResolveLink(p2, &res2));
> + if (!res2 && VIR_STRDUP(res2, p2) < 0)
> + goto cleanup;
> +
> + if (STREQ_NULLABLE(res1, res2))
> + ret = 0;
> + else
> + ret = 1;
WHy not return 1 on match, 0 on non match, and -1 on failure that way
all you have to do is "ret = STREQ_NULLABLE(res1, res2);"... If so
adjust the returns comments above too.
ACK w/ adjustments.
> +
> + cleanup:
> + VIR_FREE(res1);
> + VIR_FREE(res2);
> + return ret;
> +}
> diff --git a/src/util/virfile.h b/src/util/virfile.h
> index 0343acd..5822b29 100644
> --- a/src/util/virfile.h
> +++ b/src/util/virfile.h
> @@ -331,4 +331,6 @@ void virFileFreeACLs(void **acl);
>
> int virFileCopyACLs(const char *src,
> const char *dst);
> +
> +int virFileComparePaths(const char *p1, const char *p2);
> #endif /* __VIR_FILE_H */
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
