[libvirt] [PATCH] virFileIsSharedFSType: Check for fuse.glusterfs too
https://bugzilla.redhat.com/show_bug.cgi?id=1632711 GlusterFS is typically safe when it comes to migration. It's a network FS after all. However, it can be mounted via FUSE driver they provide. If that is the case we fail to identify it and think migration is not safe and require VIR_MIGRATE_UNSAFE flag. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virfile.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 2 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index f8ae07fe4a..ccffa063a6 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3458,6 +3458,75 @@ int virFilePrintf(FILE *fp, const char *msg, ...) # ifndef HUGETLBFS_MAGIC # define HUGETLBFS_MAGIC 0x958458f6 # endif +# ifndef FUSE_SUPER_MAGIC +# define FUSE_SUPER_MAGIC 0x65735546 +# endif + +# define PROC_MOUNTS "/proc/mounts" + +static int +virFileIsShareFixFUSE(const char *path, + long *f_type) +{ + char *dirpath = NULL; + const char **mounts = NULL; + size_t nmounts = 0; + size_t i; + char *p; + FILE *f = NULL; + struct mntent mb; + char mntbuf[1024]; + bool found = false; + int ret = -1; + + if (VIR_STRDUP(dirpath, path) < 0) + return -1; + + if (!(f = setmntent(PROC_MOUNTS, "r"))) { + virReportSystemError(errno, + _("Unable to open %s"), + PROC_MOUNTS); + goto cleanup; + } + + while (getmntent_r(f, &mb, mntbuf, sizeof(mntbuf))) { + if (STRNEQ("fuse.glusterfs", mb.mnt_type)) + continue; + + if (VIR_APPEND_ELEMENT_COPY(mounts, nmounts, mb.mnt_dir) < 0) + goto cleanup; + } + + do { + if ((p = strrchr(dirpath, '/')) == NULL) { + virReportSystemError(EINVAL, + _("Invalid relative path '%s'"), path); + goto cleanup; + } + + if (p == dirpath) + *(p+1) = '\0'; + else + *p = '\0'; + + for (i = 0; i < nmounts; i++) { + if (STREQ(dirpath, mounts[i])) { + found = true; + VIR_DEBUG("Found gluster FUSE mountpoint=%s for path=%s. " + "Fixing shared FS type", mounts[i], path); + *f_type = GFS2_MAGIC; + } + } + } while (!found && p != dirpath); + + ret = 0; + cleanup: + endmntent(f); + VIR_FREE(mounts); + VIR_FREE(dirpath); + return ret; +} + int virFileIsSharedFSType(const char *path, @@ -3503,6 +3572,12 @@ virFileIsSharedFSType(const char *path, return -1; } + if (sb.f_type == FUSE_SUPER_MAGIC) { + VIR_DEBUG("Found FUSE mount for path=%s. Trying to fix it", path); + if (virFileIsShareFixFUSE(path, (long *) &sb.f_type) < 0) + return -1; + } + VIR_DEBUG("Check if path %s with FS magic %lld is shared", path, (long long int)sb.f_type); @@ -3594,8 +3669,6 @@ virFileGetDefaultHugepageSize(unsigned long long *size) return 0; } -# define PROC_MOUNTS "/proc/mounts" - int virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs, size_t *ret_nfs) -- 2.16.4
On Mon, Oct 01, 2018 at 09:56:01 +0200, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1632711
GlusterFS is typically safe when it comes to migration. It's a network FS after all. However, it can be mounted via FUSE driver they provide. If that is the case we fail to identify it and think migration is not safe and require VIR_MIGRATE_UNSAFE flag.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virfile.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 2 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c index f8ae07fe4a..ccffa063a6 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3458,6 +3458,75 @@ int virFilePrintf(FILE *fp, const char *msg, ...) # ifndef HUGETLBFS_MAGIC # define HUGETLBFS_MAGIC 0x958458f6 # endif +# ifndef FUSE_SUPER_MAGIC +# define FUSE_SUPER_MAGIC 0x65735546 +# endif + +# define PROC_MOUNTS "/proc/mounts" + +static int +virFileIsShareFixFUSE(const char *path,
s/Share/Shared/
+ long *f_type) +{ + char *dirpath = NULL; + const char **mounts = NULL; + size_t nmounts = 0; + size_t i; + char *p; + FILE *f = NULL; + struct mntent mb; + char mntbuf[1024]; + bool found = false; + int ret = -1; + + if (VIR_STRDUP(dirpath, path) < 0) + return -1; + + if (!(f = setmntent(PROC_MOUNTS, "r"))) { + virReportSystemError(errno, + _("Unable to open %s"), + PROC_MOUNTS); + goto cleanup; + } + + while (getmntent_r(f, &mb, mntbuf, sizeof(mntbuf))) { + if (STRNEQ("fuse.glusterfs", mb.mnt_type)) + continue; + + if (VIR_APPEND_ELEMENT_COPY(mounts, nmounts, mb.mnt_dir) < 0) + goto cleanup; + } + + do { + if ((p = strrchr(dirpath, '/')) == NULL) { + virReportSystemError(EINVAL, + _("Invalid relative path '%s'"), path); + goto cleanup; + } + + if (p == dirpath) + *(p+1) = '\0'; + else + *p = '\0'; + + for (i = 0; i < nmounts; i++) { + if (STREQ(dirpath, mounts[i])) { + found = true; + VIR_DEBUG("Found gluster FUSE mountpoint=%s for path=%s. " + "Fixing shared FS type", mounts[i], path); + *f_type = GFS2_MAGIC; + } + }
You could have used a string list and virStringListHasString instead of this loop and get rid of "found" and just break the loop. Even in the current form you should at least break the for loop if dirpath is found. On the other hand, the code would need to be changed if we ever want to extend it for other fuse-mounted file systems. So why not modifying it a bit right away even if we'll still be checking just fuse.glusterfs? It could save us some refactoring later...
+ } while (!found && p != dirpath); + + ret = 0; + cleanup: + endmntent(f); + VIR_FREE(mounts); + VIR_FREE(dirpath); + return ret; +} +
int virFileIsSharedFSType(const char *path, @@ -3503,6 +3572,12 @@ virFileIsSharedFSType(const char *path, return -1; }
+ if (sb.f_type == FUSE_SUPER_MAGIC) { + VIR_DEBUG("Found FUSE mount for path=%s. Trying to fix it", path); + if (virFileIsShareFixFUSE(path, (long *) &sb.f_type) < 0) + return -1;
I don't think any failure while parsing /proc/mounts should be fatal here. If it fails, we can just keep the FUSE fs type. Jirka
participants (2)
-
Jiri Denemark -
Michal Privoznik