8:34 a.m.
Few cleanups for problems I've noticed when I had to understand how the
shared FS detection is supposed to work.
Peter Krempa (7):
util: virFileIsSharedFSType: Pass bitmap of checked fs types as
unsigned
util: virFileIsSharedFSType: Annotate (some) shared filesystem names
util: virFileIsSharedFixFUSE: Refactor cleanup
util: virfile: Rewrite matching of FUSE-based shared filesystems
util: virfile: Don't use VIR_FILE_SHFS_GFS2 for glusterfs
util: virfile: Drop QB_MAGIC constant
util: virFileIsSharedFSType: Simplify shared fs type declarations
src/util/virfile.c | 139 +++++++++++++++++++++------------------------
src/util/virfile.h | 19 ++++---
2 files changed, 76 insertions(+), 82 deletions(-)
--
2.37.1
8:34 a.m.
New subject: [PATCH 1/7] util: virFileIsSharedFSType: Pass bitmap of checked fs types as unsigned
We populate the bits individualy so unsigned is the proper type.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virfile.c | 4 ++--
src/util/virfile.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index ce541b8946..1def05b6c4 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3394,7 +3394,7 @@ virFileIsSharedFixFUSE(const char *path,
int
virFileIsSharedFSType(const char *path,
- int fstypes)
+ unsigned int fstypes)
{
g_autofree char *dirpath = NULL;
char *p = NULL;
@@ -3601,7 +3601,7 @@ virFileFindHugeTLBFS(virHugeTLBFS **ret_fs,
#else /* defined __linux__ */
int virFileIsSharedFSType(const char *path G_GNUC_UNUSED,
- int fstypes G_GNUC_UNUSED)
+ unsigned int fstypes G_GNUC_UNUSED)
{
/* XXX implement me :-) */
return 0;
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 4af1ad9136..8d6df034f8 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -224,7 +224,7 @@ enum {
VIR_FILE_SHFS_ACFS = (1 << 9),
};
-int virFileIsSharedFSType(const char *path, int fstypes) ATTRIBUTE_NONNULL(1);
+int virFileIsSharedFSType(const char *path, unsigned int fstypes) ATTRIBUTE_NONNULL(1);
int virFileIsSharedFS(const char *path) ATTRIBUTE_NONNULL(1);
int virFileIsClusterFS(const char *path) ATTRIBUTE_NONNULL(1);
int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1);
--
2.37.1
9:02 a.m.
New subject: [PATCH 1/7] util: virFileIsSharedFSType: Pass bitmap of checked fs types as unsigned
On a Monday in 2022, Peter Krempa wrote:
We populate the bits individualy so unsigned is the proper type.
*individually
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virfile.c | 4 ++--
src/util/virfile.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
8:34 a.m.
New subject: [PATCH 2/7] util: virFileIsSharedFSType: Annotate (some) shared filesystem names
Expand some of the uncommon or unobvious filesystem names in a comment.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virfile.h | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 8d6df034f8..c4d11ea21a 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -213,15 +213,15 @@ bool virFileIsRegular(const char *file) ATTRIBUTE_NONNULL(1);
enum {
VIR_FILE_SHFS_NFS = (1 << 0),
- VIR_FILE_SHFS_GFS2 = (1 << 1),
- VIR_FILE_SHFS_OCFS = (1 << 2),
- VIR_FILE_SHFS_AFS = (1 << 3),
- VIR_FILE_SHFS_SMB = (1 << 4),
- VIR_FILE_SHFS_CIFS = (1 << 5),
+ VIR_FILE_SHFS_GFS2 = (1 << 1), /* Global File System 2 */
+ VIR_FILE_SHFS_OCFS = (1 << 2), /* Oracle Cluster FS (2) */
+ VIR_FILE_SHFS_AFS = (1 << 3), /* Andrew File System */
+ VIR_FILE_SHFS_SMB = (1 << 4), /* Server message block - windows shares */
+ VIR_FILE_SHFS_CIFS = (1 << 5), /* Common Internet File System - windows shares
*/
VIR_FILE_SHFS_CEPH = (1 << 6),
- VIR_FILE_SHFS_GPFS = (1 << 7),
- VIR_FILE_SHFS_QB = (1 << 8),
- VIR_FILE_SHFS_ACFS = (1 << 9),
+ VIR_FILE_SHFS_GPFS = (1 << 7), /* General Parallel File System/IBM Spectrum
Scale */
+ VIR_FILE_SHFS_QB = (1 << 8), /* Quobyte shared filesystem */
+ VIR_FILE_SHFS_ACFS = (1 << 9), /* Oracle ASM Cluster File System */
};
int virFileIsSharedFSType(const char *path, unsigned int fstypes) ATTRIBUTE_NONNULL(1);
--
2.37.1
8:34 a.m.
New subject: [PATCH 3/7] util: virFileIsSharedFixFUSE: Refactor cleanup
Automatically free memory of 'canonPath' so that the failure of
'setmntent' doesn't have to go to 'cleanup'. This allows us to remove
the cleanup section and the 'ret' variable as the rest of the function
can't fail.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virfile.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 1def05b6c4..a8443acf8d 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3333,24 +3333,19 @@ virFileIsSharedFixFUSE(const char *path,
char mntbuf[1024];
char *mntDir = NULL;
char *mntType = NULL;
- char *canonPath = NULL;
+ g_autofree char *canonPath = NULL;
size_t maxMatching = 0;
- int ret = -1;
if (!(canonPath = virFileCanonicalizePath(path))) {
- virReportSystemError(errno,
- _("unable to canonicalize %s"),
- path);
+ virReportSystemError(errno, _("unable to canonicalize %s"), path);
return -1;
}
VIR_DEBUG("Path canonicalization: %s->%s", path, canonPath);
if (!(f = setmntent(PROC_MOUNTS, "r"))) {
- virReportSystemError(errno,
- _("Unable to open %s"),
- PROC_MOUNTS);
- goto cleanup;
+ virReportSystemError(errno, _("Unable to open %s"), PROC_MOUNTS);
+ return -1;
}
while (getmntent_r(f, &mb, mntbuf, sizeof(mntbuf))) {
@@ -3372,6 +3367,8 @@ virFileIsSharedFixFUSE(const char *path,
}
}
+ endmntent(f);
+
if (STREQ_NULLABLE(mntType, "fuse.glusterfs")) {
VIR_DEBUG("Found gluster FUSE mountpoint=%s for path=%s. "
"Fixing shared FS type", mntDir, canonPath);
@@ -3382,13 +3379,9 @@ virFileIsSharedFixFUSE(const char *path,
*f_type = QB_MAGIC;
}
- ret = 0;
- cleanup:
- VIR_FREE(canonPath);
VIR_FREE(mntType);
VIR_FREE(mntDir);
- endmntent(f);
- return ret;
+ return 0;
}
--
2.37.1
8:34 a.m.
New subject: [PATCH 4/7] util: virfile: Rewrite matching of FUSE-based shared filesystems
'virFileIsSharedFixFUSE' was used to update the 'f_type' field for
certain shared filesystem types.
This patch renames it to 'virFileIsSharedFsFUSE' and makes it directly
return whether the FUSE filesystem is shared or not and simplifies
additions to the list of shared FUSE filesystems.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virfile.c | 54 ++++++++++++++++++++++++++++------------------
1 file changed, 33 insertions(+), 21 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index a8443acf8d..33a3ffe8f7 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3324,17 +3324,27 @@ virFileRemoveLastComponent(char *path)
# define PROC_MOUNTS "/proc/mounts"
+
+struct virFileSharedFsData {
+ const char *mnttype;
+ unsigned int fstype;
+};
+
+static const struct virFileSharedFsData virFileSharedFsFUSE[] = {
+ { .mnttype = "fuse.glusterfs", .fstype = VIR_FILE_SHFS_GFS2 },
+ { .mnttype = "fuse.quobyte", .fstype = VIR_FILE_SHFS_QB },
+};
+
static int
-virFileIsSharedFixFUSE(const char *path,
- long long *f_type)
+virFileIsSharedFsFUSE(const char *path,
+ unsigned int fstypes)
{
FILE *f = NULL;
struct mntent mb;
char mntbuf[1024];
- char *mntDir = NULL;
- char *mntType = NULL;
g_autofree char *canonPath = NULL;
size_t maxMatching = 0;
+ bool isShared = false;
if (!(canonPath = virFileCanonicalizePath(path))) {
virReportSystemError(errno, _("unable to canonicalize %s"), path);
@@ -3359,28 +3369,30 @@ virFileIsSharedFixFUSE(const char *path,
continue;
if (len > maxMatching) {
+ size_t i;
+ bool found = false;
+
+ for (i = 0; i < G_N_ELEMENTS(virFileSharedFsFUSE); i++) {
+ if (STREQ_NULLABLE(mb.mnt_type, virFileSharedFsFUSE[i].mnttype)
&&
+ (fstypes & virFileSharedFsFUSE[i].fstype) > 0) {
+ found = true;
+ break;
+ }
+ }
+
+ VIR_DEBUG("Updating shared='%d' for mountpoint '%s' type
'%s'",
+ found, p, mb.mnt_type);
+
+ isShared = found;
maxMatching = len;
- VIR_FREE(mntType);
- VIR_FREE(mntDir);
- mntDir = g_strdup(mb.mnt_dir);
- mntType = g_strdup(mb.mnt_type);
}
}
endmntent(f);
- if (STREQ_NULLABLE(mntType, "fuse.glusterfs")) {
- VIR_DEBUG("Found gluster FUSE mountpoint=%s for path=%s. "
- "Fixing shared FS type", mntDir, canonPath);
- *f_type = GFS2_MAGIC;
- } else if (STREQ_NULLABLE(mntType, "fuse.quobyte")) {
- VIR_DEBUG("Found Quobyte FUSE mountpoint=%s for path=%s. "
- "Fixing shared FS type", mntDir, canonPath);
- *f_type = QB_MAGIC;
- }
+ if (isShared)
+ return 1;
- VIR_FREE(mntType);
- VIR_FREE(mntDir);
return 0;
}
@@ -3432,8 +3444,8 @@ virFileIsSharedFSType(const char *path,
f_type = sb.f_type;
if (f_type == FUSE_SUPER_MAGIC) {
- VIR_DEBUG("Found FUSE mount for path=%s. Trying to fix it", path);
- virFileIsSharedFixFUSE(path, &f_type);
+ VIR_DEBUG("Found FUSE mount for path=%s", path);
+ return virFileIsSharedFsFUSE(path, fstypes);
}
VIR_DEBUG("Check if path %s with FS magic %lld is shared",
--
2.37.1
8:34 a.m.
New subject: [PATCH 5/7] util: virfile: Don't use VIR_FILE_SHFS_GFS2 for glusterfs
While the code works properly as no code path is specifically wanting to
check for glusterfs, we should properly declare glusterfs as a separate
from GFS2.
Fixes: 478da65fb46
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virfile.c | 8 +++++---
src/util/virfile.h | 1 +
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 33a3ffe8f7..f5a61abd9f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3331,7 +3331,7 @@ struct virFileSharedFsData {
};
static const struct virFileSharedFsData virFileSharedFsFUSE[] = {
- { .mnttype = "fuse.glusterfs", .fstype = VIR_FILE_SHFS_GFS2 },
+ { .mnttype = "fuse.glusterfs", .fstype = VIR_FILE_SHFS_GLUSTERFS },
{ .mnttype = "fuse.quobyte", .fstype = VIR_FILE_SHFS_QB },
};
@@ -3668,7 +3668,8 @@ int virFileIsSharedFS(const char *path)
VIR_FILE_SHFS_CEPH |
VIR_FILE_SHFS_GPFS|
VIR_FILE_SHFS_QB |
- VIR_FILE_SHFS_ACFS);
+ VIR_FILE_SHFS_ACFS |
+ VIR_FILE_SHFS_GLUSTERFS);
}
@@ -3681,7 +3682,8 @@ virFileIsClusterFS(const char *path)
return virFileIsSharedFSType(path,
VIR_FILE_SHFS_GFS2 |
VIR_FILE_SHFS_OCFS |
- VIR_FILE_SHFS_CEPH);
+ VIR_FILE_SHFS_CEPH |
+ VIR_FILE_SHFS_GLUSTERFS);
}
diff --git a/src/util/virfile.h b/src/util/virfile.h
index c4d11ea21a..f7a31d9f57 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -222,6 +222,7 @@ enum {
VIR_FILE_SHFS_GPFS = (1 << 7), /* General Parallel File System/IBM Spectrum
Scale */
VIR_FILE_SHFS_QB = (1 << 8), /* Quobyte shared filesystem */
VIR_FILE_SHFS_ACFS = (1 << 9), /* Oracle ASM Cluster File System */
+ VIR_FILE_SHFS_GLUSTERFS = (1 << 10), /* gluster's FUSE-based client */
};
int virFileIsSharedFSType(const char *path, unsigned int fstypes) ATTRIBUTE_NONNULL(1);
--
2.37.1
8:34 a.m.
New subject: [PATCH 6/7] util: virfile: Drop QB_MAGIC constant
The filesystem type magic constant was added for the 'quobyte' shared
filesystem in commit 451094bd153 but is present neither in the kernel
sources nor in coreutils which we've historically used as source of
information.
Since the code dealing with FUSE-based filesystems doesn't need the
constand we can remove it and the now-dead check for it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virfile.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index f5a61abd9f..6302364797 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3316,9 +3316,6 @@ virFileRemoveLastComponent(char *path)
# ifndef GPFS_SUPER_MAGIC
# define GPFS_SUPER_MAGIC 0x47504653
# endif
-# ifndef QB_MAGIC
-# define QB_MAGIC 0x51626d6e
-# endif
# define VIR_ACFS_MAGIC 0x61636673
@@ -3476,9 +3473,6 @@ virFileIsSharedFSType(const char *path,
if ((fstypes & VIR_FILE_SHFS_GPFS) &&
(f_type == GPFS_SUPER_MAGIC))
return 1;
- if ((fstypes & VIR_FILE_SHFS_QB) &&
- (f_type == QB_MAGIC))
- return 1;
if ((fstypes & VIR_FILE_SHFS_ACFS) &&
(f_type == VIR_ACFS_MAGIC))
return 1;
--
2.37.1
9:06 a.m.
New subject: [PATCH 6/7] util: virfile: Drop QB_MAGIC constant
On a Monday in 2022, Peter Krempa wrote:
The filesystem type magic constant was added for the 'quobyte'
shared
filesystem in commit 451094bd153 but is present neither in the kernel
sources nor in coreutils which we've historically used as source of
information.
I'm afraid the constant is some downstream magic number:
https://listman.redhat.com/archives/libvir-list/2019-July/187233.html
Since the code dealing with FUSE-based filesystems doesn't need
the
constand we can remove it and the now-dead check for it.
*constant
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virfile.c | 6 ------
1 file changed, 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
8:34 a.m.
New subject: [PATCH 7/7] util: virFileIsSharedFSType: Simplify shared fs type declarations
Use approach similar to virFileIsSharedFsFUSE to declaratively handle
the filesystem magic numbers mapping to libvirt's fstypes.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/virfile.c | 48 +++++++++++++++++++---------------------------
1 file changed, 20 insertions(+), 28 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 6302364797..ec40c04b1f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3324,6 +3324,7 @@ virFileRemoveLastComponent(char *path)
struct virFileSharedFsData {
const char *mnttype;
+ unsigned int magic;
unsigned int fstype;
};
@@ -3394,6 +3395,19 @@ virFileIsSharedFsFUSE(const char *path,
}
+static const struct virFileSharedFsData virFileSharedFs[] = {
+ { .fstype = VIR_FILE_SHFS_NFS, .magic = NFS_SUPER_MAGIC },
+ { .fstype = VIR_FILE_SHFS_GFS2, .magic = GFS2_MAGIC },
+ { .fstype = VIR_FILE_SHFS_OCFS, .magic = OCFS2_SUPER_MAGIC },
+ { .fstype = VIR_FILE_SHFS_AFS, .magic = AFS_FS_MAGIC },
+ { .fstype = VIR_FILE_SHFS_SMB, .magic = SMB_SUPER_MAGIC },
+ { .fstype = VIR_FILE_SHFS_CIFS, .magic = CIFS_SUPER_MAGIC },
+ { .fstype = VIR_FILE_SHFS_CEPH, .magic = CEPH_SUPER_MAGIC },
+ { .fstype = VIR_FILE_SHFS_GPFS, .magic = GPFS_SUPER_MAGIC },
+ { .fstype = VIR_FILE_SHFS_ACFS, .magic = VIR_ACFS_MAGIC },
+};
+
+
int
virFileIsSharedFSType(const char *path,
unsigned int fstypes)
@@ -3403,6 +3417,7 @@ virFileIsSharedFSType(const char *path,
struct statfs sb;
int statfs_ret;
long long f_type = 0;
+ size_t i;
dirpath = g_strdup(path);
@@ -3448,34 +3463,11 @@ virFileIsSharedFSType(const char *path,
VIR_DEBUG("Check if path %s with FS magic %lld is shared",
path, f_type);
- if ((fstypes & VIR_FILE_SHFS_NFS) &&
- (f_type == NFS_SUPER_MAGIC))
- return 1;
-
- if ((fstypes & VIR_FILE_SHFS_GFS2) &&
- (f_type == GFS2_MAGIC))
- return 1;
- if ((fstypes & VIR_FILE_SHFS_OCFS) &&
- (f_type == OCFS2_SUPER_MAGIC))
- return 1;
- if ((fstypes & VIR_FILE_SHFS_AFS) &&
- (f_type == AFS_FS_MAGIC))
- return 1;
- if ((fstypes & VIR_FILE_SHFS_SMB) &&
- (f_type == SMB_SUPER_MAGIC))
- return 1;
- if ((fstypes & VIR_FILE_SHFS_CIFS) &&
- (f_type == CIFS_SUPER_MAGIC))
- return 1;
- if ((fstypes & VIR_FILE_SHFS_CEPH) &&
- (f_type == CEPH_SUPER_MAGIC))
- return 1;
- if ((fstypes & VIR_FILE_SHFS_GPFS) &&
- (f_type == GPFS_SUPER_MAGIC))
- return 1;
- if ((fstypes & VIR_FILE_SHFS_ACFS) &&
- (f_type == VIR_ACFS_MAGIC))
- return 1;
+ for (i = 0; i < G_N_ELEMENTS(virFileSharedFs); i++) {
+ if (f_type == virFileSharedFs[i].magic &&
+ (fstypes & virFileSharedFs[i].fstype) > 0)
+ return 1;
+ }
return 0;
}
--
2.37.1
9:07 a.m.
On a Monday in 2022, Peter Krempa wrote:
Few cleanups for problems I've noticed when I had to understand
how the
shared FS detection is supposed to work.
Peter Krempa (7):
util: virFileIsSharedFSType: Pass bitmap of checked fs types as
unsigned
util: virFileIsSharedFSType: Annotate (some) shared filesystem names
util: virFileIsSharedFixFUSE: Refactor cleanup
util: virfile: Rewrite matching of FUSE-based shared filesystems
util: virfile: Don't use VIR_FILE_SHFS_GFS2 for glusterfs
util: virfile: Drop QB_MAGIC constant
util: virFileIsSharedFSType: Simplify shared fs type declarations
src/util/virfile.c | 139 +++++++++++++++++++++------------------------
src/util/virfile.h | 19 ++++---
2 files changed, 76 insertions(+), 82 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
838
days inactive
838
days old
10 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa