[LTP] [PATCH v1] fsconfig04.c: Use FSCONFIG_SET_FD set overlayfs
Andrea Cervesato
andrea.cervesato@suse.com
Wed Apr 30 10:28:02 CEST 2025
Hi!
We miss a description for the commit.
On 4/30/25 21:22, Wei Gao via ltp wrote:
> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
> runtest/syscalls | 1 +
> testcases/kernel/syscalls/fsconfig/.gitignore | 1 +
> .../kernel/syscalls/fsconfig/fsconfig04.c | 132 ++++++++++++++++++
> 3 files changed, 134 insertions(+)
> create mode 100644 testcases/kernel/syscalls/fsconfig/fsconfig04.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index e369536ea..658ab24c4 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -410,6 +410,7 @@ fremovexattr02 fremovexattr02
> fsconfig01 fsconfig01
> fsconfig02 fsconfig02
> fsconfig03 fsconfig03
> +fsconfig04 fsconfig04
>
> fsmount01 fsmount01
> fsmount02 fsmount02
> diff --git a/testcases/kernel/syscalls/fsconfig/.gitignore b/testcases/kernel/syscalls/fsconfig/.gitignore
> index cfedae5f7..bd3754c34 100644
> --- a/testcases/kernel/syscalls/fsconfig/.gitignore
> +++ b/testcases/kernel/syscalls/fsconfig/.gitignore
> @@ -1,3 +1,4 @@
> /fsconfig01
> /fsconfig02
> /fsconfig03
> +/fsconfig04
> diff --git a/testcases/kernel/syscalls/fsconfig/fsconfig04.c b/testcases/kernel/syscalls/fsconfig/fsconfig04.c
> new file mode 100644
> index 000000000..c77dc9e3a
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fsconfig/fsconfig04.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2025 Wei Gao <wegao@suse.com>
> + *
> + * Basic fsconfig() test include use FSCONFIG_SET_FD set overlayfs
The description doesn't explain what test is doing. Also, we are not
testing only overlayfs inside it due to .all_filesystems=1, so I'm a bit
confused.
> + * Refer to selftests/filesystems/overlayfs/set_layers_via_fds.c
I checked the selftests but I don't see a 1:1 with our test. Is this a
port? If it's not we probably don't need this comment.
> + */
> +
> +#include "tst_test.h"
> +#include "lapi/fsmount.h"
> +#include "tst_safe_file_at.h"
> +#include "tst_safe_stdio.h"
> +
> +#define MNTPOINT "mntpoint"
> +#define OVERLAYMNT "set_layers_via_fds"
> +
> +static int fd, fd_context, fd_overlay;
They are not initialized to -1, so we are going to fail SAFE_CLOSE
inside cleanup function.
Also it's better to define variables on multiple lines.
> +
> +static void cleanup(void)
> +{
> + if (fd != -1)
> + SAFE_CLOSE(fd);
> +
> + if (fd_context != -1)
> + SAFE_CLOSE(fd_context);
> +
> + if (fd_overlay != -1)
> + SAFE_CLOSE(fd_overlay);
> +}
> +
> +static void run(void)
> +{
> + int fsmfd;
> +
> + fd = TST_EXP_FD(fsopen(tst_device->fs_type, 0));
> +
> + TST_EXP_PASS(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
This should be tested separately.
> +
> + TST_EXP_PASS(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
Also this.
> +
> + fsmfd = TST_EXP_FD(fsmount(fd, 0, 0));
> +
> + TST_EXP_PASS(mkdirat(fsmfd, "w", 0755));
> + TST_EXP_PASS(mkdirat(fsmfd, "u", 0755));
> + TST_EXP_PASS(mkdirat(fsmfd, "l1", 0755));
> + TST_EXP_PASS(mkdirat(fsmfd, "l2", 0755));
> +
> + int layer_fds[] = { [0 ... 3] = -EBADF };
> +
> + layer_fds[0] = SAFE_OPENAT(fsmfd, "w", O_DIRECTORY);
> + layer_fds[1] = SAFE_OPENAT(fsmfd, "u", O_DIRECTORY);
> + layer_fds[2] = SAFE_OPENAT(fsmfd, "l1", O_DIRECTORY);
> + layer_fds[3] = SAFE_OPENAT(fsmfd, "l2", O_DIRECTORY);
> +
> + TST_EXP_PASS(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
> + MOVE_MOUNT_F_EMPTY_PATH));
> +
> + SAFE_CLOSE(fsmfd);
This is one test case.
> +
> + fd_context = TST_EXP_FD(fsopen("overlay", 0));
> +
> + TST_EXP_PASS(fsconfig(fd_context, FSCONFIG_SET_FD, "workdir", NULL, layer_fds[0]));
> + TST_EXP_PASS(fsconfig(fd_context, FSCONFIG_SET_FD, "upperdir", NULL, layer_fds[1]));
> + TST_EXP_PASS(fsconfig(fd_context, FSCONFIG_SET_FD, "lowerdir+", NULL, layer_fds[2]));
> + TST_EXP_PASS(fsconfig(fd_context, FSCONFIG_SET_FD, "lowerdir+", NULL, layer_fds[3]));
> +
> + TST_EXP_PASS(fsconfig(fd_context, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
> +
> + SAFE_MKDIR(OVERLAYMNT, 0555);
> +
> + fd_overlay = TST_EXP_FD(fsmount(fd_context, 0, 0));
> + SAFE_CLOSE(fd_context);
> +
> + TST_EXP_PASS(move_mount(fd_overlay, "", AT_FDCWD, OVERLAYMNT,
> + MOVE_MOUNT_F_EMPTY_PATH));
> +
> + char line[PATH_MAX];
> + FILE *file;
> + int ret = 0;
> + char workdir[PATH_MAX], upperdir[PATH_MAX], lowerdir1[PATH_MAX], lowerdir2[PATH_MAX];
> + char *tmpdir_path = tst_tmpdir_path();
> +
> + sprintf(workdir, "workdir=%s/%s/w", tmpdir_path, MNTPOINT);
> + sprintf(upperdir, "upperdir=%s/%s/u", tmpdir_path, MNTPOINT);
> + sprintf(lowerdir1, "lowerdir+=%s/%s/l1", tmpdir_path, MNTPOINT);
> + sprintf(lowerdir2, "lowerdir+=%s/%s/l2", tmpdir_path, MNTPOINT);
> +
> + file = SAFE_FOPEN("/proc/mounts", "r");
> + while (fgets(line, sizeof(line), file)) {
> + if (strstr(line, workdir) && strstr(line, upperdir)
> + && strstr(line, lowerdir1) && strstr(line, lowerdir2)) {
> + ret = 1;
> + break;
> + }
> + }
> +
> + if (ret == 1)
> + tst_res(TPASS, "overlay mount check pass");
> + else
> + tst_res(TFAIL, "overlay mount check failed");
> +
> + TST_EXP_PASS_SILENT(system("ls -l"));
> +
This is not needed.
> + char rmcmd[PATH_MAX];
> +
> + sprintf(rmcmd, "rm -r ./%s/*", MNTPOINT);
> + TST_EXP_PASS_SILENT(system(rmcmd));
Why we try to remove the whole mnpoint via rm command?
> +
> + SAFE_CLOSE(fd_overlay);
> + SAFE_UMOUNT(OVERLAYMNT);
> + SAFE_RMDIR(OVERLAYMNT);
> +
> + SAFE_CLOSE(fd);
> +
> + for (int i = 0; i < 4; i++)
> + SAFE_CLOSE(layer_fds[i]);
> +
> + if (tst_is_mounted_at_tmpdir(MNTPOINT)) {
> + SAFE_UMOUNT(MNTPOINT);
> + }
> +}
> +
> +static struct tst_test test = {
> + .test_all = run,
> + .setup = fsopen_supported_by_kernel,
> + .cleanup = cleanup,
> + .needs_root = 1,
> + .format_device = 1,
> + .mntpoint = MNTPOINT,
> + .all_filesystems = 1,
> + .skip_filesystems = (const char *const []){"ntfs", "vfat", "exfat", NULL},
> +};
In general I think we are not achieving the goal given by
https://github.com/linux-test-project/ltp/issues/1169 so this patch
can't be considered valid. There are many issues due to the mix of
different flags usage and the broken execution of the test as well.
The idea is to test the following flags, each one separately in order to
verify if they are correctly working:
- FSCONFIG_SET_PATH
- FSCONFIG_SET_BINARY
- FSCONFIG_SET_FD
All of them should be tested with a separate test in an atomic way: we
open a new filesystem context object using fsopen(), then we set its
key/keys via fsconfig() using one of the flags above and we verify that
they affected the filesystem context.
There are also many other flags. I suggest you to take a look at
include/uapi/linux/mount.h in case we need to cover more of them.
- Andrea
More information about the ltp
mailing list