[PATCH 0/2] Fix build on non-Linux

Jiri Denemark (2): util: Avoid statfs in virFileGetExistingParent util: Move virFileGetExistingParent out of ifdef __linux__ src/util/virfile.c | 56 ++++++++++++++++++++++----------------------- tests/virfilemock.c | 28 +++++++++++++++++++++++ 2 files changed, 56 insertions(+), 28 deletions(-) -- 2.49.0

From: Jiri Denemark <jdenemar@redhat.com> The code was separated from virFileIsSharedFSType which is Linux-only, but virFileGetExistingParent is also called from virFileIsSharedFSOverride which is OS independent. Thus we can't use statfs. Let's use virFileExists (access) instead, we were not interested in anything but success/failure from statfs anyway. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virfile.c | 3 +-- tests/virfilemock.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index 06e8e08ddc..d79a60baee 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3561,14 +3561,13 @@ static char * virFileGetExistingParent(const char *path) { g_autofree char *dirpath = g_strdup(path); - struct statfs sb; char *p = NULL; /* Try less and less of the path until we get to a directory we can stat. * Even if we don't have 'x' permission on any directory in the path on the * NFS server (assuming it's NFS), we will be able to stat the mount point. */ - while (statfs(dirpath, &sb) < 0 && p != dirpath) { + while (!virFileExists(dirpath) && p != dirpath) { if (!(p = strrchr(dirpath, '/'))) { virReportSystemError(EINVAL, _("Invalid relative path '%1$s'"), path); diff --git a/tests/virfilemock.c b/tests/virfilemock.c index 4f1b8aecd7..5a09cce855 100644 --- a/tests/virfilemock.c +++ b/tests/virfilemock.c @@ -21,6 +21,7 @@ #include <stdio.h> #include <mntent.h> #include <sys/vfs.h> +#include <unistd.h> #ifdef __linux__ # include <linux/magic.h> #endif @@ -32,6 +33,7 @@ static FILE *(*real_setmntent)(const char *filename, const char *type); static int (*real_statfs)(const char *path, struct statfs *buf); static char *(*real_realpath)(const char *path, char *resolved); +static int (*real_access)(const char *path, int mode); static void @@ -43,6 +45,7 @@ init_syms(void) VIR_MOCK_REAL_INIT(setmntent); VIR_MOCK_REAL_INIT(statfs); VIR_MOCK_REAL_INIT(realpath); + VIR_MOCK_REAL_INIT(access); } @@ -200,3 +203,28 @@ realpath(const char *path, char *resolved) return real_realpath(path, resolved); } + + +int +access(const char *path, int mode) +{ + const char *mtab = getenv("LIBVIRT_MTAB"); + + init_syms(); + + if (mtab && mode == F_OK) { + struct statfs buf; + + /* The real statfs works on any existing file on a filesystem, while + * our mocked version only works on the mount point. Thus we have to + * pretend no files on the filesystem exist to make sure + * virFileGetExistingParent returns the mount point which can later be + * checked by statfs. Instead of checking we were called for a mount + * point by walking through the mtab, we just call our mocked statfs + * that does it for us. + */ + return statfs_mock(mtab, path, &buf); + } + + return real_access(path, mode); +} -- 2.49.0

On Tue, Jun 03, 2025 at 11:07:43AM +0200, Jiri Denemark via Devel wrote:
From: Jiri Denemark <jdenemar@redhat.com>
The code was separated from virFileIsSharedFSType which is Linux-only, but virFileGetExistingParent is also called from virFileIsSharedFSOverride which is OS independent. Thus we can't use statfs. Let's use virFileExists (access) instead, we were not interested in anything but success/failure from statfs anyway.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virfile.c | 3 +-- tests/virfilemock.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c index 06e8e08ddc..d79a60baee 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3561,14 +3561,13 @@ static char * virFileGetExistingParent(const char *path) { g_autofree char *dirpath = g_strdup(path); - struct statfs sb; char *p = NULL;
/* Try less and less of the path until we get to a directory we can stat.
s/stat/access/ maybe?
* Even if we don't have 'x' permission on any directory in the path on the * NFS server (assuming it's NFS), we will be able to stat the mount point. */ - while (statfs(dirpath, &sb) < 0 && p != dirpath) { + while (!virFileExists(dirpath) && p != dirpath) { if (!(p = strrchr(dirpath, '/'))) { virReportSystemError(EINVAL, _("Invalid relative path '%1$s'"), path);

From: Jiri Denemark <jdenemar@redhat.com> The function is called by virFileIsSharedFSOverride which is not Linux specific and thus building on anything but Linux failes. Fixes: 94fb348d670f612c0b58901c9829b4eec81faa50 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virfile.c | 55 +++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index d79a60baee..1b3f37db7a 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3444,6 +3444,34 @@ virFileRemoveLastComponent(char *path) path[0] = '\0'; } + +static char * +virFileGetExistingParent(const char *path) +{ + g_autofree char *dirpath = g_strdup(path); + char *p = NULL; + + /* Try less and less of the path until we get to a directory we can stat. + * Even if we don't have 'x' permission on any directory in the path on the + * NFS server (assuming it's NFS), we will be able to stat the mount point. + */ + while (!virFileExists(dirpath) && p != dirpath) { + if (!(p = strrchr(dirpath, '/'))) { + virReportSystemError(EINVAL, + _("Invalid relative path '%1$s'"), path); + return NULL; + } + + if (p == dirpath) + *(p + 1) = '\0'; + else + *p = '\0'; + } + + return g_steal_pointer(&dirpath); +} + + #ifdef __linux__ # ifndef NFS_SUPER_MAGIC @@ -3557,33 +3585,6 @@ virFileIsSharedFsFUSE(const char *path, } -static char * -virFileGetExistingParent(const char *path) -{ - g_autofree char *dirpath = g_strdup(path); - char *p = NULL; - - /* Try less and less of the path until we get to a directory we can stat. - * Even if we don't have 'x' permission on any directory in the path on the - * NFS server (assuming it's NFS), we will be able to stat the mount point. - */ - while (!virFileExists(dirpath) && p != dirpath) { - if (!(p = strrchr(dirpath, '/'))) { - virReportSystemError(EINVAL, - _("Invalid relative path '%1$s'"), path); - return NULL; - } - - if (p == dirpath) - *(p + 1) = '\0'; - else - *p = '\0'; - } - - return g_steal_pointer(&dirpath); -} - - static const struct virFileSharedFsData virFileSharedFs[] = { { .fstype = VIR_FILE_SHFS_NFS, .magic = NFS_SUPER_MAGIC }, { .fstype = VIR_FILE_SHFS_GFS2, .magic = GFS2_MAGIC }, -- 2.49.0

On Tue, Jun 03, 2025 at 11:07:42AM +0200, Jiri Denemark via Devel wrote:
Jiri Denemark (2): util: Avoid statfs in virFileGetExistingParent util: Move virFileGetExistingParent out of ifdef __linux__
Let's hope this fixes everything O:-) Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
src/util/virfile.c | 56 ++++++++++++++++++++++----------------------- tests/virfilemock.c | 28 +++++++++++++++++++++++ 2 files changed, 56 insertions(+), 28 deletions(-)
-- 2.49.0
participants (2)
-
Jiri Denemark
-
Martin Kletzander