All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls
@ 2020-02-27  5:14 Viresh Kumar
  2020-02-27  5:14 ` [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper Viresh Kumar
                   ` (10 more replies)
  0 siblings, 11 replies; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

Hello,

Here is the fifth version of my work rebased over the latest changes pushed
around fsmount work by Zorro. This is tested on both ARM and X86 platforms now
and no more errors are seen with it.

V4->V5:
- s/TBROK/TFAIL/ in run(int n) callbacks.
- Many conversions like TEST(fd = fsopen());
- A new implementation of tst_is_mounted().
- fspick.h created to take care of duplicate code.
- tmp directory not removed from code as that will be auto done anyway.
- Other minor improvements.

V3->V4:
- Only the first patch was resent after fixing git log and minor code
  modifications.

V2->V3:
- Fix errors with xfs.
- Add macros to define test names.
- Continue with multiple iteration tests on failures, instead of
  returning early.
- Set value of is_mounted in open_tree patch.
- Rename tst_ismount() and make it return 1 on success.
- Fix various formatting issues.
- Add all reviewed/ack tags.

V1->V2:
- Lots of changes, really :)
- Rebased over Zorro's work with fsmount.
- The success tests are modified to test all possible flags, attributes
  as well.
- Create tst_ismount() and fsopen_supported_by_kernel() helpers.
- Verify if mount is successful or not using tst_ismount().
- Remove min_kver and instead check if older kernels have patches
  backported or not.
- Test on multiple filesystems.
- Better handling of failures during setup/run callbacks.
- One of the patches around fsmount.h is already merged, dropped it now.
- Other minor cleanups.

Viresh Kumar (10):
  tst_device: Add tst_is_mounted() helper
  lapi/fsmount.h: Add fsopen_supported_by_kernel()
  lapi/fsmount.h: Include "lapi/fcntl.h"
  syscalls/fsopen: New tests
  syscalls/fsconfig: New tests
  syscalls/fsmount: Improve fsmount01 test
  syscalls/fsmount: Add failure tests
  syscalls/move_mount: New tests
  syscalls/fspick: New tests
  syscalls/open_tree: New tests

 include/lapi/fsmount.h                        |  11 +-
 include/tst_device.h                          |   6 +
 lib/tst_device.c                              |  14 ++
 runtest/syscalls                              |  16 +++
 testcases/kernel/syscalls/fsconfig/.gitignore |   2 +
 testcases/kernel/syscalls/fsconfig/Makefile   |   6 +
 .../kernel/syscalls/fsconfig/fsconfig01.c     | 101 ++++++++++++++
 .../kernel/syscalls/fsconfig/fsconfig02.c     |  99 ++++++++++++++
 testcases/kernel/syscalls/fsmount/.gitignore  |   1 +
 testcases/kernel/syscalls/fsmount/fsmount01.c | 123 +++++++++---------
 testcases/kernel/syscalls/fsmount/fsmount02.c |  80 ++++++++++++
 testcases/kernel/syscalls/fsopen/.gitignore   |   2 +
 testcases/kernel/syscalls/fsopen/Makefile     |   6 +
 testcases/kernel/syscalls/fsopen/fsopen01.c   |  80 ++++++++++++
 testcases/kernel/syscalls/fsopen/fsopen02.c   |  58 +++++++++
 testcases/kernel/syscalls/fspick/.gitignore   |   2 +
 testcases/kernel/syscalls/fspick/Makefile     |   6 +
 testcases/kernel/syscalls/fspick/fspick.h     |  60 +++++++++
 testcases/kernel/syscalls/fspick/fspick01.c   |  62 +++++++++
 testcases/kernel/syscalls/fspick/fspick02.c   |  54 ++++++++
 .../kernel/syscalls/move_mount/.gitignore     |   2 +
 testcases/kernel/syscalls/move_mount/Makefile |   6 +
 .../kernel/syscalls/move_mount/move_mount01.c |  83 ++++++++++++
 .../kernel/syscalls/move_mount/move_mount02.c |  92 +++++++++++++
 .../kernel/syscalls/open_tree/.gitignore      |   2 +
 testcases/kernel/syscalls/open_tree/Makefile  |   6 +
 .../kernel/syscalls/open_tree/open_tree01.c   | 108 +++++++++++++++
 .../kernel/syscalls/open_tree/open_tree02.c   | 105 +++++++++++++++
 28 files changed, 1132 insertions(+), 61 deletions(-)
 create mode 100644 testcases/kernel/syscalls/fsconfig/.gitignore
 create mode 100644 testcases/kernel/syscalls/fsconfig/Makefile
 create mode 100644 testcases/kernel/syscalls/fsconfig/fsconfig01.c
 create mode 100644 testcases/kernel/syscalls/fsconfig/fsconfig02.c
 create mode 100644 testcases/kernel/syscalls/fsmount/fsmount02.c
 create mode 100644 testcases/kernel/syscalls/fsopen/.gitignore
 create mode 100644 testcases/kernel/syscalls/fsopen/Makefile
 create mode 100644 testcases/kernel/syscalls/fsopen/fsopen01.c
 create mode 100644 testcases/kernel/syscalls/fsopen/fsopen02.c
 create mode 100644 testcases/kernel/syscalls/fspick/.gitignore
 create mode 100644 testcases/kernel/syscalls/fspick/Makefile
 create mode 100644 testcases/kernel/syscalls/fspick/fspick.h
 create mode 100644 testcases/kernel/syscalls/fspick/fspick01.c
 create mode 100644 testcases/kernel/syscalls/fspick/fspick02.c
 create mode 100644 testcases/kernel/syscalls/move_mount/.gitignore
 create mode 100644 testcases/kernel/syscalls/move_mount/Makefile
 create mode 100644 testcases/kernel/syscalls/move_mount/move_mount01.c
 create mode 100644 testcases/kernel/syscalls/move_mount/move_mount02.c
 create mode 100644 testcases/kernel/syscalls/open_tree/.gitignore
 create mode 100644 testcases/kernel/syscalls/open_tree/Makefile
 create mode 100644 testcases/kernel/syscalls/open_tree/open_tree01.c
 create mode 100644 testcases/kernel/syscalls/open_tree/open_tree02.c

-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
@ 2020-02-27  5:14 ` Viresh Kumar
  2020-03-06 12:45   ` Cyril Hrubis
  2020-02-27  5:14 ` [LTP] [PATCH V5 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel() Viresh Kumar
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

This patch moves the ismount() helper added to the fsmount syscall tests
to the standard library and renames it to tst_is_mounted(). The helper
can be used across different files now.

Minor modifications are also done to the helper.

Acked-by: Li Wang <liwang@redhat.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/tst_device.h                          |  6 +++++
 lib/tst_device.c                              | 14 +++++++++++
 testcases/kernel/syscalls/fsmount/fsmount01.c | 25 +------------------
 3 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/include/tst_device.h b/include/tst_device.h
index f5609f5986bb..bd6910672d2f 100644
--- a/include/tst_device.h
+++ b/include/tst_device.h
@@ -36,6 +36,12 @@ extern struct tst_device *tst_device;
  */
 int tst_umount(const char *path);
 
+/*
+ * Verifies if an earlier mount is successful or not.
+ * @path: Mount path to verify
+ */
+int tst_is_mounted(const char *path);
+
 /*
  * Clears a first few blocks of the device. This is needed when device has
  * already been formatted with a filesystems, subset of mkfs.foo utils aborts
diff --git a/lib/tst_device.c b/lib/tst_device.c
index 8b5459def1cb..8bf6dacf5973 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -386,6 +386,20 @@ int tst_umount(const char *path)
 	return -1;
 }
 
+int tst_is_mounted(const char *path)
+{
+	char cmd[PATH_MAX];
+	int ret;
+
+	snprintf(cmd, sizeof(cmd), "mountpoint -q %s", path);
+	ret = tst_system(cmd);
+
+	if (ret)
+		tst_resm(TINFO, "No device is mounted at %s", path);
+
+	return !ret;
+}
+
 int find_stat_file(const char *dev, char *path, size_t path_len)
 {
 	const char *devname = strrchr(dev, '/') + 1;
diff --git a/testcases/kernel/syscalls/fsmount/fsmount01.c b/testcases/kernel/syscalls/fsmount/fsmount01.c
index 83185b48aedd..8e29a1537334 100644
--- a/testcases/kernel/syscalls/fsmount/fsmount01.c
+++ b/testcases/kernel/syscalls/fsmount/fsmount01.c
@@ -12,30 +12,10 @@
 #include "tst_test.h"
 #include "lapi/fcntl.h"
 #include "lapi/fsmount.h"
-#include "tst_safe_stdio.h"
 
-#define LINELENGTH 256
 #define MNTPOINT "newmount_point"
 static int sfd, mfd, is_mounted;
 
-static int ismount(char *mntpoint)
-{
-	int ret = 0;
-	FILE *file;
-	char line[LINELENGTH];
-
-	file = SAFE_FOPEN("/proc/mounts", "r");
-
-	while (fgets(line, sizeof(line), file)) {
-		if (strstr(line, mntpoint) != NULL) {
-			ret = 1;
-			break;
-		}
-	}
-	SAFE_FCLOSE(file);
-	return ret;
-}
-
 static void cleanup(void)
 {
 	if (is_mounted)
@@ -76,12 +56,9 @@ static void test_fsmount(void)
 	tst_res(TPASS, "move_mount() attached to the mount point");
 	SAFE_CLOSE(mfd);
 
-	if (ismount(MNTPOINT)) {
-		tst_res(TPASS, "device mounted");
+	if (tst_is_mounted(MNTPOINT)) {
 		SAFE_UMOUNT(MNTPOINT);
 		is_mounted = 0;
-	} else {
-		tst_res(TFAIL, "device not mounted");
 	}
 }
 
-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel()
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
  2020-02-27  5:14 ` [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper Viresh Kumar
@ 2020-02-27  5:14 ` Viresh Kumar
  2020-03-06 12:47   ` Cyril Hrubis
  2020-02-27  5:14 ` [LTP] [PATCH V5 03/10] lapi/fsmount.h: Include "lapi/fcntl.h" Viresh Kumar
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

Add a helper to check if the fsmount() related syscalls are supported by
the kernel or not.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Acked-by: Li Wang <liwang@redhat.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/lapi/fsmount.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/lapi/fsmount.h b/include/lapi/fsmount.h
index 87f2f229c371..a6a24904e66d 100644
--- a/include/lapi/fsmount.h
+++ b/include/lapi/fsmount.h
@@ -130,5 +130,14 @@ enum fsconfig_command {
 
 #endif /* OPEN_TREE_CLONE */
 
+void fsopen_supported_by_kernel(void)
+{
+	if ((tst_kvercmp(5, 2, 0)) < 0) {
+		/* Check if the syscall is backported on an older kernel */
+		TEST(syscall(__NR_fsopen, NULL, 0));
+		if (TST_RET == -1 && TST_ERR == ENOSYS)
+			tst_brk(TCONF, "Test not supported on kernel version < v5.2");
+	}
+}
 
 #endif /* FSMOUNT_H__ */
-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 03/10] lapi/fsmount.h: Include "lapi/fcntl.h"
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
  2020-02-27  5:14 ` [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper Viresh Kumar
  2020-02-27  5:14 ` [LTP] [PATCH V5 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel() Viresh Kumar
@ 2020-02-27  5:14 ` Viresh Kumar
  2020-02-27  5:14 ` [LTP] [PATCH V5 04/10] syscalls/fsopen: New tests Viresh Kumar
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

All the files that include (and that will include it in future) are most
probably going to need the definitions from "lapi/fcntl.h", include it
directly instead of <fcntl.h>, which will break it for old RHL distros.

Acked-by: Petr Vorel <pvorel@suse.cz>
Acked-by: Li Wang <liwang@redhat.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/lapi/fsmount.h                        | 2 +-
 testcases/kernel/syscalls/fsmount/fsmount01.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/lapi/fsmount.h b/include/lapi/fsmount.h
index a6a24904e66d..5e5eaca7e6ff 100644
--- a/include/lapi/fsmount.h
+++ b/include/lapi/fsmount.h
@@ -7,12 +7,12 @@
 #ifndef FSMOUNT_H__
 #define FSMOUNT_H__
 
-#include <fcntl.h>
 #include <sys/mount.h>
 #include <sys/syscall.h>
 #include <sys/types.h>
 
 #include "config.h"
+#include "lapi/fcntl.h"
 #include "lapi/syscalls.h"
 
 #ifndef HAVE_FSOPEN
diff --git a/testcases/kernel/syscalls/fsmount/fsmount01.c b/testcases/kernel/syscalls/fsmount/fsmount01.c
index 8e29a1537334..514d3b0b38f8 100644
--- a/testcases/kernel/syscalls/fsmount/fsmount01.c
+++ b/testcases/kernel/syscalls/fsmount/fsmount01.c
@@ -10,7 +10,6 @@
 #include <sys/mount.h>
 
 #include "tst_test.h"
-#include "lapi/fcntl.h"
 #include "lapi/fsmount.h"
 
 #define MNTPOINT "newmount_point"
-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 04/10] syscalls/fsopen: New tests
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (2 preceding siblings ...)
  2020-02-27  5:14 ` [LTP] [PATCH V5 03/10] lapi/fsmount.h: Include "lapi/fcntl.h" Viresh Kumar
@ 2020-02-27  5:14 ` Viresh Kumar
  2020-03-06 13:10   ` Cyril Hrubis
  2020-02-27  5:14 ` [LTP] [PATCH V5 05/10] syscalls/fsconfig: " Viresh Kumar
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

Add tests to check working of fsopen() syscall.

Acked-by: Li Wang <liwang@redhat.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 runtest/syscalls                            |  3 +
 testcases/kernel/syscalls/fsopen/.gitignore |  2 +
 testcases/kernel/syscalls/fsopen/Makefile   |  6 ++
 testcases/kernel/syscalls/fsopen/fsopen01.c | 80 +++++++++++++++++++++
 testcases/kernel/syscalls/fsopen/fsopen02.c | 58 +++++++++++++++
 5 files changed, 149 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fsopen/.gitignore
 create mode 100644 testcases/kernel/syscalls/fsopen/Makefile
 create mode 100644 testcases/kernel/syscalls/fsopen/fsopen01.c
 create mode 100644 testcases/kernel/syscalls/fsopen/fsopen02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index f51456b8fb53..1f21cc55bf2d 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -343,6 +343,9 @@ fremovexattr02 fremovexattr02
 
 fsmount01 fsmount01
 
+fsopen01 fsopen01
+fsopen02 fsopen02
+
 fstat02 fstat02
 fstat02_64 fstat02_64
 fstat03 fstat03
diff --git a/testcases/kernel/syscalls/fsopen/.gitignore b/testcases/kernel/syscalls/fsopen/.gitignore
new file mode 100644
index 000000000000..5da868621883
--- /dev/null
+++ b/testcases/kernel/syscalls/fsopen/.gitignore
@@ -0,0 +1,2 @@
+/fsopen01
+/fsopen02
diff --git a/testcases/kernel/syscalls/fsopen/Makefile b/testcases/kernel/syscalls/fsopen/Makefile
new file mode 100644
index 000000000000..5ea7d67db123
--- /dev/null
+++ b/testcases/kernel/syscalls/fsopen/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/fsopen/fsopen01.c b/testcases/kernel/syscalls/fsopen/fsopen01.c
new file mode 100644
index 000000000000..abf5f15c4721
--- /dev/null
+++ b/testcases/kernel/syscalls/fsopen/fsopen01.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic fsopen() test which tries to configure and mount the filesystem as
+ * well.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+
+#define MNTPOINT	"mntpoint"
+
+#define TCASE_ENTRY(_flags)	{.name = "Flag " #_flags, .flags = _flags}
+
+static struct tcase {
+	char *name;
+	unsigned int flags;
+} tcases[] = {
+	TCASE_ENTRY(0),
+	TCASE_ENTRY(FSOPEN_CLOEXEC),
+};
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+	int fd, fsmfd;
+
+	TEST(fd = fsopen(tst_device->fs_type, tc->flags));
+	if (fd == -1) {
+		tst_res(TFAIL | TERRNO, "fsopen() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		goto out;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		goto out;
+	}
+
+	TEST(fsmfd = fsmount(fd, 0, 0));
+	if (fsmfd == -1) {
+		tst_res(TFAIL | TERRNO, "fsmount() failed");
+		goto out;
+	}
+
+	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
+			MOVE_MOUNT_F_EMPTY_PATH));
+
+	SAFE_CLOSE(fsmfd);
+
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "move_mount() failed");
+		goto out;
+	}
+
+	if (tst_is_mounted(MNTPOINT))
+		tst_res(TPASS, "%s: fsopen() passed", tc->name);
+
+	SAFE_UMOUNT(MNTPOINT);
+
+out:
+	SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = fsopen_supported_by_kernel,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
+};
diff --git a/testcases/kernel/syscalls/fsopen/fsopen02.c b/testcases/kernel/syscalls/fsopen/fsopen02.c
new file mode 100644
index 000000000000..3f287bf2962b
--- /dev/null
+++ b/testcases/kernel/syscalls/fsopen/fsopen02.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic fsopen() failure tests.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+
+const char *invalid_fs = "invalid";
+const char *valid_fs;
+
+static struct tcase {
+	char *name;
+	const char **fs;
+	unsigned int flags;
+	int exp_errno;
+} tcases[] = {
+	{"invalid-fs", &invalid_fs, 0, ENODEV},
+	{"invalid-flags", &valid_fs, 0x10, EINVAL},
+};
+
+static void setup(void)
+{
+	fsopen_supported_by_kernel();
+
+	valid_fs = tst_device->fs_type;
+}
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	TEST(fsopen(*tc->fs, tc->flags));
+
+	if (TST_RET != -1) {
+		SAFE_CLOSE(TST_RET);
+		tst_res(TFAIL, "%s: fsopen() succeeded unexpectedly (index: %d)",
+			tc->name, n);
+		return;
+	}
+
+	if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: fsopen() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+		return;
+	}
+
+	tst_res(TPASS | TTERRNO, "%s: fsopen() failed as expected", tc->name);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = setup,
+	.needs_root = 1,
+	.needs_device = 1,
+};
-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 05/10] syscalls/fsconfig: New tests
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (3 preceding siblings ...)
  2020-02-27  5:14 ` [LTP] [PATCH V5 04/10] syscalls/fsopen: New tests Viresh Kumar
@ 2020-02-27  5:14 ` Viresh Kumar
  2020-02-28 16:01   ` Petr Vorel
  2020-02-27  5:14 ` [LTP] [PATCH V5 06/10] syscalls/fsmount: Improve fsmount01 test Viresh Kumar
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

Add tests to check working of fsconfig() syscall.

Acked-by: Li Wang <liwang@redhat.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 runtest/syscalls                              |   3 +
 testcases/kernel/syscalls/fsconfig/.gitignore |   2 +
 testcases/kernel/syscalls/fsconfig/Makefile   |   6 ++
 .../kernel/syscalls/fsconfig/fsconfig01.c     | 101 ++++++++++++++++++
 .../kernel/syscalls/fsconfig/fsconfig02.c     |  99 +++++++++++++++++
 5 files changed, 211 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fsconfig/.gitignore
 create mode 100644 testcases/kernel/syscalls/fsconfig/Makefile
 create mode 100644 testcases/kernel/syscalls/fsconfig/fsconfig01.c
 create mode 100644 testcases/kernel/syscalls/fsconfig/fsconfig02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 1f21cc55bf2d..97c0fea2fe57 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -341,6 +341,9 @@ fpathconf01 fpathconf01
 fremovexattr01 fremovexattr01
 fremovexattr02 fremovexattr02
 
+fsconfig01 fsconfig01
+fsconfig02 fsconfig02
+
 fsmount01 fsmount01
 
 fsopen01 fsopen01
diff --git a/testcases/kernel/syscalls/fsconfig/.gitignore b/testcases/kernel/syscalls/fsconfig/.gitignore
new file mode 100644
index 000000000000..2bc54b82751b
--- /dev/null
+++ b/testcases/kernel/syscalls/fsconfig/.gitignore
@@ -0,0 +1,2 @@
+/fsconfig01
+/fsconfig02
diff --git a/testcases/kernel/syscalls/fsconfig/Makefile b/testcases/kernel/syscalls/fsconfig/Makefile
new file mode 100644
index 000000000000..5ea7d67db123
--- /dev/null
+++ b/testcases/kernel/syscalls/fsconfig/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/fsconfig/fsconfig01.c b/testcases/kernel/syscalls/fsconfig/fsconfig01.c
new file mode 100644
index 000000000000..e32b23e9586b
--- /dev/null
+++ b/testcases/kernel/syscalls/fsconfig/fsconfig01.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic fsconfig() test which tries to configure and mount the filesystem as
+ * well.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+
+#define MNTPOINT	"mntpoint"
+
+static void run(void)
+{
+	int fd, fsmfd;
+
+	TEST(fd = fsopen(tst_device->fs_type, 0));
+	if (fd == -1)
+		tst_brk(TBROK | TERRNO, "fsopen() failed");
+
+	TEST(fsconfig(fd, FSCONFIG_SET_FLAG, "rw", NULL, 0));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		goto out;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		goto out;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_PATH, "sync", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		if (TST_ERR == EOPNOTSUPP) {
+			tst_res(TCONF, "fsconfig(): FSCONFIG_SET_PATH not supported");
+		} else {
+			tst_res(TFAIL | TERRNO, "fsconfig() failed");
+			goto out;
+		}
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_PATH_EMPTY, "sync", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		if (TST_ERR == EOPNOTSUPP) {
+			tst_res(TCONF, "fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported");
+		} else {
+			tst_res(TFAIL | TERRNO, "fsconfig() failed");
+			goto out;
+		}
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_FD, "sync", NULL, 0));
+	if (TST_RET == -1) {
+		if (TST_ERR == EOPNOTSUPP) {
+			tst_res(TCONF, "fsconfig(): FSCONFIG_SET_FD not supported");
+		} else {
+			tst_res(TFAIL | TERRNO, "fsconfig() failed");
+			goto out;
+		}
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		goto out;
+	}
+
+	TEST(fsmfd = fsmount(fd, 0, 0));
+	if (fsmfd == -1) {
+		tst_res(TBROK | TERRNO, "fsmount() failed");
+		goto out;
+	}
+
+	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
+			MOVE_MOUNT_F_EMPTY_PATH));
+	SAFE_CLOSE(fsmfd);
+
+	if (TST_RET == -1) {
+		tst_res(TBROK | TERRNO, "move_mount() failed");
+		goto out;
+	}
+
+	if (tst_is_mounted(MNTPOINT))
+		tst_res(TPASS, "fsconfig() passed");
+
+	SAFE_UMOUNT(MNTPOINT);
+
+out:
+	SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = fsopen_supported_by_kernel,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
+};
diff --git a/testcases/kernel/syscalls/fsconfig/fsconfig02.c b/testcases/kernel/syscalls/fsconfig/fsconfig02.c
new file mode 100644
index 000000000000..e6e7b4b9f1de
--- /dev/null
+++ b/testcases/kernel/syscalls/fsconfig/fsconfig02.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic fsconfig() failure tests.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+
+static int fd = -1, temp_fd = -1, invalid_fd = -1;
+static int aux_0 = 0, aux_1 = 1, aux_fdcwd = AT_FDCWD, aux_minus1 = -1;
+
+static struct tcase {
+	char *name;
+	int *fd;
+	unsigned int cmd;
+	const char *key;
+	const void *value;
+	int *aux;
+	int exp_errno;
+} tcases[] = {
+	{"invalid-fd", &invalid_fd, FSCONFIG_SET_FLAG, "user_xattr", NULL, &aux_0, EINVAL},
+	{"invalid-cmd", &fd, 100, "rw", NULL, &aux_0, EOPNOTSUPP},
+	{"set-flag-key", &fd, FSCONFIG_SET_FLAG, NULL, NULL, &aux_0, EINVAL},
+	{"set-flag-value", &fd, FSCONFIG_SET_FLAG, "rw", "foo", &aux_0, EINVAL},
+	{"set-flag-aux", &fd, FSCONFIG_SET_FLAG, "rw", NULL, &aux_1, EINVAL},
+	{"set-string-key", &fd, FSCONFIG_SET_STRING, NULL, "#grand.central.org:root.cell.", &aux_0, EINVAL},
+	{"set-string-value", &fd, FSCONFIG_SET_STRING, "source", NULL, &aux_0, EINVAL},
+	{"set-string-aux", &fd, FSCONFIG_SET_STRING, "source", "#grand.central.org:root.cell.", &aux_1, EINVAL},
+	{"set-binary-key", &fd, FSCONFIG_SET_BINARY, NULL, "foo", &aux_1, EINVAL},
+	{"set-binary-value", &fd, FSCONFIG_SET_BINARY, "sync", NULL, &aux_1, EINVAL},
+	{"set-binary-aux", &fd, FSCONFIG_SET_BINARY, "sync", "foo", &aux_0, EINVAL},
+	{"set-path-key", &fd, FSCONFIG_SET_PATH, NULL, "/dev/sda1", &aux_fdcwd, EINVAL},
+	{"set-path-value", &fd, FSCONFIG_SET_PATH, "sync", NULL, &aux_fdcwd, EINVAL},
+	{"set-path-aux", &fd, FSCONFIG_SET_PATH, "sync", "/dev/sda1", &aux_minus1, EINVAL},
+	{"set-path-empty-key", &fd, FSCONFIG_SET_PATH_EMPTY, NULL, "/dev/foo", &aux_fdcwd, EINVAL},
+	{"set-path-empty-value", &fd, FSCONFIG_SET_PATH_EMPTY, "sync", NULL, &aux_fdcwd, EINVAL},
+	{"set-path-empty-aux", &fd, FSCONFIG_SET_PATH_EMPTY, "sync", "/dev/foo", &aux_minus1, EINVAL},
+	{"set-fd-key", &fd, FSCONFIG_SET_FD, NULL, NULL, &temp_fd, EINVAL},
+	{"set-fd-value", &fd, FSCONFIG_SET_FD, "sync", "foo", &temp_fd, EINVAL},
+	{"set-fd-aux", &fd, FSCONFIG_SET_FD, "sync", NULL, &aux_minus1, EINVAL},
+	{"cmd-create-key", &fd, FSCONFIG_CMD_CREATE, "foo", NULL, &aux_0, EINVAL},
+	{"cmd-create-value", &fd, FSCONFIG_CMD_CREATE, NULL, "foo", &aux_0, EINVAL},
+	{"cmd-create-aux", &fd, FSCONFIG_CMD_CREATE, NULL, NULL, &aux_1, EINVAL},
+	{"cmd-reconfigure-key", &fd, FSCONFIG_CMD_RECONFIGURE, "foo", NULL, &aux_0, EINVAL},
+	{"cmd-reconfigure-value", &fd, FSCONFIG_CMD_RECONFIGURE, NULL, "foo", &aux_0, EINVAL},
+	{"cmd-reconfigure-aux", &fd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, &aux_1, EINVAL},
+};
+
+static void setup(void)
+{
+	fsopen_supported_by_kernel();
+
+	TEST(fd = fsopen(tst_device->fs_type, 0));
+	if (fd == -1)
+		tst_brk(TBROK | TERRNO, "fsopen() failed");
+
+	temp_fd = open("testfile", O_RDWR | O_CREAT, 01444);
+	if (temp_fd == -1)
+		tst_res(TBROK, "Can't obtain temp_fd, open() failed");
+}
+
+static void cleanup(void)
+{
+	if (temp_fd != -1)
+		SAFE_CLOSE(temp_fd);
+	if (fd != -1)
+		SAFE_CLOSE(fd);
+}
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	TEST(fsconfig(*tc->fd, tc->cmd, tc->key, tc->value, *tc->aux));
+
+	if (TST_RET != -1) {
+		tst_res(TFAIL, "%s: fsconfig() succeeded unexpectedly (index: %d)",
+			tc->name, n);
+		return;
+	}
+
+	if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: fsconfig() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+		return;
+	}
+
+	tst_res(TPASS | TTERRNO, "%s: fsconfig() failed as expected", tc->name);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_device = 1,
+};
-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 06/10] syscalls/fsmount: Improve fsmount01 test
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (4 preceding siblings ...)
  2020-02-27  5:14 ` [LTP] [PATCH V5 05/10] syscalls/fsconfig: " Viresh Kumar
@ 2020-02-27  5:14 ` Viresh Kumar
  2020-02-27  5:14 ` [LTP] [PATCH V5 07/10] syscalls/fsmount: Add failure tests Viresh Kumar
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

This patch updates the fsmount01.c file to make it look similar to all
other fsmount related syscall tests and here is the list of all changes:

- Test all fsmount flags and mount attributes
- Remove extra PASS messages as all we want to test here is fsmount()
  and not other syscalls.
- On the same lines, print TFAIL for fsmount() syscall and TBROK for
  other calls.
- close sfd on failures
- Make the file look similar to other fsmount related tests
- General cleanup

Acked-by: Li Wang <liwang@redhat.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 testcases/kernel/syscalls/fsmount/fsmount01.c | 103 +++++++++++-------
 1 file changed, 65 insertions(+), 38 deletions(-)

diff --git a/testcases/kernel/syscalls/fsmount/fsmount01.c b/testcases/kernel/syscalls/fsmount/fsmount01.c
index 514d3b0b38f8..8c3538bf8c84 100644
--- a/testcases/kernel/syscalls/fsmount/fsmount01.c
+++ b/testcases/kernel/syscalls/fsmount/fsmount01.c
@@ -3,67 +3,94 @@
  * Copyright (C) 2020 Red Hat, Inc.  All rights reserved.
  * Author: Zorro Lang <zlang@redhat.com>
  *
- * Use new mount API from v5.2 (fsopen(), fsconfig(), fsmount(), move_mount())
- * to mount a filesystem without any specified mount options.
+ * Basic fsmount() test.
  */
 
-#include <sys/mount.h>
-
 #include "tst_test.h"
 #include "lapi/fsmount.h"
 
-#define MNTPOINT "newmount_point"
-static int sfd, mfd, is_mounted;
+#define MNTPOINT	"mntpoint"
 
-static void cleanup(void)
-{
-	if (is_mounted)
-		SAFE_UMOUNT(MNTPOINT);
-}
+#define TCASE_ENTRY(_flags, _attrs)	{.name = "Flag " #_flags ", Attr " #_attrs, .flags = _flags, .attrs = _attrs}
 
-static void test_fsmount(void)
+static struct tcase {
+	char *name;
+	unsigned int flags;
+	unsigned int attrs;
+} tcases[] = {
+	TCASE_ENTRY(0, MOUNT_ATTR_RDONLY),
+	TCASE_ENTRY(0, MOUNT_ATTR_NOSUID),
+	TCASE_ENTRY(0, MOUNT_ATTR_NODEV),
+	TCASE_ENTRY(0, MOUNT_ATTR_NOEXEC),
+	TCASE_ENTRY(0, MOUNT_ATTR_RELATIME),
+	TCASE_ENTRY(0, MOUNT_ATTR_NOATIME),
+	TCASE_ENTRY(0, MOUNT_ATTR_STRICTATIME),
+	TCASE_ENTRY(0, MOUNT_ATTR_NODIRATIME),
+	TCASE_ENTRY(FSMOUNT_CLOEXEC, MOUNT_ATTR_RDONLY),
+	TCASE_ENTRY(FSMOUNT_CLOEXEC, MOUNT_ATTR_NOSUID),
+	TCASE_ENTRY(FSMOUNT_CLOEXEC, MOUNT_ATTR_NODEV),
+	TCASE_ENTRY(FSMOUNT_CLOEXEC, MOUNT_ATTR_NOEXEC),
+	TCASE_ENTRY(FSMOUNT_CLOEXEC, MOUNT_ATTR_RELATIME),
+	TCASE_ENTRY(FSMOUNT_CLOEXEC, MOUNT_ATTR_NOATIME),
+	TCASE_ENTRY(FSMOUNT_CLOEXEC, MOUNT_ATTR_STRICTATIME),
+	TCASE_ENTRY(FSMOUNT_CLOEXEC, MOUNT_ATTR_NODIRATIME),
+};
+
+static void run(unsigned int n)
 {
-	TEST(fsopen(tst_device->fs_type, FSOPEN_CLOEXEC));
-	if (TST_RET < 0)
-		tst_brk(TBROK | TTERRNO, "fsopen() on %s failed", tst_device->fs_type);
-	sfd = TST_RET;
-	tst_res(TPASS, "fsopen() on %s", tst_device->fs_type);
+	struct tcase *tc = &tcases[n];
+	int sfd, mfd;
+
+	TEST(sfd = fsopen(tst_device->fs_type, FSOPEN_CLOEXEC));
+	if (sfd == -1) {
+		tst_res(TFAIL | TTERRNO, "fsopen() on %s failed",
+			tst_device->fs_type);
+		return;
+	}
 
 	TEST(fsconfig(sfd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
-	if (TST_RET < 0)
-		tst_brk(TBROK | TTERRNO,
+	if (TST_RET == -1) {
+		SAFE_CLOSE(sfd);
+		tst_res(TFAIL | TTERRNO,
 			"fsconfig() failed to set source to %s", tst_device->dev);
-	tst_res(TPASS, "fsconfig() set source to %s", tst_device->dev);
-
+		return;
+	}
 
 	TEST(fsconfig(sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
-	if (TST_RET < 0)
-		tst_brk(TBROK | TTERRNO, "fsconfig() created superblock");
-	tst_res(TPASS, "fsconfig() created superblock");
+	if (TST_RET == -1) {
+		SAFE_CLOSE(sfd);
+		tst_res(TFAIL | TTERRNO, "fsconfig() created superblock");
+		return;
+	}
 
-	TEST(fsmount(sfd, FSMOUNT_CLOEXEC, 0));
-	if (TST_RET < 0)
-		tst_brk(TBROK | TTERRNO, "fsmount() failed to create a mount object");
-	mfd = TST_RET;
-	tst_res(TPASS, "fsmount() created a mount object");
+	TEST(mfd = fsmount(sfd, tc->flags, tc->attrs));
 	SAFE_CLOSE(sfd);
 
+	if (mfd == -1) {
+		tst_res(TFAIL | TTERRNO,
+			"fsmount() failed to create a mount object");
+		return;
+	}
+
 	TEST(move_mount(mfd, "", AT_FDCWD, MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH));
-	if (TST_RET < 0)
-		tst_brk(TBROK | TTERRNO, "move_mount() failed to attach to the mount point");
-	is_mounted = 1;
-	tst_res(TPASS, "move_mount() attached to the mount point");
 	SAFE_CLOSE(mfd);
 
-	if (tst_is_mounted(MNTPOINT)) {
-		SAFE_UMOUNT(MNTPOINT);
-		is_mounted = 0;
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TTERRNO,
+			"move_mount() failed to attach to the mount point");
+		return;
 	}
+
+	if (tst_is_mounted(MNTPOINT))
+		tst_res(TPASS, "%s: fsmount() passed", tc->name);
+
+	SAFE_UMOUNT(MNTPOINT);
 }
 
 static struct tst_test test = {
-	.test_all = test_fsmount,
-	.cleanup = cleanup,
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = fsopen_supported_by_kernel,
 	.needs_root = 1,
 	.mntpoint = MNTPOINT,
 	.format_device = 1,
-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 07/10] syscalls/fsmount: Add failure tests
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (5 preceding siblings ...)
  2020-02-27  5:14 ` [LTP] [PATCH V5 06/10] syscalls/fsmount: Improve fsmount01 test Viresh Kumar
@ 2020-02-27  5:14 ` Viresh Kumar
  2020-02-27  5:14 ` [LTP] [PATCH V5 08/10] syscalls/move_mount: New tests Viresh Kumar
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

This adds fsmount02.c tests to verify all the errors returned on
failures.

Acked-by: Li Wang <liwang@redhat.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 runtest/syscalls                              |  1 +
 testcases/kernel/syscalls/fsmount/.gitignore  |  1 +
 testcases/kernel/syscalls/fsmount/fsmount02.c | 80 +++++++++++++++++++
 3 files changed, 82 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fsmount/fsmount02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 97c0fea2fe57..1ac8c84d2567 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -345,6 +345,7 @@ fsconfig01 fsconfig01
 fsconfig02 fsconfig02
 
 fsmount01 fsmount01
+fsmount02 fsmount02
 
 fsopen01 fsopen01
 fsopen02 fsopen02
diff --git a/testcases/kernel/syscalls/fsmount/.gitignore b/testcases/kernel/syscalls/fsmount/.gitignore
index e2f01ea17a40..7b77c5e33ee6 100644
--- a/testcases/kernel/syscalls/fsmount/.gitignore
+++ b/testcases/kernel/syscalls/fsmount/.gitignore
@@ -1 +1,2 @@
 /fsmount01
+/fsmount02
diff --git a/testcases/kernel/syscalls/fsmount/fsmount02.c b/testcases/kernel/syscalls/fsmount/fsmount02.c
new file mode 100644
index 000000000000..e3419200961c
--- /dev/null
+++ b/testcases/kernel/syscalls/fsmount/fsmount02.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic fsmount() failure tests.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+
+int fd = -1, invalid_fd = -1;
+
+#define MNTPOINT	"mntpoint"
+
+static struct tcase {
+	char *name;
+	int *fd;
+	unsigned int flags;
+	unsigned int mount_attrs;
+	int exp_errno;
+} tcases[] = {
+	{"invalid-fd", &invalid_fd, FSMOUNT_CLOEXEC, 0, EBADF},
+	{"invalid-flags", &fd, 0x02, 0, EINVAL},
+	{"invalid-attrs", &fd, FSMOUNT_CLOEXEC, 0x100, EINVAL},
+};
+
+static void cleanup(void)
+{
+	if (fd != -1)
+		SAFE_CLOSE(fd);
+}
+
+static void setup(void)
+{
+	fsopen_supported_by_kernel();
+
+	TEST(fd = fsopen(tst_device->fs_type, 0));
+	if (fd == -1)
+		tst_brk(TBROK | TERRNO, "fsopen() failed");
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1)
+		tst_brk(TBROK | TERRNO, "fsconfig() failed");
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1)
+		tst_brk(TBROK | TERRNO, "fsconfig() failed");
+}
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	TEST(fsmount(*tc->fd, tc->flags, tc->mount_attrs));
+	if (TST_RET != -1) {
+		SAFE_CLOSE(TST_RET);
+		tst_res(TFAIL, "%s: fsmount() succeeded unexpectedly (index: %d)",
+			tc->name, n);
+		return;
+	}
+
+	if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: fsmount() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+		return;
+	}
+
+	tst_res(TPASS | TTERRNO, "%s: fsmount() failed as expected", tc->name);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.format_device = 1,
+	.all_filesystems = 1,
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
+};
-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 08/10] syscalls/move_mount: New tests
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (6 preceding siblings ...)
  2020-02-27  5:14 ` [LTP] [PATCH V5 07/10] syscalls/fsmount: Add failure tests Viresh Kumar
@ 2020-02-27  5:14 ` Viresh Kumar
  2020-02-27  5:14 ` [LTP] [PATCH V5 09/10] syscalls/fspick: " Viresh Kumar
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

Add tests to check working of move_mount() syscall.

Acked-by: Li Wang <liwang@redhat.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 runtest/syscalls                              |  3 +
 .../kernel/syscalls/move_mount/.gitignore     |  2 +
 testcases/kernel/syscalls/move_mount/Makefile |  6 ++
 .../kernel/syscalls/move_mount/move_mount01.c | 83 +++++++++++++++++
 .../kernel/syscalls/move_mount/move_mount02.c | 92 +++++++++++++++++++
 5 files changed, 186 insertions(+)
 create mode 100644 testcases/kernel/syscalls/move_mount/.gitignore
 create mode 100644 testcases/kernel/syscalls/move_mount/Makefile
 create mode 100644 testcases/kernel/syscalls/move_mount/move_mount01.c
 create mode 100644 testcases/kernel/syscalls/move_mount/move_mount02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 1ac8c84d2567..32b3f36e4dcc 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -731,6 +731,9 @@ mount04 mount04
 mount05 mount05
 mount06 mount06
 
+move_mount01 move_mount01
+move_mount02 move_mount02
+
 move_pages01 move_pages01
 move_pages02 move_pages02
 move_pages03 move_pages03
diff --git a/testcases/kernel/syscalls/move_mount/.gitignore b/testcases/kernel/syscalls/move_mount/.gitignore
new file mode 100644
index 000000000000..83ae40145dff
--- /dev/null
+++ b/testcases/kernel/syscalls/move_mount/.gitignore
@@ -0,0 +1,2 @@
+/move_mount01
+/move_mount02
diff --git a/testcases/kernel/syscalls/move_mount/Makefile b/testcases/kernel/syscalls/move_mount/Makefile
new file mode 100644
index 000000000000..5ea7d67db123
--- /dev/null
+++ b/testcases/kernel/syscalls/move_mount/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/move_mount/move_mount01.c b/testcases/kernel/syscalls/move_mount/move_mount01.c
new file mode 100644
index 000000000000..7f561718764a
--- /dev/null
+++ b/testcases/kernel/syscalls/move_mount/move_mount01.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic move_mount() test.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+
+#define MNTPOINT	"mntpoint"
+
+#define TCASE_ENTRY(_flags)	{.name = "Flag " #_flags, .flags = _flags}
+
+static struct tcase {
+	char *name;
+	unsigned int flags;
+} tcases[] = {
+	TCASE_ENTRY(MOVE_MOUNT_F_SYMLINKS),
+	TCASE_ENTRY(MOVE_MOUNT_F_AUTOMOUNTS),
+	TCASE_ENTRY(MOVE_MOUNT_F_EMPTY_PATH),
+	TCASE_ENTRY(MOVE_MOUNT_T_SYMLINKS),
+	TCASE_ENTRY(MOVE_MOUNT_T_AUTOMOUNTS),
+	TCASE_ENTRY(MOVE_MOUNT_T_EMPTY_PATH),
+};
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+	int fsmfd, fd;
+
+	TEST(fd = fsopen(tst_device->fs_type, 0));
+	if (fd == -1) {
+		tst_res(TFAIL | TERRNO, "fsopen() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsmfd = fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (fsmfd == -1) {
+		tst_res(TFAIL | TERRNO, "fsmount() failed");
+		return;
+	}
+
+	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
+			tc->flags | MOVE_MOUNT_F_EMPTY_PATH));
+	SAFE_CLOSE(fsmfd);
+
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "move_mount() failed");
+		return;
+	}
+
+	if (tst_is_mounted(MNTPOINT))
+		tst_res(TPASS, "%s: move_mount() passed", tc->name);
+
+	SAFE_UMOUNT(MNTPOINT);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = fsopen_supported_by_kernel,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
+};
diff --git a/testcases/kernel/syscalls/move_mount/move_mount02.c b/testcases/kernel/syscalls/move_mount/move_mount02.c
new file mode 100644
index 000000000000..dfb48a1b3dc6
--- /dev/null
+++ b/testcases/kernel/syscalls/move_mount/move_mount02.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic move_mount() failure tests.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+
+#define MNTPOINT	"mntpoint"
+
+int invalid_fd = -1, fsmfd;
+
+static struct tcase {
+	char *name;
+	int *from_dirfd;
+	const char *from_pathname;
+	int to_dirfd;
+	const char *to_pathname;
+	unsigned int flags;
+	int exp_errno;
+} tcases[] = {
+	{"invalid-from-fd", &invalid_fd, "", AT_FDCWD, MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH, EBADF},
+	{"invalid-from-path", &fsmfd, "invalid", AT_FDCWD, MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH, ENOENT},
+	{"invalid-to-fd", &fsmfd, "", -1, MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH, EBADF},
+	{"invalid-to-path", &fsmfd, "", AT_FDCWD, "invalid", MOVE_MOUNT_F_EMPTY_PATH, ENOENT},
+	{"invalid-flags", &fsmfd, "", AT_FDCWD, MNTPOINT, 0x08, EINVAL},
+};
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+	int fd;
+
+	TEST(fd = fsopen(tst_device->fs_type, 0));
+	if (fd == -1) {
+		tst_res(TFAIL | TERRNO, "fsopen() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsmfd = fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (fsmfd == -1) {
+		tst_res(TFAIL | TERRNO, "fsmount() failed");
+		return;
+	}
+
+	TEST(move_mount(*tc->from_dirfd, tc->from_pathname, tc->to_dirfd,
+			tc->to_pathname, tc->flags));
+	SAFE_CLOSE(fsmfd);
+
+	if (TST_RET != -1) {
+		SAFE_UMOUNT(MNTPOINT);
+		tst_res(TFAIL, "%s: move_mount() succeeded unexpectedly (index: %d)",
+			tc->name, n);
+		return;
+	}
+
+	if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: move_mount() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+		return;
+	}
+
+	tst_res(TPASS | TTERRNO, "%s: move_mount() failed as expected", tc->name);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = fsopen_supported_by_kernel,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
+};
-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 09/10] syscalls/fspick: New tests
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (7 preceding siblings ...)
  2020-02-27  5:14 ` [LTP] [PATCH V5 08/10] syscalls/move_mount: New tests Viresh Kumar
@ 2020-02-27  5:14 ` Viresh Kumar
  2020-02-27  5:14 ` [LTP] [PATCH V5 10/10] syscalls/open_tree: " Viresh Kumar
  2020-03-06 13:18 ` [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Cyril Hrubis
  10 siblings, 0 replies; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

Add tests to check working of fspick() syscall.

Acked-by: Li Wang <liwang@redhat.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 runtest/syscalls                            |  3 +
 testcases/kernel/syscalls/fspick/.gitignore |  2 +
 testcases/kernel/syscalls/fspick/Makefile   |  6 ++
 testcases/kernel/syscalls/fspick/fspick.h   | 60 ++++++++++++++++++++
 testcases/kernel/syscalls/fspick/fspick01.c | 62 +++++++++++++++++++++
 testcases/kernel/syscalls/fspick/fspick02.c | 54 ++++++++++++++++++
 6 files changed, 187 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fspick/.gitignore
 create mode 100644 testcases/kernel/syscalls/fspick/Makefile
 create mode 100644 testcases/kernel/syscalls/fspick/fspick.h
 create mode 100644 testcases/kernel/syscalls/fspick/fspick01.c
 create mode 100644 testcases/kernel/syscalls/fspick/fspick02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 32b3f36e4dcc..ce27f835c42e 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -350,6 +350,9 @@ fsmount02 fsmount02
 fsopen01 fsopen01
 fsopen02 fsopen02
 
+fspick01 fspick01
+fspick02 fspick02
+
 fstat02 fstat02
 fstat02_64 fstat02_64
 fstat03 fstat03
diff --git a/testcases/kernel/syscalls/fspick/.gitignore b/testcases/kernel/syscalls/fspick/.gitignore
new file mode 100644
index 000000000000..a8aa61dce18b
--- /dev/null
+++ b/testcases/kernel/syscalls/fspick/.gitignore
@@ -0,0 +1,2 @@
+/fspick01
+/fspick02
diff --git a/testcases/kernel/syscalls/fspick/Makefile b/testcases/kernel/syscalls/fspick/Makefile
new file mode 100644
index 000000000000..5ea7d67db123
--- /dev/null
+++ b/testcases/kernel/syscalls/fspick/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/fspick/fspick.h b/testcases/kernel/syscalls/fspick/fspick.h
new file mode 100644
index 000000000000..62864355da06
--- /dev/null
+++ b/testcases/kernel/syscalls/fspick/fspick.h
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ */
+
+#ifndef FSPICK_H__
+#define FSPICK_H__
+
+#define MNTPOINT	"mntpoint"
+
+static int ismounted;
+
+static void cleanup(void)
+{
+	if (ismounted)
+		SAFE_UMOUNT(MNTPOINT);
+}
+
+static void setup(void)
+{
+	int fd, fsmfd;
+
+	fsopen_supported_by_kernel();
+
+	TEST(fd = fsopen(tst_device->fs_type, 0));
+	if (fd == -1)
+		tst_brk(TBROK | TERRNO, "fsopen() failed");
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_brk(TBROK | TERRNO, "fsconfig() failed");
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_brk(TBROK | TERRNO, "fsconfig() failed");
+	}
+
+	TEST(fsmfd = fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (fsmfd == -1)
+		tst_brk(TBROK | TERRNO, "fsmount() failed");
+
+	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
+			MOVE_MOUNT_F_EMPTY_PATH));
+	SAFE_CLOSE(fsmfd);
+
+	if (TST_RET == -1)
+		tst_brk(TBROK | TERRNO, "move_mount() failed");
+
+	ismounted = 1;
+
+	if (!tst_is_mounted(MNTPOINT))
+		tst_brk(TBROK | TERRNO, "device not mounted");
+}
+
+#endif /* FSPICK_H__ */
diff --git a/testcases/kernel/syscalls/fspick/fspick01.c b/testcases/kernel/syscalls/fspick/fspick01.c
new file mode 100644
index 000000000000..4e1daeaee14a
--- /dev/null
+++ b/testcases/kernel/syscalls/fspick/fspick01.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic fspick() test.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+#include "fspick.h"
+
+#define TCASE_ENTRY(_flags)	{.name = "Flag " #_flags, .flags = _flags}
+
+static struct tcase {
+	char *name;
+	unsigned int flags;
+} tcases[] = {
+	TCASE_ENTRY(FSPICK_CLOEXEC),
+	TCASE_ENTRY(FSPICK_SYMLINK_NOFOLLOW),
+	TCASE_ENTRY(FSPICK_NO_AUTOMOUNT),
+	TCASE_ENTRY(FSPICK_EMPTY_PATH),
+};
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+	int fspick_fd;
+
+	TEST(fspick_fd = fspick(AT_FDCWD, MNTPOINT, tc->flags));
+	if (fspick_fd == -1) {
+		tst_res(TFAIL | TERRNO, "fspick() failed");
+		return;
+	}
+
+	TEST(fsconfig(fspick_fd, FSCONFIG_SET_STRING, "sync", "false", 0));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		goto out;
+	}
+
+	TEST(fsconfig(fspick_fd, FSCONFIG_SET_FLAG, "ro", NULL, 0));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		goto out;
+	}
+
+	tst_res(TPASS, "%s: fspick() passed", tc->name);
+
+out:
+	SAFE_CLOSE(fspick_fd);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
+};
diff --git a/testcases/kernel/syscalls/fspick/fspick02.c b/testcases/kernel/syscalls/fspick/fspick02.c
new file mode 100644
index 000000000000..dc8f153ccc48
--- /dev/null
+++ b/testcases/kernel/syscalls/fspick/fspick02.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic fspick() failure tests.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+#include "fspick.h"
+
+static struct tcase {
+	char *name;
+	int dirfd;
+	const char *pathname;
+	unsigned int flags;
+	int exp_errno;
+} tcases[] = {
+	{"invalid-fd", -1, MNTPOINT, FSPICK_NO_AUTOMOUNT | FSPICK_CLOEXEC, EBADF},
+	{"invalid-path", AT_FDCWD, "invalid", FSPICK_NO_AUTOMOUNT | FSPICK_CLOEXEC, ENOENT},
+	{"invalid-flags", AT_FDCWD, MNTPOINT, 0x10, EINVAL},
+};
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	TEST(fspick(tc->dirfd, tc->pathname, tc->flags));
+	if (TST_RET != -1) {
+		SAFE_CLOSE(TST_RET);
+		tst_res(TFAIL, "%s: fspick() succeeded unexpectedly (index: %d)",
+			tc->name, n);
+		return;
+	}
+
+	if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: fspick() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+		return;
+	}
+
+	tst_res(TPASS | TTERRNO, "%s: fspick() failed as expected", tc->name);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
+};
-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 10/10] syscalls/open_tree: New tests
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (8 preceding siblings ...)
  2020-02-27  5:14 ` [LTP] [PATCH V5 09/10] syscalls/fspick: " Viresh Kumar
@ 2020-02-27  5:14 ` Viresh Kumar
  2020-03-06 13:18 ` [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Cyril Hrubis
  10 siblings, 0 replies; 31+ messages in thread
From: Viresh Kumar @ 2020-02-27  5:14 UTC (permalink / raw)
  To: ltp

Add tests to check working of open_tree() syscall.

Acked-by: Li Wang <liwang@redhat.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 runtest/syscalls                              |   3 +
 .../kernel/syscalls/open_tree/.gitignore      |   2 +
 testcases/kernel/syscalls/open_tree/Makefile  |   6 +
 .../kernel/syscalls/open_tree/open_tree01.c   | 108 ++++++++++++++++++
 .../kernel/syscalls/open_tree/open_tree02.c   | 105 +++++++++++++++++
 5 files changed, 224 insertions(+)
 create mode 100644 testcases/kernel/syscalls/open_tree/.gitignore
 create mode 100644 testcases/kernel/syscalls/open_tree/Makefile
 create mode 100644 testcases/kernel/syscalls/open_tree/open_tree01.c
 create mode 100644 testcases/kernel/syscalls/open_tree/open_tree02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index ce27f835c42e..cf4b55e08262 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -844,6 +844,9 @@ openat01 openat01
 openat02 openat02
 openat03 openat03
 
+open_tree01 open_tree01
+open_tree02 open_tree02
+
 mincore01 mincore01
 mincore02 mincore02
 
diff --git a/testcases/kernel/syscalls/open_tree/.gitignore b/testcases/kernel/syscalls/open_tree/.gitignore
new file mode 100644
index 000000000000..2f732b44518e
--- /dev/null
+++ b/testcases/kernel/syscalls/open_tree/.gitignore
@@ -0,0 +1,2 @@
+/open_tree01
+/open_tree02
diff --git a/testcases/kernel/syscalls/open_tree/Makefile b/testcases/kernel/syscalls/open_tree/Makefile
new file mode 100644
index 000000000000..5ea7d67db123
--- /dev/null
+++ b/testcases/kernel/syscalls/open_tree/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/open_tree/open_tree01.c b/testcases/kernel/syscalls/open_tree/open_tree01.c
new file mode 100644
index 000000000000..b83dbab9df34
--- /dev/null
+++ b/testcases/kernel/syscalls/open_tree/open_tree01.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic open_tree() test.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+
+#define MNTPOINT	"mntpoint"
+#define OT_MNTPOINT	"ot_mntpoint"
+
+#define TCASE_ENTRY(_flags)	{.name = "Flag " #_flags, .flags = _flags}
+
+static struct tcase {
+	char *name;
+	unsigned int flags;
+} tcases[] = {
+	TCASE_ENTRY(OPEN_TREE_CLONE),
+	TCASE_ENTRY(OPEN_TREE_CLOEXEC)
+};
+
+static void setup(void)
+{
+	fsopen_supported_by_kernel();
+	SAFE_MKDIR(OT_MNTPOINT, 0777);
+}
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+	int fd, fsmfd, otfd;
+
+	TEST(fd = fsopen(tst_device->fs_type, 0));
+	if (fd == -1) {
+		tst_res(TFAIL | TERRNO, "fsopen() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsmfd = fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (fsmfd == -1) {
+		tst_res(TFAIL | TERRNO, "fsmount() failed");
+		return;
+	}
+
+	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
+			MOVE_MOUNT_F_EMPTY_PATH));
+	SAFE_CLOSE(fsmfd);
+
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "move_mount() failed");
+		return;
+	}
+
+	if (!tst_is_mounted(MNTPOINT)) {
+		tst_res(TFAIL | TERRNO, "device not mounted");
+		goto out;
+	}
+
+	TEST(otfd = open_tree(AT_FDCWD, MNTPOINT, tc->flags | OPEN_TREE_CLONE));
+	if (otfd == -1) {
+		tst_res(TFAIL | TERRNO, "open_tree() failed");
+		goto out;
+	}
+
+	TEST(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT,
+			MOVE_MOUNT_F_EMPTY_PATH));
+	SAFE_CLOSE(otfd);
+
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "move_mount() failed");
+		goto out;
+	}
+
+	if (tst_is_mounted(OT_MNTPOINT))
+		tst_res(TPASS, "%s: open_tree() passed", tc->name);
+
+	SAFE_UMOUNT(OT_MNTPOINT);
+out:
+	SAFE_UMOUNT(MNTPOINT);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = setup,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
+};
diff --git a/testcases/kernel/syscalls/open_tree/open_tree02.c b/testcases/kernel/syscalls/open_tree/open_tree02.c
new file mode 100644
index 000000000000..a7e3641d9537
--- /dev/null
+++ b/testcases/kernel/syscalls/open_tree/open_tree02.c
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Basic open_tree() failure tests.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+
+#define MNTPOINT	"mntpoint"
+
+static struct tcase {
+	char *name;
+	int dirfd;
+	const char *pathname;
+	unsigned int flags;
+	int exp_errno;
+} tcases[] = {
+	{"invalid-fd", -1, MNTPOINT, OPEN_TREE_CLONE, EBADF},
+	{"invalid-path", AT_FDCWD, "invalid", OPEN_TREE_CLONE, ENOENT},
+	{"invalid-flags", AT_FDCWD, MNTPOINT, 0xFFFFFFFF, EINVAL},
+};
+
+static int ismounted;
+
+static void cleanup(void)
+{
+	if (ismounted)
+		SAFE_UMOUNT(MNTPOINT);
+}
+
+static void setup(void)
+{
+	int fd, fsmfd;
+
+	fsopen_supported_by_kernel();
+
+	TEST(fd = fsopen(tst_device->fs_type, 0));
+	if (fd == -1)
+		tst_brk(TBROK | TERRNO, "fsopen() failed");
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_brk(TBROK | TERRNO, "fsconfig() failed");
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_brk(TBROK | TERRNO, "fsconfig() failed");
+	}
+
+	TEST(fsmfd = fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (fsmfd == -1)
+		tst_brk(TBROK | TERRNO, "fsmount() failed");
+
+	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
+			MOVE_MOUNT_F_EMPTY_PATH));
+	SAFE_CLOSE(fsmfd);
+
+	if (TST_RET == -1)
+		tst_brk(TBROK | TERRNO, "move_mount() failed");
+
+	ismounted = 1;
+
+	if (!tst_is_mounted(MNTPOINT))
+		tst_brk(TBROK | TERRNO, "device not mounted");
+}
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	TEST(open_tree(tc->dirfd, tc->pathname, tc->flags));
+	if (TST_RET != -1) {
+		SAFE_CLOSE(TST_RET);
+		tst_res(TFAIL, "%s: open_tree() succeeded unexpectedly (index: %d)",
+			tc->name, n);
+		return;
+	}
+
+	if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: open_tree() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+		return;
+	}
+
+	tst_res(TPASS | TTERRNO, "%s: open_tree() failed as expected",
+		tc->name);
+}
+
+static struct tst_test test = {
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
+};
-- 
2.21.0.rc0.269.g1a574e7a288b


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 05/10] syscalls/fsconfig: New tests
  2020-02-27  5:14 ` [LTP] [PATCH V5 05/10] syscalls/fsconfig: " Viresh Kumar
@ 2020-02-28 16:01   ` Petr Vorel
  2020-03-02  8:21     ` Viresh Kumar
  2020-03-06 13:25     ` Petr Vorel
  0 siblings, 2 replies; 31+ messages in thread
From: Petr Vorel @ 2020-02-28 16:01 UTC (permalink / raw)
  To: ltp

> Add tests to check working of fsconfig() syscall.

...
> +++ b/testcases/kernel/syscalls/fsconfig/fsconfig01.c
...
> +static void run(void)
> +{
> +	int fd, fsmfd;
> +
> +	TEST(fd = fsopen(tst_device->fs_type, 0));
> +	if (fd == -1)
> +		tst_brk(TBROK | TERRNO, "fsopen() failed");
> +
> +	TEST(fsconfig(fd, FSCONFIG_SET_FLAG, "rw", NULL, 0));
> +	if (TST_RET == -1) {
> +		tst_res(TFAIL | TERRNO, "fsconfig() failed");
> +		goto out;
Because there is .test_all (single run), although which is called N times for
each filesystem (.all_filesystems = 1), but cleanup is called for each of them
it's I'll move SAFE_CLOSE(fd) to cleanup function and use here (and on the rest
of calls) return (trying to avoid goto when possible).
I'll change it before merge.

> +
> +	TEST(fsmfd = fsmount(fd, 0, 0));
> +	if (fsmfd == -1) {
> +		tst_res(TBROK | TERRNO, "fsmount() failed");
> +		goto out;
BTW This needs to be now tst_res(TFAIL) (this patchset now does not compile).

> +	TEST(fsconfig(fd, FSCONFIG_SET_FD, "sync", NULL, 0));
> +	if (TST_RET == -1) {
> +		if (TST_ERR == EOPNOTSUPP) {
> +			tst_res(TCONF, "fsconfig(): FSCONFIG_SET_FD not supported");
> +		} else {
> +			tst_res(TFAIL | TERRNO, "fsconfig() failed");
> +			goto out;
> +		}
> +	}

I get TCONF for all fsconfig01 results, while I'm using 5.5.5-1-default:

tst_test.c:1290: INFO: Testing on ext2
tst_mkfs.c:89: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
mke2fs 1.45.5 (07-Jan-2020)
tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
fsconfig01.c:92: PASS: fsconfig() passed
tst_test.c:1290: INFO: Testing on ext3
tst_mkfs.c:89: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts=''
mke2fs 1.45.5 (07-Jan-2020)
tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
fsconfig01.c:92: PASS: fsconfig() passed
tst_test.c:1290: INFO: Testing on ext4
tst_mkfs.c:89: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts=''
mke2fs 1.45.5 (07-Jan-2020)
tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
fsconfig01.c:92: PASS: fsconfig() passed
tst_test.c:1290: INFO: Testing on xfs
tst_mkfs.c:89: INFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
fsconfig01.c:92: PASS: fsconfig() passed
tst_test.c:1290: INFO: Testing on btrfs
tst_mkfs.c:89: INFO: Formatting /dev/loop0 with btrfs opts='' extra opts=''
tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
fsconfig01.c:92: PASS: fsconfig() passed
tst_test.c:1290: INFO: Testing on vfat
tst_mkfs.c:89: INFO: Formatting /dev/loop0 with vfat opts='' extra opts=''
tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
fsconfig01.c:92: PASS: fsconfig() passed

Not yet merged man page [1] (I reposted David Howells commit) there is
explanation for EOPNOTSUPP: The command given by cmd was not valid.

First, I suspected "sync" option is wrong. But looking at kernel sources it's
really not implemented:

fs/fsopen.c
	if (fc->ops == &legacy_fs_context_ops) {
		switch (cmd) {
		case FSCONFIG_SET_BINARY:
		case FSCONFIG_SET_PATH:
		case FSCONFIG_SET_PATH_EMPTY:
		case FSCONFIG_SET_FD:
			ret = -EOPNOTSUPP;
			goto out_f;
		}
	}

fs/fs_context.c
	/* TODO: Make all filesystems support this unconditionally */
	init_fs_context = fc->fs_type->init_fs_context;
	if (!init_fs_context)
		init_fs_context = legacy_init_fs_context;
...
/*
 * Initialise a legacy context for a filesystem that doesn't support
 * fs_context.
 */
static int legacy_init_fs_context(struct fs_context *fc)
{
	fc->fs_private = kzalloc(sizeof(struct legacy_fs_context), GFP_KERNEL);
	if (!fc->fs_private)
		return -ENOMEM;
	fc->ops = &legacy_fs_context_ops;
	return 0;
}

Code coming from v5.1-rc1 f3a09c92018a91ad0981146a4ac59414f814d801 introduce
fs_context methods [2]. Other patchsets here [3] shows there is some support for
fs_context in VFS. So I wonder how to achieve not end up with legacy context.

> +++ b/testcases/kernel/syscalls/fsconfig/fsconfig02.c
> +  {"set-path-key", &fd, FSCONFIG_SET_PATH, NULL, "/dev/sda1", &aux_fdcwd, EINVAL},
...
> +  {"set-path-aux", &fd, FSCONFIG_SET_PATH, "sync", "/dev/sda1", &aux_minus1, EINVAL},
/dev/sda1 is valid on some hosts, but invalid on others. Shouldn't we use
/dev/foo instead?

...
> +	temp_fd = open("testfile", O_RDWR | O_CREAT, 01444);
> +	if (temp_fd == -1)
> +		tst_res(TBROK, "Can't obtain temp_fd, open() failed");
Here needs to be now tst_brk(TBROK).
Again I'll change it before merge.

...

Kind regards,
Petr

[1] https://marc.info/?l=linux-man&m=158109737907972&w=2
[2] https://patchwork.kernel.org/patch/10820207/
[3] https://patchwork.kernel.org/project/linux-security-module/list/?series=82377

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 05/10] syscalls/fsconfig: New tests
  2020-02-28 16:01   ` Petr Vorel
@ 2020-03-02  8:21     ` Viresh Kumar
  2020-03-06 13:25     ` Petr Vorel
  1 sibling, 0 replies; 31+ messages in thread
From: Viresh Kumar @ 2020-03-02  8:21 UTC (permalink / raw)
  To: ltp

On 28-02-20, 17:01, Petr Vorel wrote:
> > +	TEST(fsmfd = fsmount(fd, 0, 0));
> > +	if (fsmfd == -1) {
> > +		tst_res(TBROK | TERRNO, "fsmount() failed");
> > +		goto out;
> BTW This needs to be now tst_res(TFAIL) (this patchset now does not compile).

For a minute I thought I failed with my testing :(

And then I looked at the recent changes and looks like compilation
broke due to the changes you have pushed recently. Right ?

> > +	TEST(fsconfig(fd, FSCONFIG_SET_FD, "sync", NULL, 0));
> > +	if (TST_RET == -1) {
> > +		if (TST_ERR == EOPNOTSUPP) {
> > +			tst_res(TCONF, "fsconfig(): FSCONFIG_SET_FD not supported");
> > +		} else {
> > +			tst_res(TFAIL | TERRNO, "fsconfig() failed");
> > +			goto out;
> > +		}
> > +	}
> 
> I get TCONF for all fsconfig01 results, while I'm using 5.5.5-1-default:

s/all/most/

I know, same here.


> tst_test.c:1290: INFO: Testing on ext2
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
> mke2fs 1.45.5 (07-Jan-2020)
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
> fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
> fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
> fsconfig01.c:92: PASS: fsconfig() passed
> tst_test.c:1290: INFO: Testing on ext3
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts=''
> mke2fs 1.45.5 (07-Jan-2020)
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
> fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
> fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
> fsconfig01.c:92: PASS: fsconfig() passed
> tst_test.c:1290: INFO: Testing on ext4
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts=''
> mke2fs 1.45.5 (07-Jan-2020)
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
> fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
> fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
> fsconfig01.c:92: PASS: fsconfig() passed
> tst_test.c:1290: INFO: Testing on xfs
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:92: PASS: fsconfig() passed

You didn't get them for xfs :)

> tst_test.c:1290: INFO: Testing on btrfs
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with btrfs opts='' extra opts=''
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
> fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
> fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
> fsconfig01.c:92: PASS: fsconfig() passed
> tst_test.c:1290: INFO: Testing on vfat
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with vfat opts='' extra opts=''
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
> fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
> fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
> fsconfig01.c:92: PASS: fsconfig() passed
> 
> Not yet merged man page [1] (I reposted David Howells commit) there is
> explanation for EOPNOTSUPP: The command given by cmd was not valid.
> 
> First, I suspected "sync" option is wrong. But looking at kernel sources it's
> really not implemented:
> 
> fs/fsopen.c
> 	if (fc->ops == &legacy_fs_context_ops) {
> 		switch (cmd) {
> 		case FSCONFIG_SET_BINARY:
> 		case FSCONFIG_SET_PATH:
> 		case FSCONFIG_SET_PATH_EMPTY:
> 		case FSCONFIG_SET_FD:
> 			ret = -EOPNOTSUPP;
> 			goto out_f;
> 		}
> 	}
> 
> fs/fs_context.c
> 	/* TODO: Make all filesystems support this unconditionally */
> 	init_fs_context = fc->fs_type->init_fs_context;
> 	if (!init_fs_context)
> 		init_fs_context = legacy_init_fs_context;
> ...
> /*
>  * Initialise a legacy context for a filesystem that doesn't support
>  * fs_context.
>  */
> static int legacy_init_fs_context(struct fs_context *fc)
> {
> 	fc->fs_private = kzalloc(sizeof(struct legacy_fs_context), GFP_KERNEL);
> 	if (!fc->fs_private)
> 		return -ENOMEM;
> 	fc->ops = &legacy_fs_context_ops;
> 	return 0;
> }

Right, I have seen that all earlier and so I knew that the failures
here are just fine.

> Code coming from v5.1-rc1 f3a09c92018a91ad0981146a4ac59414f814d801 introduce
> fs_context methods [2]. Other patchsets here [3] shows there is some support for
> fs_context in VFS. So I wonder how to achieve not end up with legacy context.
> 
> > +++ b/testcases/kernel/syscalls/fsconfig/fsconfig02.c
> > +  {"set-path-key", &fd, FSCONFIG_SET_PATH, NULL, "/dev/sda1", &aux_fdcwd, EINVAL},
> ...
> > +  {"set-path-aux", &fd, FSCONFIG_SET_PATH, "sync", "/dev/sda1", &aux_minus1, EINVAL},
> /dev/sda1 is valid on some hosts, but invalid on others. Shouldn't we use
> /dev/foo instead?

Should be fine I think.

> ...
> > +	temp_fd = open("testfile", O_RDWR | O_CREAT, 01444);
> > +	if (temp_fd == -1)
> > +		tst_res(TBROK, "Can't obtain temp_fd, open() failed");
> Here needs to be now tst_brk(TBROK).
> Again I'll change it before merge.

Thanks.

-- 
viresh

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper
  2020-02-27  5:14 ` [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper Viresh Kumar
@ 2020-03-06 12:45   ` Cyril Hrubis
  2020-03-07 12:42     ` Li Wang
  0 siblings, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2020-03-06 12:45 UTC (permalink / raw)
  To: ltp

Hi!
> This patch moves the ismount() helper added to the fsmount syscall tests
> to the standard library and renames it to tst_is_mounted(). The helper
> can be used across different files now.
> 
> Minor modifications are also done to the helper.
> 
> Acked-by: Li Wang <liwang@redhat.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  include/tst_device.h                          |  6 +++++
>  lib/tst_device.c                              | 14 +++++++++++
>  testcases/kernel/syscalls/fsmount/fsmount01.c | 25 +------------------
>  3 files changed, 21 insertions(+), 24 deletions(-)
> 
> diff --git a/include/tst_device.h b/include/tst_device.h
> index f5609f5986bb..bd6910672d2f 100644
> --- a/include/tst_device.h
> +++ b/include/tst_device.h
> @@ -36,6 +36,12 @@ extern struct tst_device *tst_device;
>   */
>  int tst_umount(const char *path);
>  
> +/*
> + * Verifies if an earlier mount is successful or not.
> + * @path: Mount path to verify
> + */
> +int tst_is_mounted(const char *path);
> +
>  /*
>   * Clears a first few blocks of the device. This is needed when device has
>   * already been formatted with a filesystems, subset of mkfs.foo utils aborts
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index 8b5459def1cb..8bf6dacf5973 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -386,6 +386,20 @@ int tst_umount(const char *path)
>  	return -1;
>  }
>  
> +int tst_is_mounted(const char *path)
> +{
> +	char cmd[PATH_MAX];
> +	int ret;
> +
> +	snprintf(cmd, sizeof(cmd), "mountpoint -q %s", path);
> +	ret = tst_system(cmd);

I'm not sure that depending on mountpoint command is right choice, there
are all kinds of embedded systems out there that may be missing it.

Also this does not even handle the case that the command is missing.

Looking at the v4 version, all we need is to correctly parse each line
from from /proc/mounts. I would just use strsep() with space as a
delimited and took first token that starts with a slash i.e. '/', then
we can just strcmp() it against the path. Or do I miss something?

> +	if (ret)
> +		tst_resm(TINFO, "No device is mounted at %s", path);
> +
> +	return !ret;
> +}
> +
>  int find_stat_file(const char *dev, char *path, size_t path_len)
>  {
>  	const char *devname = strrchr(dev, '/') + 1;
> diff --git a/testcases/kernel/syscalls/fsmount/fsmount01.c b/testcases/kernel/syscalls/fsmount/fsmount01.c
> index 83185b48aedd..8e29a1537334 100644
> --- a/testcases/kernel/syscalls/fsmount/fsmount01.c
> +++ b/testcases/kernel/syscalls/fsmount/fsmount01.c
> @@ -12,30 +12,10 @@
>  #include "tst_test.h"
>  #include "lapi/fcntl.h"
>  #include "lapi/fsmount.h"
> -#include "tst_safe_stdio.h"
>  
> -#define LINELENGTH 256
>  #define MNTPOINT "newmount_point"
>  static int sfd, mfd, is_mounted;
>  
> -static int ismount(char *mntpoint)
> -{
> -	int ret = 0;
> -	FILE *file;
> -	char line[LINELENGTH];
> -
> -	file = SAFE_FOPEN("/proc/mounts", "r");
> -
> -	while (fgets(line, sizeof(line), file)) {
> -		if (strstr(line, mntpoint) != NULL) {
> -			ret = 1;
> -			break;
> -		}
> -	}
> -	SAFE_FCLOSE(file);
> -	return ret;
> -}
> -
>  static void cleanup(void)
>  {
>  	if (is_mounted)
> @@ -76,12 +56,9 @@ static void test_fsmount(void)
>  	tst_res(TPASS, "move_mount() attached to the mount point");
>  	SAFE_CLOSE(mfd);
>  
> -	if (ismount(MNTPOINT)) {
> -		tst_res(TPASS, "device mounted");
> +	if (tst_is_mounted(MNTPOINT)) {
>  		SAFE_UMOUNT(MNTPOINT);
>  		is_mounted = 0;
> -	} else {
> -		tst_res(TFAIL, "device not mounted");
>  	}
>  }
>  
> -- 
> 2.21.0.rc0.269.g1a574e7a288b
> 

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel()
  2020-02-27  5:14 ` [LTP] [PATCH V5 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel() Viresh Kumar
@ 2020-03-06 12:47   ` Cyril Hrubis
  2020-03-11  7:22     ` Viresh Kumar
  0 siblings, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2020-03-06 12:47 UTC (permalink / raw)
  To: ltp

On Thu, Feb 27, 2020 at 10:44:30AM +0530, Viresh Kumar wrote:
> Add a helper to check if the fsmount() related syscalls are supported by
> the kernel or not.
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Acked-by: Li Wang <liwang@redhat.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  include/lapi/fsmount.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/lapi/fsmount.h b/include/lapi/fsmount.h
> index 87f2f229c371..a6a24904e66d 100644
> --- a/include/lapi/fsmount.h
> +++ b/include/lapi/fsmount.h
> @@ -130,5 +130,14 @@ enum fsconfig_command {
>  
>  #endif /* OPEN_TREE_CLONE */
>  
> +void fsopen_supported_by_kernel(void)
> +{
> +	if ((tst_kvercmp(5, 2, 0)) < 0) {
> +		/* Check if the syscall is backported on an older kernel */
> +		TEST(syscall(__NR_fsopen, NULL, 0));
> +		if (TST_RET == -1 && TST_ERR == ENOSYS)
> +			tst_brk(TCONF, "Test not supported on kernel version < v5.2");

		Shouldn't we close the TST_RET here?

> +	}
> +}
>  
>  #endif /* FSMOUNT_H__ */
> -- 
> 2.21.0.rc0.269.g1a574e7a288b
> 

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 04/10] syscalls/fsopen: New tests
  2020-02-27  5:14 ` [LTP] [PATCH V5 04/10] syscalls/fsopen: New tests Viresh Kumar
@ 2020-03-06 13:10   ` Cyril Hrubis
  2020-03-11  7:25     ` Viresh Kumar
  0 siblings, 1 reply; 31+ messages in thread
From: Cyril Hrubis @ 2020-03-06 13:10 UTC (permalink / raw)
  To: ltp

Hi!
> +	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
> +			MOVE_MOUNT_F_EMPTY_PATH));
> +
> +	SAFE_CLOSE(fsmfd);
> +
> +	if (TST_RET == -1) {
> +		tst_res(TFAIL | TERRNO, "move_mount() failed");
> +		goto out;
> +	}
> +
> +	if (tst_is_mounted(MNTPOINT))
> +		tst_res(TPASS, "%s: fsopen() passed", tc->name);
> +
> +	SAFE_UMOUNT(MNTPOINT);

I gues sthat the SAFE_UMOUNT() should be inside of the if here and in
the rest of the testcases.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls
  2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (9 preceding siblings ...)
  2020-02-27  5:14 ` [LTP] [PATCH V5 10/10] syscalls/open_tree: " Viresh Kumar
@ 2020-03-06 13:18 ` Cyril Hrubis
  10 siblings, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2020-03-06 13:18 UTC (permalink / raw)
  To: ltp

Hi!
Otherwise the minor things I and Peter pointed out the patchset looks
good to me.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 05/10] syscalls/fsconfig: New tests
  2020-02-28 16:01   ` Petr Vorel
  2020-03-02  8:21     ` Viresh Kumar
@ 2020-03-06 13:25     ` Petr Vorel
  1 sibling, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2020-03-06 13:25 UTC (permalink / raw)
  To: ltp

Hi Viresh,

> I get TCONF for all fsconfig01 results, while I'm using 5.5.5-1-default:

> tst_test.c:1290: INFO: Testing on ext2
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
> mke2fs 1.45.5 (07-Jan-2020)
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
> fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
> fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
> fsconfig01.c:92: PASS: fsconfig() passed
> tst_test.c:1290: INFO: Testing on ext3
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts=''
> mke2fs 1.45.5 (07-Jan-2020)
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
> fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
> fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
> fsconfig01.c:92: PASS: fsconfig() passed
> tst_test.c:1290: INFO: Testing on ext4
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts=''
> mke2fs 1.45.5 (07-Jan-2020)
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
> fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
> fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
> fsconfig01.c:92: PASS: fsconfig() passed
> tst_test.c:1290: INFO: Testing on xfs
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:92: PASS: fsconfig() passed
> tst_test.c:1290: INFO: Testing on btrfs
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with btrfs opts='' extra opts=''
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
> fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
> fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
> fsconfig01.c:92: PASS: fsconfig() passed
> tst_test.c:1290: INFO: Testing on vfat
> tst_mkfs.c:89: INFO: Formatting /dev/loop0 with vfat opts='' extra opts=''
> tst_test.c:1227: INFO: Timeout per run is 0h 05m 00s
> fsconfig01.c:43: CONF: fsconfig(): FSCONFIG_SET_PATH not supported
> fsconfig01.c:53: CONF: fsconfig(): FSCONFIG_SET_PATH_EMPTY not supported
> fsconfig01.c:63: CONF: fsconfig(): FSCONFIG_SET_FD not supported
> fsconfig01.c:92: PASS: fsconfig() passed

> Not yet merged man page [1] (I reposted David Howells commit) there is
> explanation for EOPNOTSUPP: The command given by cmd was not valid.

> First, I suspected "sync" option is wrong. But looking at kernel sources it's
> really not implemented:
I'm sorry, I was wrong. I overlooked It's supported, but just by xfs
(which is surprising).

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper
  2020-03-06 12:45   ` Cyril Hrubis
@ 2020-03-07 12:42     ` Li Wang
  2020-03-11  7:31       ` Viresh Kumar
  2020-03-11 10:26       ` Cyril Hrubis
  0 siblings, 2 replies; 31+ messages in thread
From: Li Wang @ 2020-03-07 12:42 UTC (permalink / raw)
  To: ltp

On Fri, Mar 6, 2020 at 8:45 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> ...
> >
> > +int tst_is_mounted(const char *path)
> > +{
> > +     char cmd[PATH_MAX];
> > +     int ret;
> > +
> > +     snprintf(cmd, sizeof(cmd), "mountpoint -q %s", path);
> > +     ret = tst_system(cmd);
>
> I'm not sure that depending on mountpoint command is right choice, there
> are all kinds of embedded systems out there that may be missing it.


Good point, we'd better avoid involving other packages as the dependence of
LTP.


> Also this does not even handle the case that the command is missing.
>
> Looking at the v4 version, all we need is to correctly parse each line
> from from /proc/mounts. I would just use strsep() with space as a
> delimited and took first token that starts with a slash i.e. '/', then
> we can just strcmp() it against the path. Or do I miss something?
>

I'm afraid strcmp() can not satisfy the requirement for us. As you know LTP
creates the MNTPOINT in temp dir that means it could not accurately match
the string path which extracts from /proc/mounts with a slash.

e.g
#define MNTPOINT "fallocate"
...
/dev/loop4 on /tmp/FPp7kh/fallocate type xfs
(rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
...
strcmp("/tmp/FPp7kh/fallocate", MNTPOINT) will never ruturn 0 to us.

What I can think of is to use strrchr() to cut the string after last '/',
but that can only work for test mount fs in LTP ways. Other situations
might not satisfy.

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200307/02bcfb77/attachment.htm>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel()
  2020-03-06 12:47   ` Cyril Hrubis
@ 2020-03-11  7:22     ` Viresh Kumar
  0 siblings, 0 replies; 31+ messages in thread
From: Viresh Kumar @ 2020-03-11  7:22 UTC (permalink / raw)
  To: ltp

On 06-03-20, 13:47, Cyril Hrubis wrote:
> On Thu, Feb 27, 2020 at 10:44:30AM +0530, Viresh Kumar wrote:
> > Add a helper to check if the fsmount() related syscalls are supported by
> > the kernel or not.
> > 
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > Acked-by: Li Wang <liwang@redhat.com>
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  include/lapi/fsmount.h | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/include/lapi/fsmount.h b/include/lapi/fsmount.h
> > index 87f2f229c371..a6a24904e66d 100644
> > --- a/include/lapi/fsmount.h
> > +++ b/include/lapi/fsmount.h
> > @@ -130,5 +130,14 @@ enum fsconfig_command {
> >  
> >  #endif /* OPEN_TREE_CLONE */
> >  
> > +void fsopen_supported_by_kernel(void)
> > +{
> > +	if ((tst_kvercmp(5, 2, 0)) < 0) {
> > +		/* Check if the syscall is backported on an older kernel */
> > +		TEST(syscall(__NR_fsopen, NULL, 0));
> > +		if (TST_RET == -1 && TST_ERR == ENOSYS)
> > +			tst_brk(TCONF, "Test not supported on kernel version < v5.2");
> 
> 		Shouldn't we close the TST_RET here?

I didn't do that in the else part as this call should never succeed
and it will be a bug if it succeeds. Do you still want me to do it ?

-- 
viresh

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 04/10] syscalls/fsopen: New tests
  2020-03-06 13:10   ` Cyril Hrubis
@ 2020-03-11  7:25     ` Viresh Kumar
  2020-03-12  8:11       ` Petr Vorel
  0 siblings, 1 reply; 31+ messages in thread
From: Viresh Kumar @ 2020-03-11  7:25 UTC (permalink / raw)
  To: ltp

On 06-03-20, 14:10, Cyril Hrubis wrote:
> Hi!
> > +	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
> > +			MOVE_MOUNT_F_EMPTY_PATH));
> > +
> > +	SAFE_CLOSE(fsmfd);
> > +
> > +	if (TST_RET == -1) {
> > +		tst_res(TFAIL | TERRNO, "move_mount() failed");
> > +		goto out;
> > +	}
> > +
> > +	if (tst_is_mounted(MNTPOINT))
> > +		tst_res(TPASS, "%s: fsopen() passed", tc->name);
> > +
> > +	SAFE_UMOUNT(MNTPOINT);
> 
> I gues sthat the SAFE_UMOUNT() should be inside of the if here and in
> the rest of the testcases.

Petr had a similar comment earlier and here is my explanation to it.

There should always be a unmount() in response to a successful call to
mount() APIs. What if, because of some other bugs in the kernel or
testsuite, tst_is_mounted() returns 0? We should still do the
unmount() part as the mount() API didn't return an error.

-- 
viresh

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper
  2020-03-07 12:42     ` Li Wang
@ 2020-03-11  7:31       ` Viresh Kumar
  2020-03-11 10:20         ` Cyril Hrubis
  2020-03-11 10:26       ` Cyril Hrubis
  1 sibling, 1 reply; 31+ messages in thread
From: Viresh Kumar @ 2020-03-11  7:31 UTC (permalink / raw)
  To: ltp

On 07-03-20, 20:42, Li Wang wrote:
> On Fri, Mar 6, 2020 at 8:45 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> 
> > ...
> > >
> > > +int tst_is_mounted(const char *path)
> > > +{
> > > +     char cmd[PATH_MAX];
> > > +     int ret;
> > > +
> > > +     snprintf(cmd, sizeof(cmd), "mountpoint -q %s", path);
> > > +     ret = tst_system(cmd);
> >
> > I'm not sure that depending on mountpoint command is right choice, there
> > are all kinds of embedded systems out there that may be missing it.
> 
> 
> Good point, we'd better avoid involving other packages as the dependence of
> LTP.
> 
> 
> > Also this does not even handle the case that the command is missing.
> >
> > Looking at the v4 version, all we need is to correctly parse each line
> > from from /proc/mounts. I would just use strsep() with space as a
> > delimited and took first token that starts with a slash i.e. '/', then
> > we can just strcmp() it against the path. Or do I miss something?
> >
> 
> I'm afraid strcmp() can not satisfy the requirement for us. As you know LTP
> creates the MNTPOINT in temp dir that means it could not accurately match
> the string path which extracts from /proc/mounts with a slash.
> 
> e.g
> #define MNTPOINT "fallocate"
> ...
> /dev/loop4 on /tmp/FPp7kh/fallocate type xfs
> (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
> ...
> strcmp("/tmp/FPp7kh/fallocate", MNTPOINT) will never ruturn 0 to us.
> 
> What I can think of is to use strrchr() to cut the string after last '/',
> but that can only work for test mount fs in LTP ways. Other situations
> might not satisfy.

@Cyril, can we please finalize what you guys want me to do here ? I
don't really want to repost the patch, which still has issues :)

-- 
viresh

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper
  2020-03-11  7:31       ` Viresh Kumar
@ 2020-03-11 10:20         ` Cyril Hrubis
  0 siblings, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2020-03-11 10:20 UTC (permalink / raw)
  To: ltp

Hi!
> > I'm afraid strcmp() can not satisfy the requirement for us. As you know LTP
> > creates the MNTPOINT in temp dir that means it could not accurately match
> > the string path which extracts from /proc/mounts with a slash.
> > 
> > e.g
> > #define MNTPOINT "fallocate"
> > ...
> > /dev/loop4 on /tmp/FPp7kh/fallocate type xfs
> > (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
> > ...
> > strcmp("/tmp/FPp7kh/fallocate", MNTPOINT) will never ruturn 0 to us.
> > 
> > What I can think of is to use strrchr() to cut the string after last '/',
> > but that can only work for test mount fs in LTP ways. Other situations
> > might not satisfy.
> 
> @Cyril, can we please finalize what you guys want me to do here ? I
> don't really want to repost the patch, which still has issues :)

Sorry for the back and forth.

I remmebered yesterday that there is setmntent() in libc, so i we call
that on /proc/mounts, the whole problem would be reduced to a path
comparsion.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper
  2020-03-07 12:42     ` Li Wang
  2020-03-11  7:31       ` Viresh Kumar
@ 2020-03-11 10:26       ` Cyril Hrubis
  2020-03-11 12:45         ` Li Wang
  2020-03-12 11:03         ` Viresh Kumar
  1 sibling, 2 replies; 31+ messages in thread
From: Cyril Hrubis @ 2020-03-11 10:26 UTC (permalink / raw)
  To: ltp

Hi!
> > Also this does not even handle the case that the command is missing.
> >
> > Looking at the v4 version, all we need is to correctly parse each line
> > from from /proc/mounts. I would just use strsep() with space as a
> > delimited and took first token that starts with a slash i.e. '/', then
> > we can just strcmp() it against the path. Or do I miss something?
> >
> 
> I'm afraid strcmp() can not satisfy the requirement for us. As you know LTP
> creates the MNTPOINT in temp dir that means it could not accurately match
> the string path which extracts from /proc/mounts with a slash.
> 
> e.g
> #define MNTPOINT "fallocate"
> ...
> /dev/loop4 on /tmp/FPp7kh/fallocate type xfs
> (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
> ...
> strcmp("/tmp/FPp7kh/fallocate", MNTPOINT) will never ruturn 0 to us.
> 
> What I can think of is to use strrchr() to cut the string after last '/',
> but that can only work for test mount fs in LTP ways. Other situations
> might not satisfy.

Hmm, for that we have to have compose the path for the comparsion
anyways, since unless we pass an absoule path we can never be user if we
have a right match or not. There may be leftover mount points from a
failed tests that have faile to cleanup properly as well.

So I guess that we need one more function, with tmpdir in name, that
would compose the right path for us and then call the tst_is_mntpoint().

I would have called that:

int tst_is_mntpoint_at_tmpdir(const char *path);

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper
  2020-03-11 10:26       ` Cyril Hrubis
@ 2020-03-11 12:45         ` Li Wang
  2020-03-11 13:11           ` Li Wang
  2020-03-12 11:03         ` Viresh Kumar
  1 sibling, 1 reply; 31+ messages in thread
From: Li Wang @ 2020-03-11 12:45 UTC (permalink / raw)
  To: ltp

On Wed, Mar 11, 2020 at 6:26 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > > Also this does not even handle the case that the command is missing.
> > >
> > > Looking at the v4 version, all we need is to correctly parse each line
> > > from from /proc/mounts. I would just use strsep() with space as a
> > > delimited and took first token that starts with a slash i.e. '/', then
> > > we can just strcmp() it against the path. Or do I miss something?
> > >
> >
> > I'm afraid strcmp() can not satisfy the requirement for us. As you know
> LTP
> > creates the MNTPOINT in temp dir that means it could not accurately match
> > the string path which extracts from /proc/mounts with a slash.
> >
> > e.g
> > #define MNTPOINT "fallocate"
> > ...
> > /dev/loop4 on /tmp/FPp7kh/fallocate type xfs
> > (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
> > ...
> > strcmp("/tmp/FPp7kh/fallocate", MNTPOINT) will never ruturn 0 to us.
> >
> > What I can think of is to use strrchr() to cut the string after last '/',
> > but that can only work for test mount fs in LTP ways. Other situations
> > might not satisfy.
>
> Hmm, for that we have to have compose the path for the comparsion
> anyways, since unless we pass an absoule path we can never be user if we
> have a right match or not. There may be leftover mount points from a
> failed tests that have faile to cleanup properly as well.
>
> So I guess that we need one more function, with tmpdir in name, that
> would compose the right path for us and then call the tst_is_mntpoint().
>

+1 it makes sense to me.


>
> I would have called that:
>
> int tst_is_mntpoint_at_tmpdir(const char *path);
>

Hmm, the return value shouldn't the full right path, why return int here?
or I misunderstand here?

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200311/1972cc02/attachment.htm>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper
  2020-03-11 12:45         ` Li Wang
@ 2020-03-11 13:11           ` Li Wang
  0 siblings, 0 replies; 31+ messages in thread
From: Li Wang @ 2020-03-11 13:11 UTC (permalink / raw)
  To: ltp

On Wed, Mar 11, 2020 at 8:45 PM Li Wang <liwang@redhat.com> wrote:

> ...
>>
>> int tst_is_mntpoint_at_tmpdir(const char *path);
>>
>
> Hmm, the return value shouldn't the full right path, why return int here?
> or I misunderstand here?
>
>
s/shouldn't/should

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200311/538c16c8/attachment.htm>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 04/10] syscalls/fsopen: New tests
  2020-03-11  7:25     ` Viresh Kumar
@ 2020-03-12  8:11       ` Petr Vorel
  2020-03-12 10:03         ` Viresh Kumar
  0 siblings, 1 reply; 31+ messages in thread
From: Petr Vorel @ 2020-03-12  8:11 UTC (permalink / raw)
  To: ltp

Hi Viresh,

> > > +	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
> > > +			MOVE_MOUNT_F_EMPTY_PATH));
> > > +
> > > +	SAFE_CLOSE(fsmfd);
> > > +
> > > +	if (TST_RET == -1) {
> > > +		tst_res(TFAIL | TERRNO, "move_mount() failed");
> > > +		goto out;
> > > +	}
> > > +
> > > +	if (tst_is_mounted(MNTPOINT))
> > > +		tst_res(TPASS, "%s: fsopen() passed", tc->name);
> > > +
> > > +	SAFE_UMOUNT(MNTPOINT);

> > I gues sthat the SAFE_UMOUNT() should be inside of the if here and in
> > the rest of the testcases.

> Petr had a similar comment earlier and here is my explanation to it.

> There should always be a unmount() in response to a successful call to
> mount() APIs. What if, because of some other bugs in the kernel or
> testsuite, tst_is_mounted() returns 0? We should still do the
> unmount() part as the mount() API didn't return an error.
But IMHO if device is not mounted we get TBROK due EINVAL in safe_umount().
I'd understand if this was in cleanup function where TBROK turns to TWARN.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 04/10] syscalls/fsopen: New tests
  2020-03-12  8:11       ` Petr Vorel
@ 2020-03-12 10:03         ` Viresh Kumar
  2020-03-12 10:11           ` Cyril Hrubis
  0 siblings, 1 reply; 31+ messages in thread
From: Viresh Kumar @ 2020-03-12 10:03 UTC (permalink / raw)
  To: ltp

On 12-03-20, 09:11, Petr Vorel wrote:
> Hi Viresh,
> 
> > > > +	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
> > > > +			MOVE_MOUNT_F_EMPTY_PATH));
> > > > +
> > > > +	SAFE_CLOSE(fsmfd);
> > > > +
> > > > +	if (TST_RET == -1) {
> > > > +		tst_res(TFAIL | TERRNO, "move_mount() failed");
> > > > +		goto out;
> > > > +	}
> > > > +
> > > > +	if (tst_is_mounted(MNTPOINT))
> > > > +		tst_res(TPASS, "%s: fsopen() passed", tc->name);
> > > > +
> > > > +	SAFE_UMOUNT(MNTPOINT);
> 
> > > I gues sthat the SAFE_UMOUNT() should be inside of the if here and in
> > > the rest of the testcases.
> 
> > Petr had a similar comment earlier and here is my explanation to it.
> 
> > There should always be a unmount() in response to a successful call to
> > mount() APIs. What if, because of some other bugs in the kernel or
> > testsuite, tst_is_mounted() returns 0? We should still do the
> > unmount() part as the mount() API didn't return an error.
> But IMHO if device is not mounted we get TBROK due EINVAL in safe_umount().

But why won't move_mount() fail if device isn't mounted ? Why do we
need the tst_is_mounted() helper at all ? Just to catch a case where
move_mount() is buggy and doesn't report the failure properly, right ?
In case of that bug, isn't it possible that move_mount() allocates
some resources which must be freed with a call to umount()
nevertheless ?

-- 
viresh

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 04/10] syscalls/fsopen: New tests
  2020-03-12 10:03         ` Viresh Kumar
@ 2020-03-12 10:11           ` Cyril Hrubis
  0 siblings, 0 replies; 31+ messages in thread
From: Cyril Hrubis @ 2020-03-12 10:11 UTC (permalink / raw)
  To: ltp

Hi!
> > > > I gues sthat the SAFE_UMOUNT() should be inside of the if here and in
> > > > the rest of the testcases.
> > 
> > > Petr had a similar comment earlier and here is my explanation to it.
> > 
> > > There should always be a unmount() in response to a successful call to
> > > mount() APIs. What if, because of some other bugs in the kernel or
> > > testsuite, tst_is_mounted() returns 0? We should still do the
> > > unmount() part as the mount() API didn't return an error.
> > But IMHO if device is not mounted we get TBROK due EINVAL in safe_umount().
> 
> But why won't move_mount() fail if device isn't mounted ? Why do we
> need the tst_is_mounted() helper at all ? Just to catch a case where
> move_mount() is buggy and doesn't report the failure properly, right ?
> In case of that bug, isn't it possible that move_mount() allocates
> some resources which must be freed with a call to umount()
> nevertheless ?

We can't really clean things up when something in kernel is misbehaving.
If there is a bug we enter the wast lands of undefined behavior where
anything is possible and any attempt to restore the system in a defined
state is doomed to fail anyways.

So in the end I do not care here as long as we clean up properly when
things work as expected, so either way is fine. It only looks a bit
strange when we attempt to umount things that are possibly not mounted.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper
  2020-03-11 10:26       ` Cyril Hrubis
  2020-03-11 12:45         ` Li Wang
@ 2020-03-12 11:03         ` Viresh Kumar
  2020-03-12 11:35           ` Petr Vorel
  1 sibling, 1 reply; 31+ messages in thread
From: Viresh Kumar @ 2020-03-12 11:03 UTC (permalink / raw)
  To: ltp

On 11-03-20, 11:26, Cyril Hrubis wrote:
> Hmm, for that we have to have compose the path for the comparsion
> anyways, since unless we pass an absoule path we can never be user if we
> have a right match or not. There may be leftover mount points from a
> failed tests that have faile to cleanup properly as well.
> 
> So I guess that we need one more function, with tmpdir in name, that
> would compose the right path for us and then call the tst_is_mntpoint().
> 
> I would have called that:
> 
> int tst_is_mntpoint_at_tmpdir(const char *path);

Is everyone fine with this code now :)

int tst_is_mounted(const char *path)
{
        char line[PATH_MAX];
        FILE *file;
        int ret = 0;

        file = SAFE_FOPEN(NULL, "/proc/mounts", "r");

        while (fgets(line, sizeof(line), file)) {
                if (strstr(line, path) != NULL) {
                        ret = 1;
                        break;
                }
        }

        SAFE_FCLOSE(NULL, file);

        if (!ret)
                tst_resm(TINFO, "No device is mounted at %s", path);

        return ret;
}

int tst_is_mounted_at_tmpdir(const char *path)
{
        char cdir[PATH_MAX], mpath[PATH_MAX];
        int ret;

        if (!getcwd(cdir, PATH_MAX))
                return 0;

        ret = snprintf(mpath, PATH_MAX, "%s/%s", cdir, path);
        if (ret < 0 || ret >= PATH_MAX)
                return 0;

        return tst_is_mounted(mpath);
}

-- 
viresh

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper
  2020-03-12 11:03         ` Viresh Kumar
@ 2020-03-12 11:35           ` Petr Vorel
  0 siblings, 0 replies; 31+ messages in thread
From: Petr Vorel @ 2020-03-12 11:35 UTC (permalink / raw)
  To: ltp

Hi Viresh,

> Is everyone fine with this code now :)

> int tst_is_mounted(const char *path)
> {
>         char line[PATH_MAX];
>         FILE *file;
>         int ret = 0;

>         file = SAFE_FOPEN(NULL, "/proc/mounts", "r");

>         while (fgets(line, sizeof(line), file)) {
>                 if (strstr(line, path) != NULL) {
>                         ret = 1;
>                         break;
>                 }
>         }

>         SAFE_FCLOSE(NULL, file);

>         if (!ret)
>                 tst_resm(TINFO, "No device is mounted at %s", path);

>         return ret;
> }

> int tst_is_mounted_at_tmpdir(const char *path)
> {
>         char cdir[PATH_MAX], mpath[PATH_MAX];
>         int ret;

>         if (!getcwd(cdir, PATH_MAX))
>                 return 0;
LGTM. I guess we can ignore this, but maybe tst_res(TWARN | TERRNO, "..."),
could be added here. But maybe it's not important.

>         ret = snprintf(mpath, PATH_MAX, "%s/%s", cdir, path);
>         if (ret < 0 || ret >= PATH_MAX)
>                 return 0;

>         return tst_is_mounted(mpath);
> }

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2020-03-12 11:35 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27  5:14 [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
2020-02-27  5:14 ` [LTP] [PATCH V5 01/10] tst_device: Add tst_is_mounted() helper Viresh Kumar
2020-03-06 12:45   ` Cyril Hrubis
2020-03-07 12:42     ` Li Wang
2020-03-11  7:31       ` Viresh Kumar
2020-03-11 10:20         ` Cyril Hrubis
2020-03-11 10:26       ` Cyril Hrubis
2020-03-11 12:45         ` Li Wang
2020-03-11 13:11           ` Li Wang
2020-03-12 11:03         ` Viresh Kumar
2020-03-12 11:35           ` Petr Vorel
2020-02-27  5:14 ` [LTP] [PATCH V5 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel() Viresh Kumar
2020-03-06 12:47   ` Cyril Hrubis
2020-03-11  7:22     ` Viresh Kumar
2020-02-27  5:14 ` [LTP] [PATCH V5 03/10] lapi/fsmount.h: Include "lapi/fcntl.h" Viresh Kumar
2020-02-27  5:14 ` [LTP] [PATCH V5 04/10] syscalls/fsopen: New tests Viresh Kumar
2020-03-06 13:10   ` Cyril Hrubis
2020-03-11  7:25     ` Viresh Kumar
2020-03-12  8:11       ` Petr Vorel
2020-03-12 10:03         ` Viresh Kumar
2020-03-12 10:11           ` Cyril Hrubis
2020-02-27  5:14 ` [LTP] [PATCH V5 05/10] syscalls/fsconfig: " Viresh Kumar
2020-02-28 16:01   ` Petr Vorel
2020-03-02  8:21     ` Viresh Kumar
2020-03-06 13:25     ` Petr Vorel
2020-02-27  5:14 ` [LTP] [PATCH V5 06/10] syscalls/fsmount: Improve fsmount01 test Viresh Kumar
2020-02-27  5:14 ` [LTP] [PATCH V5 07/10] syscalls/fsmount: Add failure tests Viresh Kumar
2020-02-27  5:14 ` [LTP] [PATCH V5 08/10] syscalls/move_mount: New tests Viresh Kumar
2020-02-27  5:14 ` [LTP] [PATCH V5 09/10] syscalls/fspick: " Viresh Kumar
2020-02-27  5:14 ` [LTP] [PATCH V5 10/10] syscalls/open_tree: " Viresh Kumar
2020-03-06 13:18 ` [LTP] [PATCH V5 00/10] Add new LTP tests related to fsmount family of syscalls Cyril Hrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.