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

[Michal Privoznik]

On 10/10/2018 01:08 PM, Jiri Denemark wrote:
> 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.


Oh right. I wanted to do this:

diff --git i/src/util/virfile.c w/src/util/virfile.c
index 19f504cc6d..03e14c757f 100644
--- i/src/util/virfile.c
+++ w/src/util/virfile.c
@@ -3490,6 +3490,9 @@ virFileIsSharedFixFUSE(const char *path,
         if (!(p = STRSKIP(path, mb.mnt_dir)))
             continue;

+        if (*p != '/')
+            continue;
+
         if (len > maxMatching) {
             len = maxMatching;
             VIR_FREE(mntType);


Michal