On Wed, Sep 16, 2026 at 5:55 PM Luis Henriques <[email protected]> wrote: > > In order to reduce new tests setup/teardown code duplication, factor-out > these functions from the existing acl_cache test into a new fuse_common.c > file that can be reused in other tests. > > While there, also move some of the FIXTURE members into a fuse_common_ctx > structure that can be passed into setup/teardown instead of passing them by > reference. > > Signed-off-by: Luis Henriques <[email protected]> > --- > .../selftests/filesystems/fuse/Makefile | 8 +- > .../filesystems/fuse/fuse_acl_cache_test.c | 73 ++++--------------- > .../selftests/filesystems/fuse/fuse_common.c | 58 +++++++++++++++ > .../selftests/filesystems/fuse/fuse_common.h | 31 ++++++++ > 4 files changed, 111 insertions(+), 59 deletions(-) > create mode 100644 tools/testing/selftests/filesystems/fuse/fuse_common.c > create mode 100644 tools/testing/selftests/filesystems/fuse/fuse_common.h > > diff --git a/tools/testing/selftests/filesystems/fuse/Makefile > b/tools/testing/selftests/filesystems/fuse/Makefile > index a3ee9b3a2f5d..7744f796eb06 100644 > --- a/tools/testing/selftests/filesystems/fuse/Makefile > +++ b/tools/testing/selftests/filesystems/fuse/Makefile > @@ -21,8 +21,12 @@ ifeq ($(VAR_LDLIBS),) > VAR_LDLIBS := -lfuse3 -pthread > endif > > +CFLAGS += $(VAR_CFLAGS) > +LDLIBS += $(VAR_LDLIBS) > + > $(OUTPUT)/fuse_mnt: CFLAGS += $(VAR_CFLAGS) > $(OUTPUT)/fuse_mnt: LDLIBS += $(VAR_LDLIBS) > > -$(OUTPUT)/fuse_acl_cache_test: CFLAGS += $(VAR_CFLAGS) > -$(OUTPUT)/fuse_acl_cache_test: LDLIBS += $(VAR_LDLIBS) > +$(OUTPUT)/fuse_acl_cache_test: fuse_common.c fuse_acl_cache_test.c > + > +EXTRA_CLEAN := fuse_common.o > diff --git a/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c > b/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c > index 2411a6e285f1..d5a966e7fc15 100644 > --- a/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c > +++ b/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c > @@ -33,23 +33,15 @@ > */ > > #define _GNU_SOURCE > -#include <errno.h> > #include <fcntl.h> > #include <linux/limits.h> > -#include <pthread.h> > #include <stdint.h> > -#include <stdio.h> > -#include <stdlib.h> > -#include <string.h> > -#include <sys/stat.h> > #include <sys/xattr.h> > -#include <unistd.h> > - > -#define FUSE_USE_VERSION 31 > -#include <fuse_lowlevel.h> > > #include "kselftest_harness.h" > > +#include "fuse_common.h" > + > /* ---- ACL binary encoding ------------------------------------------------ > */ > /* > * POSIX ACL v2 xattr format (little-endian): > @@ -88,6 +80,7 @@ static const uint8_t acl_b[] = { > > struct daemon_state { > pthread_mutex_t lock; > + struct fuse_common_ctx ctx; > const uint8_t *acl; > size_t acl_size; > int getxattr_count; > @@ -176,69 +169,35 @@ static const struct fuse_lowlevel_ops fs_ops = { > .getxattr = fs_getxattr, > }; > > -/* ---- Daemon thread > ------------------------------------------------------- */ > - > -static void *run_daemon(void *arg) > -{ > - fuse_session_loop((struct fuse_session *)arg); > - return NULL; > -} > - > /* ---- kselftest harness > --------------------------------------------------- */ > > FIXTURE(acl_cache) { > - struct fuse_session *se; > - char mountpoint[PATH_MAX]; > - char file_path[PATH_MAX]; > - pthread_t thread; > + char file_path[PATH_MAX]; > }; >
Hi Luis, Thanks for the followup cleanup. Question: why did you move struct fuse_common_ctx out of the test fixture and into the global state? It not terribly wrong but seems unnecessary and inconsistent - fs_setup/teardown are in FIXTURE_SETUP/TEARDOWN so it is more consistent that fuse_common_ctx stays embedded in the fixure object, unless you have a reason to counter. With that fix, feel free to add Reviewed-by: Amir Goldstein <[email protected]> Thanks, Amir. > FIXTURE_SETUP(acl_cache) > { > - char *fuse_argv[] = { "fuse_acl_cache_test", NULL }; > - struct fuse_args args = FUSE_ARGS_INIT(1, fuse_argv); > + char err[MAX_ERR_MSG]; > > - g_ds.acl = acl_a; > - g_ds.acl_size = sizeof(acl_a); > + pthread_mutex_lock(&g_ds.lock); > + g_ds.acl = acl_a; > + g_ds.acl_size = sizeof(acl_a); > g_ds.getxattr_count = 0; > > - strcpy(self->mountpoint, "/tmp/acl_cache_test_XXXXXX"); > - if (!mkdtemp(self->mountpoint)) > - SKIP(return, "mkdtemp: %s", strerror(errno)); > + if (fs_setup(&fs_ops, &g_ds.ctx, err)) > + SKIP(goto out, err); > > snprintf(self->file_path, sizeof(self->file_path), > - "%s/" FILE_NAME, self->mountpoint); > - > - self->se = fuse_session_new(&args, &fs_ops, sizeof(fs_ops), NULL); > - if (!self->se) { > - rmdir(self->mountpoint); > - SKIP(return, "fuse_session_new failed"); > - } > - > - if (fuse_session_mount(self->se, self->mountpoint)) { > - fuse_session_destroy(self->se); > - rmdir(self->mountpoint); > - SKIP(return, "fuse_session_mount failed " > - "(missing fusermount3 or insufficient > privileges)"); > - } > - > - if (pthread_create(&self->thread, NULL, run_daemon, self->se)) { > - fuse_session_unmount(self->se); > - fuse_session_destroy(self->se); > - rmdir(self->mountpoint); > - SKIP(return, "pthread_create: %s", strerror(errno)); > - } > - > - fuse_opt_free_args(&args); > + "%s/" FILE_NAME, g_ds.ctx.mountpoint); > +out: > + pthread_mutex_unlock(&g_ds.lock); > } > > FIXTURE_TEARDOWN(acl_cache) > { > - fuse_session_exit(self->se); > - fuse_session_unmount(self->se); > - pthread_join(self->thread, NULL); > - fuse_session_destroy(self->se); > - rmdir(self->mountpoint); > + pthread_mutex_lock(&g_ds.lock); > + fs_teardown(&g_ds.ctx); > + pthread_mutex_unlock(&g_ds.lock); > } > > static int do_force_statx(const char *path) > diff --git a/tools/testing/selftests/filesystems/fuse/fuse_common.c > b/tools/testing/selftests/filesystems/fuse/fuse_common.c > new file mode 100644 > index 000000000000..d1d3dfd2ea67 > --- /dev/null > +++ b/tools/testing/selftests/filesystems/fuse/fuse_common.c > @@ -0,0 +1,58 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include "fuse_common.h" > + > +static void *run_daemon(void *arg) > +{ > + fuse_session_loop((struct fuse_session *)arg); > + return NULL; > +} > + > +int fs_setup(const struct fuse_lowlevel_ops *fs_ops, > + struct fuse_common_ctx *ctx, char *err) > +{ > + char *fuse_argv[] = { "fuse_test", NULL }; > + struct fuse_args args = FUSE_ARGS_INIT(1, fuse_argv); > + > + strcpy(ctx->mountpoint, MOUNTPOINT_TEMPLATE); > + if (!mkdtemp(ctx->mountpoint)) { > + snprintf(err, MAX_ERR_MSG, "mkdtemp: %s", strerror(errno)); > + return -1; > + } > + > + ctx->se = fuse_session_new(&args, fs_ops, sizeof(*fs_ops), NULL); > + if (!ctx->se) { > + rmdir(ctx->mountpoint); > + snprintf(err, MAX_ERR_MSG, "fuse_session_new failed"); > + return -1; > + } > + > + if (fuse_session_mount(ctx->se, ctx->mountpoint)) { > + fuse_session_destroy(ctx->se); > + rmdir(ctx->mountpoint); > + snprintf(err, MAX_ERR_MSG, "fuse_session_mount failed " > + "(missing fusermount3 or insufficient privileges)"); > + return -1; > + } > + > + if (pthread_create(&ctx->thread, NULL, run_daemon, ctx->se)) { > + fuse_session_unmount(ctx->se); > + fuse_session_destroy(ctx->se); > + rmdir(ctx->mountpoint); > + snprintf(err, MAX_ERR_MSG, "pthread_create: %s", > strerror(errno)); > + return -1; > + } > + > + fuse_opt_free_args(&args); > + > + return 0; > +} > + > +void fs_teardown(struct fuse_common_ctx *ctx) > +{ > + fuse_session_exit(ctx->se); > + fuse_session_unmount(ctx->se); > + pthread_join(ctx->thread, NULL); > + fuse_session_destroy(ctx->se); > + rmdir(ctx->mountpoint); > +} > diff --git a/tools/testing/selftests/filesystems/fuse/fuse_common.h > b/tools/testing/selftests/filesystems/fuse/fuse_common.h > new file mode 100644 > index 000000000000..e3dd85daa4b0 > --- /dev/null > +++ b/tools/testing/selftests/filesystems/fuse/fuse_common.h > @@ -0,0 +1,31 @@ > +#ifndef __SELFTEST_FUSE_COMMON_H__ > +#define __SELFTEST_FUSE_COMMON_H__ > + > +#define _GNU_SOURCE > +#include <errno.h> > +#include <pthread.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <unistd.h> > +#include <limits.h> > + > +#define FUSE_USE_VERSION 31 > +#include <fuse_lowlevel.h> > + > +#define MAX_ERR_MSG 256 > + > +#define MOUNTPOINT_TEMPLATE "/tmp/fuse_test_XXXXXX" > +#define MOUNTPOINT_SZ 64 > + > +struct fuse_common_ctx { > + struct fuse_session *se; > + char mountpoint[MOUNTPOINT_SZ]; > + pthread_t thread; > +}; > + > +int fs_setup(const struct fuse_lowlevel_ops *fs_ops, > + struct fuse_common_ctx *ctx, char *err); > +void fs_teardown(struct fuse_common_ctx *ctx); > + > +#endif /* __SELFTEST_FUSE_COMMON_H__ */
