Re: [RFC PATCH v3 4/8] selftests/fuse: factor-out test fixture setup/teardown

[Luis Henriques <luis@igalia.com>]

On Sat, Sep 05 2026, Amir Goldstein wrote:

> On Fri, Sep 4, 2026 at 12:38 PM Luis Henriques <luis@igalia.com> 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.
>>
>> Signed-off-by: Luis Henriques <luis@igalia.com>
>> ---
>>  .../selftests/filesystems/fuse/Makefile       |  8 ++-
>>  .../filesystems/fuse/fuse_acl_cache_test.c    | 62 +++----------------
>>  .../selftests/filesystems/fuse/fuse_common.c  | 60 ++++++++++++++++++
>>  .../selftests/filesystems/fuse/fuse_common.h  | 25 ++++++++
>>  4 files changed, 100 insertions(+), 55 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..12cbf9753d03 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):
>> @@ -176,69 +168,33 @@ 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                 mountpoint[MOUNTPOINT_SZ];
>>         char                 file_path[PATH_MAX];
>>         pthread_t            thread;
>>  };
>
> Nice!
> I think it would be even nicer to have struct fuse_common_ctx
> with the common members embedded in the per test state,
> but I'll let you decide if you want to do that or not.

Thanks for the suggestion.  It may make sense indeed to have that common
struct, although it would probably only include the spinlock and a bool
for enabling/disabling the cache.  I'll have a closer look and see if it
makes sense.

Cheers,
-- 
Luís

>
> 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);
>> +       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(&self->se, self->mountpoint, &fs_ops, &self->thread, err))
>> +               SKIP(return, 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);
>>  }
>>
>>  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);
>> +       fs_teardown(self->se, self->thread, self->mountpoint);
>>  }
>>
>>  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..3a91cac25b81
>> --- /dev/null
>> +++ b/tools/testing/selftests/filesystems/fuse/fuse_common.c
>> @@ -0,0 +1,60 @@
>> +// 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(struct fuse_session **se, char *mountpoint,
>> +            const struct fuse_lowlevel_ops *fs_ops,
>> +            pthread_t *thread, char *err)
>> +{
>> +       char *fuse_argv[] = { "fuse_test", NULL };
>> +       struct fuse_args args = FUSE_ARGS_INIT(1, fuse_argv);
>> +
>> +       strcpy(mountpoint, MOUNTPOINT_TEMPLATE);
>> +       if (!mkdtemp(mountpoint)) {
>> +               snprintf(err, MAX_ERR_MSG, "mkdtemp: %s", strerror(errno));
>> +               return -1;
>> +       }
>> +
>> +       *se = fuse_session_new(&args, fs_ops, sizeof(*fs_ops), NULL);
>> +       if (!*se) {
>> +               rmdir(mountpoint);
>> +               snprintf(err, MAX_ERR_MSG, "fuse_session_new failed");
>> +               return -1;
>> +       }
>> +
>> +       if (fuse_session_mount(*se, mountpoint)) {
>> +               fuse_session_destroy(*se);
>> +               rmdir(mountpoint);
>> +               snprintf(err, MAX_ERR_MSG, "fuse_session_mount failed "
>> +                       "(missing fusermount3 or insufficient privileges)");
>> +               return -1;
>> +       }
>> +
>> +       if (pthread_create(thread, NULL, run_daemon, *se)) {
>> +               fuse_session_unmount(*se);
>> +               fuse_session_destroy(*se);
>> +               rmdir(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_session *se, pthread_t thread, char *mountpoint)
>> +{
>> +       fuse_session_exit(se);
>> +       fuse_session_unmount(se);
>> +       pthread_join(thread, NULL);
>> +       fuse_session_destroy(se);
>> +       rmdir(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..77d5eb58550d
>> --- /dev/null
>> +++ b/tools/testing/selftests/filesystems/fuse/fuse_common.h
>> @@ -0,0 +1,25 @@
>> +#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>
>> +
>> +#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
>> +
>> +int fs_setup(struct fuse_session **se, char *mountpoint,
>> +            const struct fuse_lowlevel_ops *fs_ops,
>> +            pthread_t *thread, char *err);
>> +void fs_teardown(struct fuse_session *se, pthread_t thread, char *mountpoint);
>> +
>> +#endif /* __SELFTEST_FUSE_COMMON_H__ */