[PATCH 00/17] lxc_fuse: Implement support for FUSE3
Unfortunately, we can't just replace FUSE with FUSE3 because Ubuntu1804 doesn't have fuse3 package, but in a few weeks time, when the support for old Ubuntu is dropped, we can drop old FUSE support too. Michal Prívozník (17): lxc_fuse: Hide struct virLXCFuse lxc_fuse: Move virLXCMeminfo struct into lxc_cgroup.h lxc_fuse.h: Don't include lxc_conf.h lxc_fuse: Move #include <fuse.h> lxc_fuse: Drop some G_GNUC_UNUSED attributes lxc_fuse.c: Modernize function declarations lxc_fuse: Prefer O_ACCMODE instead of & 3 lxcSetupFuse: Cleanup error paths lxcProcReadMeminfo: Rename @fd to @fp lxc_fuse: Use automatic file closing lxcProcReadMeminfo: Drop needless label lxcProcReadMeminfo: Drop @new_meminfo variable lxcProcReadMeminfo: Fix case when @offset != 0 lxc_fuse: Prefer fuse_file_info::direct_io over mount option lxc_fuse: Tell FUSE that /proc/meminfo is nonseekable lxc_fuse: Implement support for FUSE3 meson: Detect newer fuse meson.build | 12 +- src/lxc/lxc_cgroup.h | 13 +++ src/lxc/lxc_fuse.c | 258 ++++++++++++++++++++++++++++++------------- src/lxc/lxc_fuse.h | 28 +---- 4 files changed, 207 insertions(+), 104 deletions(-) -- 2.34.1
This structure is not used outside of lxc_fuse.c. There is no need to define it in the header file. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 9 +++++++++ src/lxc/lxc_fuse.h | 8 -------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index e75a533c70..7b95629955 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -37,6 +37,15 @@ #if WITH_FUSE +struct virLXCFuse { + virDomainDef *def; + virThread thread; + char *mountpoint; + struct fuse *fuse; + struct fuse_chan *ch; + virMutex lock; +}; + static const char *fuse_meminfo_path = "/meminfo"; static int lxcProcGetattr(const char *path, struct stat *stbuf) diff --git a/src/lxc/lxc_fuse.h b/src/lxc/lxc_fuse.h index 54a0a74155..34a59667d4 100644 --- a/src/lxc/lxc_fuse.h +++ b/src/lxc/lxc_fuse.h @@ -41,14 +41,6 @@ struct virLXCMeminfo { unsigned long long swapusage; }; -struct virLXCFuse { - virDomainDef *def; - virThread thread; - char *mountpoint; - struct fuse *fuse; - struct fuse_chan *ch; - virMutex lock; -}; typedef struct virLXCFuse virLXCFuse; int lxcSetupFuse(struct virLXCFuse **f, virDomainDef *def); -- 2.34.1
The function that fills virLXCMeminfo struct (virLXCCgroupGetMeminfo()) lives in lxc_cgroup.h. Move the struct there too. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_cgroup.h | 13 +++++++++++++ src/lxc/lxc_fuse.h | 13 ------------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/lxc/lxc_cgroup.h b/src/lxc/lxc_cgroup.h index 2b323371f9..bebdc3ff96 100644 --- a/src/lxc/lxc_cgroup.h +++ b/src/lxc/lxc_cgroup.h @@ -35,6 +35,19 @@ int virLXCCgroupSetup(virDomainDef *def, virCgroup *cgroup, virBitmap *nodemask); +struct virLXCMeminfo { + unsigned long long memtotal; + unsigned long long memusage; + unsigned long long cached; + unsigned long long active_anon; + unsigned long long inactive_anon; + unsigned long long active_file; + unsigned long long inactive_file; + unsigned long long unevictable; + unsigned long long swaptotal; + unsigned long long swapusage; +}; + int virLXCCgroupGetMeminfo(struct virLXCMeminfo *meminfo); int diff --git a/src/lxc/lxc_fuse.h b/src/lxc/lxc_fuse.h index 34a59667d4..7052391a7b 100644 --- a/src/lxc/lxc_fuse.h +++ b/src/lxc/lxc_fuse.h @@ -28,19 +28,6 @@ #include "lxc_conf.h" -struct virLXCMeminfo { - unsigned long long memtotal; - unsigned long long memusage; - unsigned long long cached; - unsigned long long active_anon; - unsigned long long inactive_anon; - unsigned long long active_file; - unsigned long long inactive_file; - unsigned long long unevictable; - unsigned long long swaptotal; - unsigned long long swapusage; -}; - typedef struct virLXCFuse virLXCFuse; int lxcSetupFuse(struct virLXCFuse **f, virDomainDef *def); -- 2.34.1
Nothing in the lxc_fuse.h header file warrants inclusion of lxc_conf.h. If anything, virconftypes.h must be included because of virDomainDef required by lxcSetupFuse(). It's actually lxc_fuse.c that requires some macros from lxc_fuse.h (e.g. LXC_STATE_DIR). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 1 + src/lxc/lxc_fuse.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 7b95629955..0550ff5ab4 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -27,6 +27,7 @@ #include "lxc_fuse.h" #include "lxc_cgroup.h" +#include "lxc_conf.h" #include "virerror.h" #include "virfile.h" #include "virbuffer.h" diff --git a/src/lxc/lxc_fuse.h b/src/lxc/lxc_fuse.h index 7052391a7b..195e1e431a 100644 --- a/src/lxc/lxc_fuse.h +++ b/src/lxc/lxc_fuse.h @@ -26,7 +26,7 @@ # include <fuse.h> #endif -#include "lxc_conf.h" +#include "virconftypes.h" typedef struct virLXCFuse virLXCFuse; -- 2.34.1
There is no need to include the fuse.h from the header file. Move the include into the lxc_fuse.c then. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 5 +++++ src/lxc/lxc_fuse.h | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 0550ff5ab4..3732f2e245 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -25,6 +25,11 @@ #include <mntent.h> #include <unistd.h> +#if WITH_FUSE +# define FUSE_USE_VERSION 26 +# include <fuse.h> +#endif + #include "lxc_fuse.h" #include "lxc_cgroup.h" #include "lxc_conf.h" diff --git a/src/lxc/lxc_fuse.h b/src/lxc/lxc_fuse.h index 195e1e431a..4065bff7ce 100644 --- a/src/lxc/lxc_fuse.h +++ b/src/lxc/lxc_fuse.h @@ -20,11 +20,6 @@ #pragma once -#define FUSE_USE_VERSION 26 - -#if WITH_FUSE -# include <fuse.h> -#endif #include "virconftypes.h" -- 2.34.1
There are few arguments that are marked as G_GNUC_UNUSED even though they are clearly used within their respective functions. Drop the annotation in such cases. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 3732f2e245..6c61cbdf02 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -103,8 +103,8 @@ static int lxcProcReaddir(const char *path, void *buf, return 0; } -static int lxcProcOpen(const char *path G_GNUC_UNUSED, - struct fuse_file_info *fi G_GNUC_UNUSED) +static int lxcProcOpen(const char *path, + struct fuse_file_info *fi) { if (STRNEQ(path, fuse_meminfo_path)) return -ENOENT; @@ -242,10 +242,10 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDef *def, return res; } -static int lxcProcRead(const char *path G_GNUC_UNUSED, - char *buf G_GNUC_UNUSED, - size_t size G_GNUC_UNUSED, - off_t offset G_GNUC_UNUSED, +static int lxcProcRead(const char *path, + char *buf, + size_t size, + off_t offset, struct fuse_file_info *fi G_GNUC_UNUSED) { int res = -ENOENT; -- 2.34.1
Our style of writing function declarations has changed since the time the file was introduced. Fix the whole file. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 71 ++++++++++++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 24 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 6c61cbdf02..b6b653e8cd 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -54,7 +54,9 @@ struct virLXCFuse { static const char *fuse_meminfo_path = "/meminfo"; -static int lxcProcGetattr(const char *path, struct stat *stbuf) +static int +lxcProcGetattr(const char *path, + struct stat *stbuf) { g_autofree char *mempath = NULL; struct stat sb; @@ -88,10 +90,12 @@ static int lxcProcGetattr(const char *path, struct stat *stbuf) return 0; } -static int lxcProcReaddir(const char *path, void *buf, - fuse_fill_dir_t filler, - off_t offset G_GNUC_UNUSED, - struct fuse_file_info *fi G_GNUC_UNUSED) +static int +lxcProcReaddir(const char *path, + void *buf, + fuse_fill_dir_t filler, + off_t offset G_GNUC_UNUSED, + struct fuse_file_info *fi G_GNUC_UNUSED) { if (STRNEQ(path, "/")) return -ENOENT; @@ -103,8 +107,9 @@ static int lxcProcReaddir(const char *path, void *buf, return 0; } -static int lxcProcOpen(const char *path, - struct fuse_file_info *fi) +static int +lxcProcOpen(const char *path, + struct fuse_file_info *fi) { if (STRNEQ(path, fuse_meminfo_path)) return -ENOENT; @@ -115,7 +120,11 @@ static int lxcProcOpen(const char *path, return 0; } -static int lxcProcHostRead(char *path, char *buf, size_t size, off_t offset) +static int +lxcProcHostRead(char *path, + char *buf, + size_t size, + off_t offset) { int fd; int res; @@ -131,8 +140,12 @@ static int lxcProcHostRead(char *path, char *buf, size_t size, off_t offset) return res; } -static int lxcProcReadMeminfo(char *hostpath, virDomainDef *def, - char *buf, size_t size, off_t offset) +static int +lxcProcReadMeminfo(char *hostpath, + virDomainDef *def, + char *buf, + size_t size, + off_t offset) { int res; FILE *fd = NULL; @@ -242,11 +255,12 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDef *def, return res; } -static int lxcProcRead(const char *path, - char *buf, - size_t size, - off_t offset, - struct fuse_file_info *fi G_GNUC_UNUSED) +static int +lxcProcRead(const char *path, + char *buf, + size_t size, + off_t offset, + struct fuse_file_info *fi G_GNUC_UNUSED) { int res = -ENOENT; g_autofree char *hostpath = NULL; @@ -273,7 +287,8 @@ static struct fuse_operations lxcProcOper = { .read = lxcProcRead, }; -static void lxcFuseDestroy(struct virLXCFuse *fuse) +static void +lxcFuseDestroy(struct virLXCFuse *fuse) { VIR_LOCK_GUARD lock = virLockGuardLock(&fuse->lock); @@ -281,7 +296,8 @@ static void lxcFuseDestroy(struct virLXCFuse *fuse) g_clear_pointer(&fuse->fuse, fuse_destroy); } -static void lxcFuseRun(void *opaque) +static void +lxcFuseRun(void *opaque) { struct virLXCFuse *fuse = opaque; @@ -292,7 +308,9 @@ static void lxcFuseRun(void *opaque) lxcFuseDestroy(fuse); } -int lxcSetupFuse(struct virLXCFuse **f, virDomainDef *def) +int +lxcSetupFuse(struct virLXCFuse **f, + virDomainDef *def) { int ret = -1; struct fuse_args args = FUSE_ARGS_INIT(0, NULL); @@ -342,7 +360,8 @@ int lxcSetupFuse(struct virLXCFuse **f, virDomainDef *def) goto cleanup; } -int lxcStartFuse(struct virLXCFuse *fuse) +int +lxcStartFuse(struct virLXCFuse *fuse) { if (virThreadCreateFull(&fuse->thread, false, lxcFuseRun, "lxc-fuse", false, (void *)fuse) < 0) { @@ -353,7 +372,8 @@ int lxcStartFuse(struct virLXCFuse *fuse) return 0; } -void lxcFreeFuse(struct virLXCFuse **f) +void +lxcFreeFuse(struct virLXCFuse **f) { struct virLXCFuse *fuse = *f; /* lxcFuseRun thread create success */ @@ -370,18 +390,21 @@ void lxcFreeFuse(struct virLXCFuse **f) } } #else -int lxcSetupFuse(struct virLXCFuse **f G_GNUC_UNUSED, - virDomainDef *def G_GNUC_UNUSED) +int +lxcSetupFuse(struct virLXCFuse **f G_GNUC_UNUSED, + virDomainDef *def G_GNUC_UNUSED) { return 0; } -int lxcStartFuse(struct virLXCFuse *f G_GNUC_UNUSED) +int +lxcStartFuse(struct virLXCFuse *f G_GNUC_UNUSED) { return 0; } -void lxcFreeFuse(struct virLXCFuse **f G_GNUC_UNUSED) +void +lxcFreeFuse(struct virLXCFuse **f G_GNUC_UNUSED) { } #endif -- 2.34.1
In lxcProcOpen() we want to check whether the /proc/memfile is being opened only for read. For that we check the fi->flags which correspond to flags open() call. Instead of explicitly masking the last two bits use O_ACCMODE constant, which is deemed to be more portable. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index b6b653e8cd..683d01525f 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -114,7 +114,7 @@ lxcProcOpen(const char *path, if (STRNEQ(path, fuse_meminfo_path)) return -ENOENT; - if ((fi->flags & 3) != O_RDONLY) + if ((fi->flags & O_ACCMODE) != O_RDONLY) return -EACCES; return 0; -- 2.34.1
In the lxcSetupFuse() function there are multiple cleanup labels, but with a bit of rewrite they can be joined into one 'error' label. And while at it, set the @f argument only in the successful path (currently is set in error case too). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 683d01525f..274702736f 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -314,19 +314,19 @@ lxcSetupFuse(struct virLXCFuse **f, { int ret = -1; struct fuse_args args = FUSE_ARGS_INIT(0, NULL); - struct virLXCFuse *fuse = g_new0(virLXCFuse, 1); + g_autofree struct virLXCFuse *fuse = g_new0(virLXCFuse, 1); fuse->def = def; if (virMutexInit(&fuse->lock) < 0) - goto cleanup2; + return -1; fuse->mountpoint = g_strdup_printf("%s/%s.fuse/", LXC_STATE_DIR, def->name); if (g_mkdir_with_parents(fuse->mountpoint, 0777) < 0) { virReportSystemError(errno, _("Cannot create %s"), fuse->mountpoint); - goto cleanup1; + goto error; } /* process name is libvirt_lxc */ @@ -334,29 +334,29 @@ lxcSetupFuse(struct virLXCFuse **f, fuse_opt_add_arg(&args, "-odirect_io") == -1 || fuse_opt_add_arg(&args, "-oallow_other") == -1 || fuse_opt_add_arg(&args, "-ofsname=libvirt") == -1) - goto cleanup1; + goto error; fuse->ch = fuse_mount(fuse->mountpoint, &args); if (fuse->ch == NULL) - goto cleanup1; + goto error; fuse->fuse = fuse_new(fuse->ch, &args, &lxcProcOper, sizeof(lxcProcOper), fuse->def); if (fuse->fuse == NULL) { - fuse_unmount(fuse->mountpoint, fuse->ch); - goto cleanup1; + goto error; } + *f = g_steal_pointer(&fuse); ret = 0; cleanup: fuse_opt_free_args(&args); - *f = fuse; return ret; - cleanup1: + + error: + if (fuse->ch) + fuse_unmount(fuse->mountpoint, fuse->ch); g_free(fuse->mountpoint); virMutexDestroy(&fuse->lock); - cleanup2: - g_free(fuse); goto cleanup; } -- 2.34.1
In lxcProcReadMeminfo() there's a variable named @fd which would suggest it's type of int, but in fact it's type of FILE *. Rename it to @fp to avoid confusion. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 274702736f..9eff188d5f 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -148,7 +148,7 @@ lxcProcReadMeminfo(char *hostpath, off_t offset) { int res; - FILE *fd = NULL; + FILE *fp = NULL; g_autofree char *line = NULL; size_t n; struct virLXCMeminfo meminfo; @@ -160,21 +160,21 @@ lxcProcReadMeminfo(char *hostpath, return -errno; } - fd = fopen(hostpath, "r"); - if (fd == NULL) { + fp = fopen(hostpath, "r"); + if (fp == NULL) { virReportSystemError(errno, _("Cannot open %s"), hostpath); res = -errno; goto cleanup; } - if (fseek(fd, offset, SEEK_SET) < 0) { + if (fseek(fp, offset, SEEK_SET) < 0) { virReportSystemError(errno, "%s", _("fseek failed")); res = -errno; goto cleanup; } res = -1; - while (getline(&line, &n, fd) > 0) { + while (getline(&line, &n, fp) > 0) { char *ptr = strchr(line, ':'); if (!ptr) continue; @@ -251,7 +251,7 @@ lxcProcReadMeminfo(char *hostpath, memcpy(buf, virBufferCurrentContent(new_meminfo), res); cleanup: - VIR_FORCE_FCLOSE(fd); + VIR_FORCE_FCLOSE(fp); return res; } -- 2.34.1
There are two functions (lxcProcHostRead() and lxcProcReadMeminfo()) that could benefit from automatic file closing. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 9eff188d5f..1dedbc4069 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -126,7 +126,7 @@ lxcProcHostRead(char *path, size_t size, off_t offset) { - int fd; + VIR_AUTOCLOSE fd = -1; int res; fd = open(path, O_RDONLY); @@ -136,7 +136,6 @@ lxcProcHostRead(char *path, if ((res = pread(fd, buf, size, offset)) < 0) res = -errno; - VIR_FORCE_CLOSE(fd); return res; } @@ -148,7 +147,7 @@ lxcProcReadMeminfo(char *hostpath, off_t offset) { int res; - FILE *fp = NULL; + g_autoptr(FILE) fp = NULL; g_autofree char *line = NULL; size_t n; struct virLXCMeminfo meminfo; @@ -251,7 +250,6 @@ lxcProcReadMeminfo(char *hostpath, memcpy(buf, virBufferCurrentContent(new_meminfo), res); cleanup: - VIR_FORCE_FCLOSE(fp); return res; } -- 2.34.1
After previous cleanups, the cleanup label is no longer needed and can be removed. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 1dedbc4069..7c435803e9 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -162,14 +162,12 @@ lxcProcReadMeminfo(char *hostpath, fp = fopen(hostpath, "r"); if (fp == NULL) { virReportSystemError(errno, _("Cannot open %s"), hostpath); - res = -errno; - goto cleanup; + return -errno; } if (fseek(fp, offset, SEEK_SET) < 0) { virReportSystemError(errno, "%s", _("fseek failed")); - res = -errno; - goto cleanup; + return -errno; } res = -1; @@ -249,7 +247,6 @@ lxcProcReadMeminfo(char *hostpath, res = size; memcpy(buf, virBufferCurrentContent(new_meminfo), res); - cleanup: return res; } -- 2.34.1
In the lxcProcReadMeminfo() function we have @buffer variable which is statically allocated and then @new_meminfo which is just a pointer to the @buffer. This is needless, the @buffer can be accessed directly. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 7c435803e9..537e16c380 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -152,7 +152,6 @@ lxcProcReadMeminfo(char *hostpath, size_t n; struct virLXCMeminfo meminfo; g_auto(virBuffer) buffer = VIR_BUFFER_INITIALIZER; - virBuffer *new_meminfo = &buffer; if (virLXCCgroupGetMeminfo(&meminfo) < 0) { virErrorSetErrnoFromLastError(); @@ -180,12 +179,12 @@ lxcProcReadMeminfo(char *hostpath, if (STREQ(line, "MemTotal") && (virMemoryLimitIsSet(def->mem.hard_limit) || virDomainDefGetMemoryTotal(def))) { - virBufferAsprintf(new_meminfo, "MemTotal: %8llu kB\n", + virBufferAsprintf(&buffer, "MemTotal: %8llu kB\n", meminfo.memtotal); } else if (STREQ(line, "MemFree") && (virMemoryLimitIsSet(def->mem.hard_limit) || virDomainDefGetMemoryTotal(def))) { - virBufferAsprintf(new_meminfo, "MemFree: %8llu kB\n", + virBufferAsprintf(&buffer, "MemFree: %8llu kB\n", (meminfo.memtotal - meminfo.memusage)); } else if (STREQ(line, "MemAvailable") && (virMemoryLimitIsSet(def->mem.hard_limit) || @@ -193,59 +192,59 @@ lxcProcReadMeminfo(char *hostpath, /* MemAvailable is actually MemFree + SRReclaimable + some other bits, but MemFree is the closest approximation we have */ - virBufferAsprintf(new_meminfo, "MemAvailable: %8llu kB\n", + virBufferAsprintf(&buffer, "MemAvailable: %8llu kB\n", (meminfo.memtotal - meminfo.memusage)); } else if (STREQ(line, "Buffers")) { - virBufferAsprintf(new_meminfo, "Buffers: %8d kB\n", 0); + virBufferAsprintf(&buffer, "Buffers: %8d kB\n", 0); } else if (STREQ(line, "Cached")) { - virBufferAsprintf(new_meminfo, "Cached: %8llu kB\n", + virBufferAsprintf(&buffer, "Cached: %8llu kB\n", meminfo.cached); } else if (STREQ(line, "Active")) { - virBufferAsprintf(new_meminfo, "Active: %8llu kB\n", + virBufferAsprintf(&buffer, "Active: %8llu kB\n", (meminfo.active_anon + meminfo.active_file)); } else if (STREQ(line, "Inactive")) { - virBufferAsprintf(new_meminfo, "Inactive: %8llu kB\n", + virBufferAsprintf(&buffer, "Inactive: %8llu kB\n", (meminfo.inactive_anon + meminfo.inactive_file)); } else if (STREQ(line, "Active(anon)")) { - virBufferAsprintf(new_meminfo, "Active(anon): %8llu kB\n", + virBufferAsprintf(&buffer, "Active(anon): %8llu kB\n", meminfo.active_anon); } else if (STREQ(line, "Inactive(anon)")) { - virBufferAsprintf(new_meminfo, "Inactive(anon): %8llu kB\n", + virBufferAsprintf(&buffer, "Inactive(anon): %8llu kB\n", meminfo.inactive_anon); } else if (STREQ(line, "Active(file)")) { - virBufferAsprintf(new_meminfo, "Active(file): %8llu kB\n", + virBufferAsprintf(&buffer, "Active(file): %8llu kB\n", meminfo.active_file); } else if (STREQ(line, "Inactive(file)")) { - virBufferAsprintf(new_meminfo, "Inactive(file): %8llu kB\n", + virBufferAsprintf(&buffer, "Inactive(file): %8llu kB\n", meminfo.inactive_file); } else if (STREQ(line, "Unevictable")) { - virBufferAsprintf(new_meminfo, "Unevictable: %8llu kB\n", + virBufferAsprintf(&buffer, "Unevictable: %8llu kB\n", meminfo.unevictable); } else if (STREQ(line, "SwapTotal") && virMemoryLimitIsSet(def->mem.swap_hard_limit)) { - virBufferAsprintf(new_meminfo, "SwapTotal: %8llu kB\n", + virBufferAsprintf(&buffer, "SwapTotal: %8llu kB\n", (meminfo.swaptotal - meminfo.memtotal)); } else if (STREQ(line, "SwapFree") && virMemoryLimitIsSet(def->mem.swap_hard_limit)) { - virBufferAsprintf(new_meminfo, "SwapFree: %8llu kB\n", + virBufferAsprintf(&buffer, "SwapFree: %8llu kB\n", (meminfo.swaptotal - meminfo.memtotal - meminfo.swapusage + meminfo.memusage)); } else if (STREQ(line, "Slab")) { - virBufferAsprintf(new_meminfo, "Slab: %8d kB\n", 0); + virBufferAsprintf(&buffer, "Slab: %8d kB\n", 0); } else if (STREQ(line, "SReclaimable")) { - virBufferAsprintf(new_meminfo, "SReclaimable: %8d kB\n", 0); + virBufferAsprintf(&buffer, "SReclaimable: %8d kB\n", 0); } else if (STREQ(line, "SUnreclaim")) { - virBufferAsprintf(new_meminfo, "SUnreclaim: %8d kB\n", 0); + virBufferAsprintf(&buffer, "SUnreclaim: %8d kB\n", 0); } else { *ptr = ':'; - virBufferAdd(new_meminfo, line, -1); + virBufferAdd(&buffer, line, -1); } } - res = strlen(virBufferCurrentContent(new_meminfo)); + res = strlen(virBufferCurrentContent(&buffer)); if (res > size) res = size; - memcpy(buf, virBufferCurrentContent(new_meminfo), res); + memcpy(buf, virBufferCurrentContent(&buffer), res); return res; } -- 2.34.1
The idea behind lxcProcReadMeminfo() is that we read the host's /proc/meminfo and copy it line by line producing the content for container, changing only those lines we need. Thus, when a process inside container opens the file and lseek()-s to a different position (or reads the content in small chunks), we mirror the seek in host's /proc/meminfo. But this doesn't work really. We are not guaranteed to end up aligned on the beginning of new line. It's better if we construct the new content and then mimic seeking in it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 537e16c380..b068d21cf4 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -152,6 +152,7 @@ lxcProcReadMeminfo(char *hostpath, size_t n; struct virLXCMeminfo meminfo; g_auto(virBuffer) buffer = VIR_BUFFER_INITIALIZER; + const char *new_meminfo = NULL; if (virLXCCgroupGetMeminfo(&meminfo) < 0) { virErrorSetErrnoFromLastError(); @@ -164,11 +165,6 @@ lxcProcReadMeminfo(char *hostpath, return -errno; } - if (fseek(fp, offset, SEEK_SET) < 0) { - virReportSystemError(errno, "%s", _("fseek failed")); - return -errno; - } - res = -1; while (getline(&line, &n, fp) > 0) { char *ptr = strchr(line, ':'); @@ -241,10 +237,18 @@ lxcProcReadMeminfo(char *hostpath, } } - res = strlen(virBufferCurrentContent(&buffer)); + + new_meminfo = virBufferCurrentContent(&buffer); + res = virBufferUse(&buffer); + + if (offset > res) + return 0; + + res -= offset; + if (res > size) res = size; - memcpy(buf, virBufferCurrentContent(&buffer), res); + memcpy(buf, new_meminfo + offset, res); return res; } -- 2.34.1
When mounting a FUSE it is possible to bypass kernel cache by specifying -odirect_io mount option. This is what we currently do. However, FUSEv3 has a different approach - the open callback (lxcProcOpen() in our case) can set direct_io member of fuse_file_info struct. This results in the same behaviour, but also works with both FUSEv1 and FUSEv3. The latter does not have the mount option and uses per file approach. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index b068d21cf4..73d3ab1015 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -117,6 +117,7 @@ lxcProcOpen(const char *path, if ((fi->flags & O_ACCMODE) != O_RDONLY) return -EACCES; + fi->direct_io = 1; return 0; } @@ -329,7 +330,6 @@ lxcSetupFuse(struct virLXCFuse **f, /* process name is libvirt_lxc */ if (fuse_opt_add_arg(&args, "libvirt_lxc") == -1 || - fuse_opt_add_arg(&args, "-odirect_io") == -1 || fuse_opt_add_arg(&args, "-oallow_other") == -1 || fuse_opt_add_arg(&args, "-ofsname=libvirt") == -1) goto error; -- 2.34.1
If an app within a container wishes to read from /proc/meminfo from a different position than the beginning of the file, we can have FUSE keep track of all the lseek()-s and reflect them in @offset argument of read callback (lxcProcRead()). This is done by setting fuse_file_info::nonseekable. If we don't do this, then FUSE reports errors back the app that does lseek(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 73d3ab1015..181b05b605 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -118,6 +118,7 @@ lxcProcOpen(const char *path, return -EACCES; fi->direct_io = 1; + fi->nonseekable = 1; return 0; } -- 2.34.1
Plenty of projects switch from FUSE to FUSE3. This commit enables libvirt to compile with newer fuse-3.1 which allows users to have just one fuse package on their systems, allows us to set O_CLOEXEC on the fuse session FD. In general, FUSE3 offers more features, but apparently we don't need them right now. There is a rewrite guide at [1] but I've took most inspiration from sshfs [2]. 1: https://github.com/libfuse/libfuse/releases/tag/fuse-3.0.0 2: https://github.com/libfuse/sshfs Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_fuse.c | 73 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 181b05b605..802b2f5a1b 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -28,6 +28,9 @@ #if WITH_FUSE # define FUSE_USE_VERSION 26 # include <fuse.h> +# if FUSE_USE_VERSION >= 31 +# include <fuse_lowlevel.h> +# endif #endif #include "lxc_fuse.h" @@ -48,15 +51,19 @@ struct virLXCFuse { virThread thread; char *mountpoint; struct fuse *fuse; +# if FUSE_USE_VERSION < 31 struct fuse_chan *ch; +# else + struct fuse_session *sess; +# endif virMutex lock; }; static const char *fuse_meminfo_path = "/meminfo"; static int -lxcProcGetattr(const char *path, - struct stat *stbuf) +lxcProcGetattrImpl(const char *path, + struct stat *stbuf) { g_autofree char *mempath = NULL; struct stat sb; @@ -90,6 +97,40 @@ lxcProcGetattr(const char *path, return 0; } +# if FUSE_USE_VERSION >= 31 +static int +lxcProcGetattr(const char *path, + struct stat *stbuf, + struct fuse_file_info *fi G_GNUC_UNUSED) +{ + return lxcProcGetattrImpl(path, stbuf); +} +# else +static int lxcProcGetattr(const char *path, struct stat *stbuf) +{ + return lxcProcGetattrImpl(path, stbuf); +} +# endif + +# if FUSE_USE_VERSION >= 31 +static int +lxcProcReaddir(const char *path, + void *buf, + fuse_fill_dir_t filler, + off_t offset G_GNUC_UNUSED, + struct fuse_file_info *fi G_GNUC_UNUSED, + enum fuse_readdir_flags unused_flags G_GNUC_UNUSED) +{ + if (STRNEQ(path, "/")) + return -ENOENT; + + filler(buf, ".", NULL, 0, 0); + filler(buf, "..", NULL, 0, 0); + filler(buf, fuse_meminfo_path + 1, NULL, 0, 0); + + return 0; +} +# else static int lxcProcReaddir(const char *path, void *buf, @@ -106,6 +147,7 @@ lxcProcReaddir(const char *path, return 0; } +# endif static int lxcProcOpen(const char *path, @@ -292,7 +334,11 @@ lxcFuseDestroy(struct virLXCFuse *fuse) { VIR_LOCK_GUARD lock = virLockGuardLock(&fuse->lock); +# if FUSE_USE_VERSION >= 31 + fuse_unmount(fuse->fuse); +# else fuse_unmount(fuse->mountpoint, fuse->ch); +# endif g_clear_pointer(&fuse->fuse, fuse_destroy); } @@ -335,6 +381,23 @@ lxcSetupFuse(struct virLXCFuse **f, fuse_opt_add_arg(&args, "-ofsname=libvirt") == -1) goto error; +# if FUSE_USE_VERSION >= 31 + fuse->fuse = fuse_new(&args, &lxcProcOper, sizeof(lxcProcOper), fuse->def); + if (fuse->fuse == NULL) + goto error; + + if (fuse_mount(fuse->fuse, fuse->mountpoint) < 0) + goto error; + + fuse->sess = fuse_get_session(fuse->fuse); + + if (virSetInherit(fuse_session_fd(fuse->sess), false) < 0) { + virReportSystemError(errno, "%s", + _("Cannot disable close-on-exec flag")); + goto error; + } + +# else fuse->ch = fuse_mount(fuse->mountpoint, &args); if (fuse->ch == NULL) goto error; @@ -344,6 +407,7 @@ lxcSetupFuse(struct virLXCFuse **f, if (fuse->fuse == NULL) { goto error; } +# endif *f = g_steal_pointer(&fuse); ret = 0; @@ -352,8 +416,13 @@ lxcSetupFuse(struct virLXCFuse **f, return ret; error: +# if FUSE_USE_VERSION < 31 if (fuse->ch) fuse_unmount(fuse->mountpoint, fuse->ch); +# else + fuse_unmount(fuse->fuse); + fuse_destroy(fuse->fuse); +# endif g_free(fuse->mountpoint); virMutexDestroy(&fuse->lock); goto cleanup; -- 2.34.1
Now that we have support for fuse-3 we can detect it during the configure phase. Even better, we can detect fuse-3 first and fallback to old fuse only if the newer version doesn't exist. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- meson.build | 12 +++++++++--- src/lxc/lxc_fuse.c | 6 +++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/meson.build b/meson.build index 519a928c9a..f6259e59a4 100644 --- a/meson.build +++ b/meson.build @@ -947,10 +947,16 @@ if dlopen_dep.found() conf.set('WITH_DLFCN_H', 1) endif -fuse_version = '2.8.6' -fuse_dep = dependency('fuse', version: '>=' + fuse_version, required: get_option('fuse')) +fuse_version = '3.1.0' +fuse_dep = dependency('fuse3', version: '>=' + fuse_version, required: false) if fuse_dep.found() - conf.set('WITH_FUSE', 1) + conf.set('WITH_FUSE', 3) +else + fuse_version = '2.8.6' + fuse_dep = dependency('fuse', version: '>=' + fuse_version, required: get_option('fuse')) + if fuse_dep.found() + conf.set('WITH_FUSE', 1) + endif endif glib_version = '2.56.0' diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c index 802b2f5a1b..5e48206c39 100644 --- a/src/lxc/lxc_fuse.c +++ b/src/lxc/lxc_fuse.c @@ -26,7 +26,11 @@ #include <unistd.h> #if WITH_FUSE -# define FUSE_USE_VERSION 26 +# if WITH_FUSE == 3 +# define FUSE_USE_VERSION 31 +# else +# define FUSE_USE_VERSION 26 +# endif # include <fuse.h> # if FUSE_USE_VERSION >= 31 # include <fuse_lowlevel.h> -- 2.34.1
On a Monday in 2022, Michal Privoznik wrote:
Unfortunately, we can't just replace FUSE with FUSE3 because Ubuntu1804 doesn't have fuse3 package, but in a few weeks time, when the support for old Ubuntu is dropped, we can drop old FUSE support too.
Michal Prívozník (17): lxc_fuse: Hide struct virLXCFuse lxc_fuse: Move virLXCMeminfo struct into lxc_cgroup.h lxc_fuse.h: Don't include lxc_conf.h lxc_fuse: Move #include <fuse.h> lxc_fuse: Drop some G_GNUC_UNUSED attributes lxc_fuse.c: Modernize function declarations lxc_fuse: Prefer O_ACCMODE instead of & 3 lxcSetupFuse: Cleanup error paths lxcProcReadMeminfo: Rename @fd to @fp lxc_fuse: Use automatic file closing lxcProcReadMeminfo: Drop needless label lxcProcReadMeminfo: Drop @new_meminfo variable lxcProcReadMeminfo: Fix case when @offset != 0 lxc_fuse: Prefer fuse_file_info::direct_io over mount option lxc_fuse: Tell FUSE that /proc/meminfo is nonseekable lxc_fuse: Implement support for FUSE3 meson: Detect newer fuse
meson.build | 12 +- src/lxc/lxc_cgroup.h | 13 +++ src/lxc/lxc_fuse.c | 258 ++++++++++++++++++++++++++++++------------- src/lxc/lxc_fuse.h | 28 +---- 4 files changed, 207 insertions(+), 104 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik