diff mbox series

[dhowells/mount-api,2/2] selftests: implement a test for a new mount API

Message ID 20180810220027.2735-2-avagin@openvz.org
State New, archived
Headers show
Series [dhowells/mount-api,1/2] fs/fsconfig: handle FSCONFIG_CMD_RECONFIGURE | expand

Commit Message

Andrey Vagin Aug. 10, 2018, 10 p.m. UTC
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

Comments

David Howells Aug. 10, 2018, 10:55 p.m. UTC | #1
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
Andrey Vagin Aug. 20, 2018, 11:29 p.m. UTC | #2
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;
+}
Dave Chinner Aug. 20, 2018, 11:47 p.m. UTC | #3
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.
David Howells Aug. 21, 2018, 9:19 a.m. UTC | #4
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 mbox series

Patch

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;
+}