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

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(a)redhat.com&gt; > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > 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.