[libvirt] [PATCH 4/4] virfile: Rework virFileIsSharedFixFUSE

[Jiri Denemark]

On Tue, Oct 09, 2018 at 15:48:47 +0200, Michal Privoznik wrote:
> There are couple of things wrong with the current implementation.
> The first one is that in the first loop the code tries to build a
> list of fuse.glusterfs mount points. Well, since the strings are
> allocated in a temporary buffer and are not duplicated this
> results in wrong decision made later in the code.
> 
> The second problem is that the code does not take into account
> subtree mounts. For instance, if there's a fuse.gluster mounted
> at /some/path and another FS mounted at /some/path/subdir the
> code would not recognize this subdir mount.
> 
> Reported-by: Han Han <hhan at redhat.com>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/util/virfile.c            | 58 +++++++++++++----------------------
>  tests/virfiledata/mounts3.txt |  2 ++
>  tests/virfiletest.c           |  2 ++
>  3 files changed, 26 insertions(+), 36 deletions(-)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 666d703f99..19f504cc6d 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -3468,18 +3468,14 @@ static int
>  virFileIsSharedFixFUSE(const char *path,
>                         long *f_type)
>  {
> -    char *dirpath = NULL;
> -    const char **mounts = NULL;
> -    size_t nmounts = 0;
> -    char *p;
>      FILE *f = NULL;
>      struct mntent mb;
>      char mntbuf[1024];
> +    char *mntDir = NULL;
> +    char *mntType = NULL;
> +    size_t maxMatching = 0;
>      int ret = -1;
>  
> -    if (VIR_STRDUP(dirpath, path) < 0)
> -        return -1;
> -
>      if (!(f = setmntent(PROC_MOUNTS, "r"))) {
>          virReportSystemError(errno,
>                               _("Unable to open %s"),
> @@ -3488,43 +3484,33 @@ virFileIsSharedFixFUSE(const char *path,
>      }
>  
>      while (getmntent_r(f, &mb, mntbuf, sizeof(mntbuf))) {
> -        if (STRNEQ("fuse.glusterfs", mb.mnt_type))
> +        const char *p;
> +        size_t len = strlen(mb.mnt_dir);
> +
> +        if (!(p = STRSKIP(path, mb.mnt_dir)))
>              continue;

I think you wanted to do something clever with the p pointer here to
avoid false positives on, e.g., /mnt/ble when searching for /mnt/bleak
path. Otherwise you could have just use STRPREFIX().

The rest of the patch looks OK.

Jirka