[libvirt] [PATCH] virfile: Take symlink into account in virFileIsSharedFixFUSE

[Erik Skultety]

On Thu, Oct 18, 2018 at 05:04:37PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1640465
>
> Weirdly enough, there can be symlinks in the path we are trying
> to fix. If it is the case our clever algorithm that finds matches
> against mount table won't work. Canonicalize path at the
> beginning then.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  cfg.mk              |  2 +-
>  src/util/virfile.c  | 17 ++++++++++++++---
>  tests/virfilemock.c | 33 ++++++++++++++++++++++++++++++++-
>  tests/virfiletest.c |  1 +
>  4 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/cfg.mk b/cfg.mk
> index 4790d0b7e7..d0183c02ff 100644
> --- a/cfg.mk
> +++ b/cfg.mk
> @@ -1229,7 +1229,7 @@ exclude_file_name_regexp--sc_prohibit_select = \
>  	^cfg\.mk$$
>
>  exclude_file_name_regexp--sc_prohibit_canonicalize_file_name = \
> -  ^cfg\.mk$$
> +  ^(cfg\.mk|tests/virfilemock\.c)$$
>
>  exclude_file_name_regexp--sc_prohibit_raw_allocation = \
>    ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|(vircgroup|nss)mock|commandhelper)\.c|tools/wireshark/src/packet-libvirt\.c)$$
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index e09992e41a..4747a26ad3 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -3473,9 +3473,19 @@ virFileIsSharedFixFUSE(const char *path,
>      char mntbuf[1024];
>      char *mntDir = NULL;
>      char *mntType = NULL;
> +    char *canonPath = NULL;
>      size_t maxMatching = 0;
>      int ret = -1;
>
> +    if (!(canonPath = virFileCanonicalizePath(path))) {
> +        virReportSystemError(errno,
> +                             _("unable to canonicalize %s"),
> +                             path);
> +        return -1;
> +    }
> +
> +    VIR_DEBUG("Path %s after canonicalization %s", path, canonPath);

s/canonicalization/canonicalization: /

or if you preferred, you could be brave and have a message like this:

("Path canonicalization: %s->%s", path, canonPath) which imho creates a better
contrast, but it's your call.

> +
>      if (!(f = setmntent(PROC_MOUNTS, "r"))) {
>          virReportSystemError(errno,
>                               _("Unable to open %s"),
> @@ -3487,7 +3497,7 @@ virFileIsSharedFixFUSE(const char *path,
>          const char *p;
>          size_t len = strlen(mb.mnt_dir);
>
> -        if (!(p = STRSKIP(path, mb.mnt_dir)))
> +        if (!(p = STRSKIP(canonPath, mb.mnt_dir)))
>              continue;
>
>          if (*(p - 1) != '/' && *p != '/' && *p != '\0')
> @@ -3504,13 +3514,14 @@ virFileIsSharedFixFUSE(const char *path,
>      }
>
>      if (STREQ_NULLABLE(mntType, "fuse.glusterfs")) {
> -        VIR_DEBUG("Found gluster FUSE mountpoint=%s for path=%s. "

It's debug, I think it's perfectly fine to leave the "path=" part in there,
just swap the corresponding source var for canonPath.

Reviewed-by: Erik Skultety <eskultet at redhat.com>