| Message ID | 20180810220027.2735-2-avagin@openvz.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [dhowells/mount-api,1/2] fs/fsconfig: handle FSCONFIG_CMD_RECONFIGURE | expand |
Andrei Vagin <avagin@openvz.org> wrote: > +static const char dev[] = "s test_fsopen_123"; Did you mean to prefix that with "s "? > + if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "source", dev, 0) < 0) > + return pr_err("fsconfig_set_string sets source = %s", dev); Note that it's not necessary to provide a source for /proc since it will just ignore it. Anyway, thanks for doing this! David
On Fri, Aug 10, 2018 at 11:55:30PM +0100, David Howells wrote: > Andrei Vagin <avagin@openvz.org> wrote: > > > +static const char dev[] = "s test_fsopen_123"; > > Did you mean to prefix that with "s "? "s " means "source" here, it is an effect of a previous version of this interface https://lwn.net/Articles/753473/ > > > + if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "source", dev, 0) < 0) > > + return pr_err("fsconfig_set_string sets source = %s", dev); > > Note that it's not necessary to provide a source for /proc since it will just > ignore it. I haven't known this. Thanks. A sligtly updated patch is attached to this message. > > Anyway, thanks for doing this! > > David > From 0fb778c4189bc739c253d0a76830307ed6d3e1d4 Mon Sep 17 00:00:00 2001 From: Andrei Vagin <avagin@gmail.com> Date: Mon, 6 Aug 2018 19:33:16 -0700 Subject: [PATCH] selftests: implement a test for a new mount API Currently, a reconfigure call-back is implemented only for the proc file system. This test creates a new mount and pid namespace and then we can create a new proc mount instance and be sure that a host configuration will not be affected. Signed-off-by: Andrei Vagin <avagin@gmail.com> --- tools/testing/selftests/fsopen/Makefile | 5 + tools/testing/selftests/fsopen/config | 2 + tools/testing/selftests/fsopen/fsopen.c | 132 ++++++++++++++++++++++++ 3 files changed, 139 insertions(+) create mode 100644 tools/testing/selftests/fsopen/Makefile create mode 100644 tools/testing/selftests/fsopen/config create mode 100644 tools/testing/selftests/fsopen/fsopen.c diff --git a/tools/testing/selftests/fsopen/Makefile b/tools/testing/selftests/fsopen/Makefile new file mode 100644 index 000000000000..bfb1dd015e37 --- /dev/null +++ b/tools/testing/selftests/fsopen/Makefile @@ -0,0 +1,5 @@ +CFLAGS = -Wall -I ../../../../usr/include $(EXTRA_CFLAGS) + +TEST_GEN_PROGS := fsopen + +include ../lib.mk diff --git a/tools/testing/selftests/fsopen/config b/tools/testing/selftests/fsopen/config new file mode 100644 index 000000000000..d2ced6498f5a --- /dev/null +++ b/tools/testing/selftests/fsopen/config @@ -0,0 +1,2 @@ +CONFIG_PROC_FS=y +CONFIG_PID_NS=y diff --git a/tools/testing/selftests/fsopen/fsopen.c b/tools/testing/selftests/fsopen/fsopen.c new file mode 100644 index 000000000000..20b75506b9f4 --- /dev/null +++ b/tools/testing/selftests/fsopen/fsopen.c @@ -0,0 +1,132 @@ +#define _GNU_SOURCE /* See feature_test_macros(7) */ +#include <unistd.h> +#include <stdio.h> +#include <unistd.h> +#include <sched.h> +#include <linux/fs.h> +#include <sys/syscall.h> +#include <sys/wait.h> + +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> + +static int sys_mount(char *dev_name, char *dir_name, char *type, + unsigned long flags, void *data) +{ + return syscall(__NR_mount, dev_name, dir_name, type, flags, data); +} + +static int sys_fsopen(const char *fs_name, unsigned int flags) +{ + return syscall(__NR_fsopen, fs_name, flags); +} + +static int sys_fsmount(int fsfd, unsigned int flags, unsigned int ms_flags) +{ + return syscall(__NR_fsmount, fsfd, flags, ms_flags); +} + +static int sys_fspick(unsigned int dirfd, const char *path, unsigned int flags) +{ + return syscall(__NR_fspick, dirfd, path, flags); +} + +static int sys_fsconfig(int fd, unsigned int cmd, const char *key, const char *value, int aux) +{ + return syscall(__NR_fsconfig, fd, cmd, key, value, aux); +} + +static int move_mount(int from_dfd, char *from_pathname, + int to_dfd, char *to_pathname, unsigned int flags) +{ + return syscall(__NR_move_mount, from_dfd, from_pathname, to_dfd, to_pathname, flags); +} + +#define pr_perror(fmt, ...) \ + ({ \ + fprintf(stderr, "%s:%d:" fmt ": %m\n", \ + __func__, __LINE__, ##__VA_ARGS__); \ + 1; \ + }) + +#define pr_err(fmt, ...) \ + ({ \ + fprintf(stderr, "%s:%d:" fmt "\n", \ + __func__, __LINE__, ##__VA_ARGS__); \ + 1; \ + }) + +//static const char opts[] = "rw"; +static int test() +{ + int fsfd, fsfd2, dfd; + char buf[4096]; + + fsfd = sys_fsopen("proc", 0); + if (fsfd < 0) + return pr_perror("sys_fsopen"); + + if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "abrakadabra", "0", 0) == 0) + return pr_err("fsconfig() didn't return an error"); + + if (read(fsfd, buf, sizeof(buf)) <= 0) + return pr_perror("Unable to get a error"); + + pr_err("%s", buf); + + if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "gid", "0", 0)) + return pr_perror("sys_fsconfig sets gid=5"); + + if (sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) < 0) + return pr_perror("fsconfig_cmd_create"); + + dfd = sys_fsmount(fsfd, 0, 0); + if (dfd < 0) + return pr_perror("sys_fsmount"); + + if (move_mount(dfd, ".", AT_FDCWD, "/proc", 0)) + return pr_perror("move_mount"); + + fsfd2 = sys_fspick(dfd, ".", 0); + if (fsfd2 < 0) + return pr_perror("sys_fspick"); + + if (sys_fsconfig(fsfd2, FSCONFIG_SET_STRING, "gid", "5", 0)) + return pr_perror("fsconfig_set_string sets gid=5"); + + if (sys_fsconfig(fsfd2, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0)) + return pr_perror("fsconfig_cmd_reconfigure"); + + if (close(dfd) < 0) + return pr_perror("close(dfd)"); + if (close(fsfd) < 0) + return pr_perror("close(fsfd)"); + if (close(fsfd2) < 0) + return pr_perror("close(fsfd2)"); + + return 0; +} + +int main() +{ + pid_t pid; + int status; + + if (unshare(CLONE_NEWNS | CLONE_NEWPID)) + return pr_perror("unshare"); + + if (sys_mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL)) + return pr_perror("mount"); + + pid = fork(); + if (pid < 0) + return pr_perror("fork"); + if (pid == 0) + return test(); + if (waitpid(pid, &status, 0) != pid) + return pr_perror("waitpid"); + if (status) + return 1; + return 0; +}
On Fri, Aug 10, 2018 at 03:00:27PM -0700, Andrei Vagin wrote: > From: Andrei Vagin <avagin@gmail.com> > > Currently, a reconfigure call-back is implemented only for the proc file > system. This test creates a new mount and pid namespace and then we can > create a new proc mount instance and be sure that a host configuration > will not be affected. > > Signed-off-by: Andrei Vagin <avagin@gmail.com> > --- > tools/testing/selftests/fsopen/Makefile | 5 + > tools/testing/selftests/fsopen/config | 2 + > tools/testing/selftests/fsopen/fsopen.c | 120 ++++++++++++++++++++++++ > 3 files changed, 127 insertions(+) > create mode 100644 tools/testing/selftests/fsopen/Makefile > create mode 100644 tools/testing/selftests/fsopen/config > create mode 100644 tools/testing/selftests/fsopen/fsopen.c can we get all this test stuff put into fstests? And then add a config option to the fstests mount commands so that they use the internal mount command to exercise the new API rather than use the existing mount binary? That way every filesystem developer /will/ exercise the new mount API in lots of interesting and unusual ways that are expected to work (or not work, as fstests exercises failure cases, too) in their day-to-day testing and development. This will flush out the bugs and problems much, much faster than a small selftest in the kernel tree that doesn't really interact with the bulk of the code that runs when filesystems are mounted/remounted. Cheers, Dave.
Andrei Vagin <avagin@virtuozzo.com> wrote: > > > +static const char dev[] = "s test_fsopen_123"; > > > > Did you mean to prefix that with "s "? > > "s " means "source" here, it is an effect of a previous version of this > interface > > https://lwn.net/Articles/753473/ Yeah, well Linus objected, so we're not using write() now. So the key string given to fsconfig() now indicates this information (ie. "source"). David
diff --git a/tools/testing/selftests/fsopen/Makefile b/tools/testing/selftests/fsopen/Makefile new file mode 100644 index 000000000000..bfb1dd015e37 --- /dev/null +++ b/tools/testing/selftests/fsopen/Makefile @@ -0,0 +1,5 @@ +CFLAGS = -Wall -I ../../../../usr/include $(EXTRA_CFLAGS) + +TEST_GEN_PROGS := fsopen + +include ../lib.mk diff --git a/tools/testing/selftests/fsopen/config b/tools/testing/selftests/fsopen/config new file mode 100644 index 000000000000..d2ced6498f5a --- /dev/null +++ b/tools/testing/selftests/fsopen/config @@ -0,0 +1,2 @@ +CONFIG_PROC_FS=y +CONFIG_PID_NS=y diff --git a/tools/testing/selftests/fsopen/fsopen.c b/tools/testing/selftests/fsopen/fsopen.c new file mode 100644 index 000000000000..c7cbfcda40ad --- /dev/null +++ b/tools/testing/selftests/fsopen/fsopen.c @@ -0,0 +1,120 @@ +#define _GNU_SOURCE /* See feature_test_macros(7) */ +#include <unistd.h> +#include <stdio.h> +#include <unistd.h> +#include <sched.h> +#include <linux/fs.h> +#include <sys/syscall.h> +#include <sys/wait.h> + +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> + +static int sys_mount(char *dev_name, char *dir_name, char *type, + unsigned long flags, void *data) +{ + return syscall(__NR_mount, dev_name, dir_name, type, flags, data); +} + +static int sys_fsopen(const char *fs_name, unsigned int flags) +{ + return syscall(__NR_fsopen, fs_name, flags); +} + +static int sys_fsmount(int fsfd, unsigned int flags, unsigned int ms_flags) +{ + return syscall(__NR_fsmount, fsfd, flags, ms_flags); +} + +static int sys_fspick(unsigned int dirfd, const char *path, unsigned int flags) +{ + return syscall(__NR_fspick, dirfd, path, flags); +} + +static int sys_fsconfig(int fd, unsigned int cmd, const char *key, const char *value, int aux) +{ + return syscall(__NR_fsconfig, fd, cmd, key, value, aux); +} + +static int move_mount(int from_dfd, char *from_pathname, + int to_dfd, char *to_pathname, unsigned int flags) +{ + return syscall(__NR_move_mount, from_dfd, from_pathname, to_dfd, to_pathname, flags); +} + +#define pr_err(fmt, ...) \ + ({ \ + fprintf(stderr, "%s:%d:" fmt ": %m\n", \ + __func__, __LINE__, ##__VA_ARGS__); \ + 1; \ + }) + +static const char dev[] = "s test_fsopen_123"; +//static const char opts[] = "rw"; +static int test() +{ + int fsfd, fsfd2, dfd; + + fsfd = sys_fsopen("proc", 0); + if (fsfd < 0) + return pr_err("sys_fsopen"); + + if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "gid", "0", 0)) + return pr_err("sys_fsconfig sets gid=5"); + + if (sys_fsconfig(fsfd, FSCONFIG_SET_STRING, "source", dev, 0) < 0) + return pr_err("fsconfig_set_string sets source = %s", dev); + + if (sys_fsconfig(fsfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0) < 0) + return pr_err("fsconfig_cmd_create"); + + dfd = sys_fsmount(fsfd, 0, 0); + if (dfd < 0) + return pr_err("sys_fsmount"); + + if (move_mount(dfd, ".", AT_FDCWD, "/proc", 0)) + return pr_err("move_mount"); + + fsfd2 = sys_fspick(dfd, ".", 0); + if (fsfd2 < 0) + return pr_err("sys_fspick"); + + if (sys_fsconfig(fsfd2, FSCONFIG_SET_STRING, "gid", "5", 0)) + return pr_err("fsconfig_set_string sets gid=5"); + + if (sys_fsconfig(fsfd2, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0)) + return pr_err("fsconfig_cmd_reconfigure"); + + if (close(dfd) < 0) + return pr_err("close(dfd)"); + if (close(fsfd) < 0) + return pr_err("close(fsfd)"); + if (close(fsfd2) < 0) + return pr_err("close(fsfd2)"); + + return 0; +} + +int main() +{ + pid_t pid; + int status; + + if (unshare(CLONE_NEWNS | CLONE_NEWPID)) + return pr_err("unshare"); + + if (sys_mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL)) + return pr_err("mount"); + + pid = fork(); + if (pid < 0) + return pr_err("fork"); + if (pid == 0) + return test(); + if (waitpid(pid, &status, 0) != pid) + return pr_err("waitpid"); + if (status) + return 1; + return 0; +}