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

Hello,

Here is the third 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.

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_ismount() 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                              |  23 ++++
 runtest/syscalls                              |  16 +++
 testcases/kernel/syscalls/fsconfig/.gitignore |   2 +
 testcases/kernel/syscalls/fsconfig/Makefile   |   6 +
 .../kernel/syscalls/fsconfig/fsconfig01.c     | 104 +++++++++++++++
 .../kernel/syscalls/fsconfig/fsconfig02.c     |  98 ++++++++++++++
 testcases/kernel/syscalls/fsmount/.gitignore  |   1 +
 testcases/kernel/syscalls/fsmount/fsmount01.c | 121 +++++++++--------
 testcases/kernel/syscalls/fsmount/fsmount02.c |  79 +++++++++++
 testcases/kernel/syscalls/fsopen/.gitignore   |   2 +
 testcases/kernel/syscalls/fsopen/Makefile     |   6 +
 testcases/kernel/syscalls/fsopen/fsopen01.c   |  83 ++++++++++++
 testcases/kernel/syscalls/fsopen/fsopen02.c   |  55 ++++++++
 testcases/kernel/syscalls/fspick/.gitignore   |   2 +
 testcases/kernel/syscalls/fspick/Makefile     |   6 +
 testcases/kernel/syscalls/fspick/fspick01.c   | 117 +++++++++++++++++
 testcases/kernel/syscalls/fspick/fspick02.c   | 105 +++++++++++++++
 .../kernel/syscalls/move_mount/.gitignore     |   2 +
 testcases/kernel/syscalls/move_mount/Makefile |   6 +
 .../kernel/syscalls/move_mount/move_mount01.c |  86 ++++++++++++
 .../kernel/syscalls/move_mount/move_mount02.c |  95 ++++++++++++++
 .../kernel/syscalls/open_tree/.gitignore      |   2 +
 testcases/kernel/syscalls/open_tree/Makefile  |   6 +
 .../kernel/syscalls/open_tree/open_tree01.c   | 124 ++++++++++++++++++
 .../kernel/syscalls/open_tree/open_tree02.c   | 104 +++++++++++++++
 27 files changed, 1209 insertions(+), 59 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/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] 33+ messages in thread

* [LTP] [PATCH V3 01/10] tst_device: Add tst_ismount() helper
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
@ 2020-02-25  6:40 ` Viresh Kumar
  2020-02-25 10:39   ` [LTP] [PATCH V4 1/10] tst_device: Add tst_is_mounted() helper Viresh Kumar
  2020-02-25  6:40 ` [LTP] [PATCH V3 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel() Viresh Kumar
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25  6:40 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_ismount(). 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                              | 23 +++++++++++++++++
 testcases/kernel/syscalls/fsmount/fsmount01.c | 25 +------------------
 3 files changed, 30 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..c53ae36cf8b9 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -386,6 +386,29 @@ int tst_umount(const char *path)
 	return -1;
 }
 
+int tst_is_mounted(const char *path)
+{
+	char line[256];
+	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(TWARN, "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] 33+ messages in thread

* [LTP] [PATCH V3 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel()
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
  2020-02-25  6:40 ` [LTP] [PATCH V3 01/10] tst_device: Add tst_ismount() helper Viresh Kumar
@ 2020-02-25  6:40 ` Viresh Kumar
  2020-02-26  5:51   ` Zorro Lang
  2020-02-25  6:40 ` [LTP] [PATCH V3 03/10] lapi/fsmount.h: Include "lapi/fcntl.h" Viresh Kumar
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25  6:40 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>
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] 33+ messages in thread

* [LTP] [PATCH V3 03/10] lapi/fsmount.h: Include "lapi/fcntl.h"
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
  2020-02-25  6:40 ` [LTP] [PATCH V3 01/10] tst_device: Add tst_ismount() helper Viresh Kumar
  2020-02-25  6:40 ` [LTP] [PATCH V3 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel() Viresh Kumar
@ 2020-02-25  6:40 ` Viresh Kumar
  2020-02-26  5:51   ` Zorro Lang
  2020-02-25  6:40 ` [LTP] [PATCH V3 04/10] syscalls/fsopen: New tests Viresh Kumar
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25  6:40 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>
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] 33+ messages in thread

* [LTP] [PATCH V3 04/10] syscalls/fsopen: New tests
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (2 preceding siblings ...)
  2020-02-25  6:40 ` [LTP] [PATCH V3 03/10] lapi/fsmount.h: Include "lapi/fcntl.h" Viresh Kumar
@ 2020-02-25  6:40 ` Viresh Kumar
  2020-02-25  6:40 ` [LTP] [PATCH V3 05/10] syscalls/fsconfig: " Viresh Kumar
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25  6:40 UTC (permalink / raw)
  To: ltp

Add tests to check working of fsopen() syscall.

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 | 83 +++++++++++++++++++++
 testcases/kernel/syscalls/fsopen/fsopen02.c | 55 ++++++++++++++
 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 d2551b2eceee..b26c995da79f 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..51fab25d9a01
--- /dev/null
+++ b/testcases/kernel/syscalls/fsopen/fsopen01.c
@@ -0,0 +1,83 @@
+// 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(fsopen(tst_device->fs_type, tc->flags));
+	fd = TST_RET;
+
+	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(TBROK | TERRNO, "fsconfig() failed");
+		goto out;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		tst_res(TBROK | TERRNO, "fsconfig() failed");
+		goto out;
+	}
+
+	TEST(fsmount(fd, 0, 0));
+	if (TST_RET == -1) {
+		tst_res(TBROK | TERRNO, "fsmount() failed");
+		goto out;
+	}
+
+	fsmfd = TST_RET;
+	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, "%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..13bc0e464497
--- /dev/null
+++ b/testcases/kernel/syscalls/fsopen/fsopen02.c
@@ -0,0 +1,55 @@
+// 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);
+	} else if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: fsopen() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+	} else {
+		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] 33+ messages in thread

* [LTP] [PATCH V3 05/10] syscalls/fsconfig: New tests
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (3 preceding siblings ...)
  2020-02-25  6:40 ` [LTP] [PATCH V3 04/10] syscalls/fsopen: New tests Viresh Kumar
@ 2020-02-25  6:40 ` Viresh Kumar
  2020-02-25 13:46   ` Petr Vorel
  2020-02-25  6:40 ` [LTP] [PATCH V3 06/10] syscalls/fsmount: Improve fsmount01 test Viresh Kumar
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25  6:40 UTC (permalink / raw)
  To: ltp

Add tests to check working of fsconfig() syscall.

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     | 104 ++++++++++++++++++
 .../kernel/syscalls/fsconfig/fsconfig02.c     |  98 +++++++++++++++++
 5 files changed, 213 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 b26c995da79f..ebfbdfb4d5aa 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..d2a0825e5a64
--- /dev/null
+++ b/testcases/kernel/syscalls/fsconfig/fsconfig01.c
@@ -0,0 +1,104 @@
+// 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(fsopen(tst_device->fs_type, 0));
+	fd = TST_RET;
+
+	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(TINFO, "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(TINFO, "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(TINFO, "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(fsmount(fd, 0, 0));
+	if (TST_RET == -1) {
+		tst_res(TBROK | TERRNO, "fsmount() failed");
+		goto out;
+	}
+
+	fsmfd = TST_RET;
+	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..d51a869ac3ff
--- /dev/null
+++ b/testcases/kernel/syscalls/fsconfig/fsconfig02.c
@@ -0,0 +1,98 @@
+// 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"
+
+int fd = -1, temp_fd = -1, invalid_fd = -1;
+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(fsopen(tst_device->fs_type, 0));
+	fd = TST_RET;
+
+	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);
+	} else if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: fsconfig() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+	} else {
+		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] 33+ messages in thread

* [LTP] [PATCH V3 06/10] syscalls/fsmount: Improve fsmount01 test
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (4 preceding siblings ...)
  2020-02-25  6:40 ` [LTP] [PATCH V3 05/10] syscalls/fsconfig: " Viresh Kumar
@ 2020-02-25  6:40 ` Viresh Kumar
  2020-02-25  6:40 ` [LTP] [PATCH V3 07/10] syscalls/fsmount: Add failure tests Viresh Kumar
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25  6:40 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

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 testcases/kernel/syscalls/fsmount/fsmount01.c | 101 +++++++++++-------
 1 file changed, 65 insertions(+), 36 deletions(-)

diff --git a/testcases/kernel/syscalls/fsmount/fsmount01.c b/testcases/kernel/syscalls/fsmount/fsmount01.c
index 514d3b0b38f8..b7810df2fd5b 100644
--- a/testcases/kernel/syscalls/fsmount/fsmount01.c
+++ b/testcases/kernel/syscalls/fsmount/fsmount01.c
@@ -3,67 +3,96 @@
  * 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 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 test_fsmount(void)
+static void run(unsigned int n)
 {
+	struct tcase *tc = &tcases[n];
+	int sfd, mfd;
+
 	TEST(fsopen(tst_device->fs_type, FSOPEN_CLOEXEC));
-	if (TST_RET < 0)
-		tst_brk(TBROK | TTERRNO, "fsopen() on %s failed", tst_device->fs_type);
+	if (TST_RET == -1) {
+		tst_res(TBROK | TTERRNO, "fsopen() on %s failed",
+			tst_device->fs_type);
+		return;
+	}
 	sfd = TST_RET;
-	tst_res(TPASS, "fsopen() on %s", tst_device->fs_type);
 
 	TEST(fsconfig(sfd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
-	if (TST_RET < 0)
-		tst_brk(TBROK | TTERRNO,
+	if (TST_RET < 0) {
+		SAFE_CLOSE(sfd);
+		tst_res(TBROK | 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 < 0) {
+		SAFE_CLOSE(sfd);
+		tst_res(TBROK | 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(fsmount(sfd, tc->flags, tc->attrs));
 	SAFE_CLOSE(sfd);
 
+	if (TST_RET < 0) {
+		tst_res(TFAIL | TTERRNO,
+			"fsmount() failed to create a mount object");
+		return;
+	}
+	mfd = TST_RET;
+
 	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 < 0) {
+		tst_res(TBROK | 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] 33+ messages in thread

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

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

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 runtest/syscalls                              |  1 +
 testcases/kernel/syscalls/fsmount/.gitignore  |  1 +
 testcases/kernel/syscalls/fsmount/fsmount02.c | 79 +++++++++++++++++++
 3 files changed, 81 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fsmount/fsmount02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index ebfbdfb4d5aa..9f61a7e6682d 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..c61caa8e8808
--- /dev/null
+++ b/testcases/kernel/syscalls/fsmount/fsmount02.c
@@ -0,0 +1,79 @@
+// 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(fsopen(tst_device->fs_type, 0));
+	fd = TST_RET;
+
+	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);
+	} else if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: fsmount() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+	} else {
+		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] 33+ messages in thread

* [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (6 preceding siblings ...)
  2020-02-25  6:40 ` [LTP] [PATCH V3 07/10] syscalls/fsmount: Add failure tests Viresh Kumar
@ 2020-02-25  6:40 ` Viresh Kumar
  2020-02-25 13:57   ` Petr Vorel
  2020-02-25  6:40 ` [LTP] [PATCH V3 09/10] syscalls/fspick: " Viresh Kumar
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25  6:40 UTC (permalink / raw)
  To: ltp

Add tests to check working of move_mount() syscall.

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 | 86 +++++++++++++++++
 .../kernel/syscalls/move_mount/move_mount02.c | 95 +++++++++++++++++++
 5 files changed, 192 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 9f61a7e6682d..36a1dcfadcca 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..aef0e15e1edf
--- /dev/null
+++ b/testcases/kernel/syscalls/move_mount/move_mount01.c
@@ -0,0 +1,86 @@
+// 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(fsopen(tst_device->fs_type, 0));
+	fd = TST_RET;
+
+	if (fd == -1) {
+		tst_res(TBROK | TERRNO, "fsopen() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TBROK | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TBROK | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (TST_RET == -1) {
+		tst_res(TBROK | TERRNO, "fsmount() failed");
+		return;
+	}
+
+	fsmfd = TST_RET;
+	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..98b7be77f9f7
--- /dev/null
+++ b/testcases/kernel/syscalls/move_mount/move_mount02.c
@@ -0,0 +1,95 @@
+// 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(fsopen(tst_device->fs_type, 0));
+	fd = TST_RET;
+
+	if (fd == -1) {
+		tst_res(TBROK | TERRNO, "fsopen() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TBROK | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TBROK | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (TST_RET == -1) {
+		tst_res(TBROK | TERRNO, "fsmount() failed");
+		return;
+	}
+
+	fsmfd = TST_RET;
+	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] 33+ messages in thread

* [LTP] [PATCH V3 09/10] syscalls/fspick: New tests
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (7 preceding siblings ...)
  2020-02-25  6:40 ` [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests Viresh Kumar
@ 2020-02-25  6:40 ` Viresh Kumar
  2020-02-25  6:40 ` [LTP] [PATCH V3 10/10] syscalls/open_tree: " Viresh Kumar
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25  6:40 UTC (permalink / raw)
  To: ltp

Add tests to check working of fspick() syscall.

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/fspick01.c | 117 ++++++++++++++++++++
 testcases/kernel/syscalls/fspick/fspick02.c | 105 ++++++++++++++++++
 5 files changed, 233 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fspick/.gitignore
 create mode 100644 testcases/kernel/syscalls/fspick/Makefile
 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 36a1dcfadcca..b899cc76542d 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/fspick01.c b/testcases/kernel/syscalls/fspick/fspick01.c
new file mode 100644
index 000000000000..9d4b28a6a073
--- /dev/null
+++ b/testcases/kernel/syscalls/fspick/fspick01.c
@@ -0,0 +1,117 @@
+// 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"
+
+#define MNTPOINT	"mntpoint"
+
+#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 int ismounted;
+
+static void cleanup(void)
+{
+	if (ismounted)
+		SAFE_UMOUNT(MNTPOINT);
+}
+
+static void setup(void)
+{
+	int fd, fsmfd;
+
+	fsopen_supported_by_kernel();
+
+	TEST(fsopen(tst_device->fs_type, 0));
+	fd = TST_RET;
+
+	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(fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (TST_RET == -1)
+		tst_brk(TBROK | TERRNO, "fsmount() failed");
+
+	fsmfd = TST_RET;
+	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];
+	int fspick_fd;
+
+	TEST(fspick(AT_FDCWD, MNTPOINT, tc->flags));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "fspick() failed");
+		return;
+	}
+
+	fspick_fd = TST_RET;
+
+	TEST(fsconfig(fspick_fd, FSCONFIG_SET_STRING, "sync", "false", 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fspick_fd);
+		tst_res(TBROK | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	TEST(fsconfig(fspick_fd, FSCONFIG_SET_FLAG, "ro", NULL, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fspick_fd);
+		tst_res(TBROK | TERRNO, "fsconfig() failed");
+		return;
+	}
+
+	SAFE_CLOSE(fspick_fd);
+	tst_res(TPASS, "%s: fspick() passed", 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,
+};
diff --git a/testcases/kernel/syscalls/fspick/fspick02.c b/testcases/kernel/syscalls/fspick/fspick02.c
new file mode 100644
index 000000000000..c702140ec8f0
--- /dev/null
+++ b/testcases/kernel/syscalls/fspick/fspick02.c
@@ -0,0 +1,105 @@
+// 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"
+
+#define MNTPOINT	"mntpoint"
+
+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 int ismounted;
+
+static void cleanup(void)
+{
+	if (ismounted)
+		SAFE_UMOUNT(MNTPOINT);
+}
+
+static void setup(void)
+{
+	int fd, fsmfd;
+
+	fsopen_supported_by_kernel();
+
+	TEST(fsopen(tst_device->fs_type, 0));
+	fd = TST_RET;
+
+	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(fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (TST_RET == -1)
+		tst_brk(TBROK | TERRNO, "fsmount() failed");
+
+	fsmfd = TST_RET;
+
+	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(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);
+	} else if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: fspick() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+	} else {
+		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] 33+ messages in thread

* [LTP] [PATCH V3 10/10] syscalls/open_tree: New tests
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (8 preceding siblings ...)
  2020-02-25  6:40 ` [LTP] [PATCH V3 09/10] syscalls/fspick: " Viresh Kumar
@ 2020-02-25  6:40 ` Viresh Kumar
  2020-02-25 14:25   ` Petr Vorel
  2020-02-25 10:00 ` [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Li Wang
  2020-02-25 14:23 ` Petr Vorel
  11 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25  6:40 UTC (permalink / raw)
  To: ltp

Add tests to check working of open_tree() syscall.

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   | 124 ++++++++++++++++++
 .../kernel/syscalls/open_tree/open_tree02.c   | 104 +++++++++++++++
 5 files changed, 239 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 b899cc76542d..2bbbcc9ce0c3 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..ef603cc270a0
--- /dev/null
+++ b/testcases/kernel/syscalls/open_tree/open_tree01.c
@@ -0,0 +1,124 @@
+// 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 int dir_created;
+
+static void setup(void)
+{
+	fsopen_supported_by_kernel();
+
+	SAFE_MKDIR(OT_MNTPOINT, 0777);
+	dir_created = 1;
+}
+
+static void cleanup(void)
+{
+	if (dir_created)
+		SAFE_RMDIR(OT_MNTPOINT);
+}
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+	int fd, fsmfd, otfd;
+
+	TEST(fsopen(tst_device->fs_type, 0));
+	fd = TST_RET;
+
+	if (fd == -1) {
+		tst_res(TBROK | TERRNO, "fsopen() failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TBROK | TERRNO, "fsconfig failed");
+		return;
+	}
+
+	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
+	if (TST_RET == -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TBROK | TERRNO, "fsconfig failed");
+		return;
+	}
+
+	TEST(fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (TST_RET == -1) {
+		tst_res(TBROK | TERRNO, "fsmount() failed");
+		return;
+	}
+
+	fsmfd = TST_RET;
+
+	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");
+		return;
+	}
+
+	if (!tst_is_mounted(MNTPOINT)) {
+		tst_res(TBROK | TERRNO, "device not mounted");
+		goto out;
+	}
+
+	TEST(open_tree(AT_FDCWD, MNTPOINT, tc->flags | OPEN_TREE_CLONE));
+	if (TST_RET == -1) {
+		tst_res(TFAIL | TERRNO, "open_tree() failed");
+		goto out;
+	}
+
+	otfd = TST_RET;
+	TEST(move_mount(otfd, "", AT_FDCWD, OT_MNTPOINT,
+			MOVE_MOUNT_F_EMPTY_PATH));
+	SAFE_CLOSE(otfd);
+
+	if (TST_RET == -1) {
+		tst_res(TBROK | 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,
+	.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/open_tree/open_tree02.c b/testcases/kernel/syscalls/open_tree/open_tree02.c
new file mode 100644
index 000000000000..5d0ffcf4eb09
--- /dev/null
+++ b/testcases/kernel/syscalls/open_tree/open_tree02.c
@@ -0,0 +1,104 @@
+// 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(fsopen(tst_device->fs_type, 0));
+	fd = TST_RET;
+
+	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(fsmount(fd, 0, 0));
+	SAFE_CLOSE(fd);
+
+	if (TST_RET == -1)
+		tst_brk(TBROK | TERRNO, "fsmount() failed");
+
+	fsmfd = TST_RET;
+	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);
+	} else if (tc->exp_errno != TST_ERR) {
+		tst_res(TFAIL | TTERRNO, "%s: open_tree() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+	} else {
+		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] 33+ messages in thread

* [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (9 preceding siblings ...)
  2020-02-25  6:40 ` [LTP] [PATCH V3 10/10] syscalls/open_tree: " Viresh Kumar
@ 2020-02-25 10:00 ` Li Wang
  2020-02-25 10:32   ` Petr Vorel
  2020-02-25 10:48   ` Viresh Kumar
  2020-02-25 14:23 ` Petr Vorel
  11 siblings, 2 replies; 33+ messages in thread
From: Li Wang @ 2020-02-25 10:00 UTC (permalink / raw)
  To: ltp

Hi Viresh,

These new tests look good, only a few comments/questions below:

Patch 1/10:
1. The git summary should be updated too (someone who push patch can help
do that:).
2. Maybe better to replace the TWARN by TINFO? Since tst_is_mounted() as a
general function to check if mount success, sometimes we just need the
return status then do next work(I tend to leave the waring or break operate
to LTP users:).

Patch 5/10, 9/10:
May I ask why we use "sync" as the key value in fsconfig()? I ask this
because it can get rid of the errors we found in XFS test before.

Patch 9/10, 10/10:
I guess that'd be better if we put the 'ismounted = 1' at the behind of
tst_is_mounted(), do you feel the code sequence looks strange which we set
'ismounted' to 1 then do mount checking?

Ack for the whole patchset v3 (+ follow some modification for above
comments):
    Acked-by: Li Wang <liwang@redhat.com>

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

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

* [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls
  2020-02-25 10:00 ` [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Li Wang
@ 2020-02-25 10:32   ` Petr Vorel
  2020-02-25 10:48   ` Viresh Kumar
  1 sibling, 0 replies; 33+ messages in thread
From: Petr Vorel @ 2020-02-25 10:32 UTC (permalink / raw)
  To: ltp

Hi,

> Patch 5/10, 9/10:
> May I ask why we use "sync" as the key value in fsconfig()? I ask this
> because it can get rid of the errors we found in XFS test before.
I guess "sync" is valid option (see FILESYSTEM-INDEPENDENT MOUNT OPTIONS
in mount(8)), but "foo" obviously not.

> Patch 9/10, 10/10:
> I guess that'd be better if we put the 'ismounted = 1' at the behind of
> tst_is_mounted(), do you feel the code sequence looks strange which we set
> 'ismounted' to 1 then do mount checking?
+1.

Kind regards,
Petr

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

* [LTP] [PATCH V4 1/10] tst_device: Add tst_is_mounted() helper
  2020-02-25  6:40 ` [LTP] [PATCH V3 01/10] tst_device: Add tst_ismount() helper Viresh Kumar
@ 2020-02-25 10:39   ` Viresh Kumar
  2020-02-26  5:14     ` Zorro Lang
  0 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25 10:39 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>
---
V3->V4:
- s/TWARN/TINFO
- Fix commit log.

 include/tst_device.h                          |  6 +++++
 lib/tst_device.c                              | 23 +++++++++++++++++
 testcases/kernel/syscalls/fsmount/fsmount01.c | 25 +------------------
 3 files changed, 30 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..d99fb8bc554a 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -386,6 +386,29 @@ int tst_umount(const char *path)
 	return -1;
 }
 
+int tst_is_mounted(const char *path)
+{
+	char line[256];
+	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 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] 33+ messages in thread

* [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls
  2020-02-25 10:00 ` [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Li Wang
  2020-02-25 10:32   ` Petr Vorel
@ 2020-02-25 10:48   ` Viresh Kumar
  1 sibling, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2020-02-25 10:48 UTC (permalink / raw)
  To: ltp

On 25-02-20, 18:00, Li Wang wrote:
> Hi Viresh,
> 
> These new tests look good, only a few comments/questions below:
> 
> Patch 1/10:
> 1. The git summary should be updated too (someone who push patch can help
> do that:).
> 2. Maybe better to replace the TWARN by TINFO? Since tst_is_mounted() as a
> general function to check if mount success, sometimes we just need the
> return status then do next work(I tend to leave the waring or break operate
> to LTP users:).

Done and sent an update as well.

> Patch 5/10, 9/10:
> May I ask why we use "sync" as the key value in fsconfig()? I ask this
> because it can get rid of the errors we found in XFS test before.

I wasn't able to test it earlier during V2 on my ARM platform and so
just added "foo" there.

This time, I tried running it on my x86 machine and so was able to run
xfs tests and so I figured out what needs to be passed by looking out
the Linux source.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/fs_context.c#n41

The first two arrays have the valid options that can be passed.

> Patch 9/10, 10/10:
> I guess that'd be better if we put the 'ismounted = 1' at the behind of
> tst_is_mounted(), do you feel the code sequence looks strange which we set
> 'ismounted' to 1 then do mount checking?

I understand the weirdness you are talking about, but I think the code
does what the right thing to do at this point is.

The question is, what are we supposed to do when move_mount() passes,
but we aren't able to see the mount in /proc/mounts ? I think, we
should call umount() anyway as move_mount() passed. And that's why I
have kept the ismounted = 1, line before the tst_is_mounted() call.

> Ack for the whole patchset v3 (+ follow some modification for above
> comments):
>     Acked-by: Li Wang <liwang@redhat.com>

Thanks for your reviews Li.

-- 
viresh

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

* [LTP] [PATCH V3 05/10] syscalls/fsconfig: New tests
  2020-02-25  6:40 ` [LTP] [PATCH V3 05/10] syscalls/fsconfig: " Viresh Kumar
@ 2020-02-25 13:46   ` Petr Vorel
  2020-02-27  4:54     ` Viresh Kumar
  0 siblings, 1 reply; 33+ messages in thread
From: Petr Vorel @ 2020-02-25 13:46 UTC (permalink / raw)
  To: ltp

Hi,

> Add tests to check working of fsconfig() syscall.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

LGTM, minor comments below.
...
> +++ b/testcases/kernel/syscalls/fsconfig/fsconfig01.c
> @@ -0,0 +1,104 @@
> +// 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(fsopen(tst_device->fs_type, 0));
> +	fd = TST_RET;
Just
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(TINFO, "fsconfig(): FSCONFIG_SET_PATH not supported");
This should be TCONF. It'd be nice to have this check in functions in lapi/fsmount.h
(just DRY). But don't want to block these tests just because DRY.

> +		} 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(TINFO, "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(TINFO, "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(fsmount(fd, 0, 0));
> +	if (TST_RET == -1) {
> +		tst_res(TBROK | TERRNO, "fsmount() failed");
> +		goto out;
> +	}
> +
> +	fsmfd = TST_RET;
> +	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..d51a869ac3ff
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fsconfig/fsconfig02.c
> @@ -0,0 +1,98 @@
> +// 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"
> +
> +int fd = -1, temp_fd = -1, invalid_fd = -1;
> +int aux_0 = 0, aux_1 = 1, aux_fdcwd = AT_FDCWD, aux_minus1 = -1;
These 2 should be static (also fd could be default 0, but who cares :)).

Kind regards,
Petr

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

* [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests
  2020-02-25  6:40 ` [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests Viresh Kumar
@ 2020-02-25 13:57   ` Petr Vorel
  2020-02-26  2:27     ` Viresh Kumar
  0 siblings, 1 reply; 33+ messages in thread
From: Petr Vorel @ 2020-02-25 13:57 UTC (permalink / raw)
  To: ltp

Hi,

> +	TEST(fsopen(tst_device->fs_type, 0));
> +	fd = TST_RET;
Again:
TEST(fd = fsopen(tst_device->fs_type, 0));

> +
> +	if (fd == -1) {
> +		tst_res(TBROK | TERRNO, "fsopen() failed");
> +		return;
> +	}
> +
> +	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
> +	if (TST_RET == -1) {
> +		SAFE_CLOSE(fd);
> +		tst_res(TBROK | TERRNO, "fsconfig() failed");
These should be TFAIL otherwise it 1) breaks all tests 2) does not report any
result:

move_mount02.c:37: BROK: fsopen() failed: SUCCESS (0)
tst_test.c:1051: BROK: Test 0 haven't reported results!

I'll send diff with these small fixes and if anybody complains, we could merge
whole patchset.

Kind regards,
Petr

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

* [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls
  2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
                   ` (10 preceding siblings ...)
  2020-02-25 10:00 ` [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Li Wang
@ 2020-02-25 14:23 ` Petr Vorel
  11 siblings, 0 replies; 33+ messages in thread
From: Petr Vorel @ 2020-02-25 14:23 UTC (permalink / raw)
  To: ltp

Hi,

I addressed some of my concerns here (+ add fspick.h to remove a bit of the duplicity):
https://github.com/pevik/ltp/tree/Viresh_Kumar/fsmount.v3.fixes

LGTM, but I'd like to have more look.
I'd also appreciate somebody else to have look.

Kind regards,
Petr

diff --git lib/tst_device.c lib/tst_device.c
index c53ae36cf..d99fb8bc5 100644
--- lib/tst_device.c
+++ lib/tst_device.c
@@ -404,7 +404,7 @@ int tst_is_mounted(const char *path)
 	SAFE_FCLOSE(NULL, file);
 
 	if (!ret)
-		tst_resm(TWARN, "No device is mounted at %s", path);
+		tst_resm(TINFO, "No device is mounted at %s", path);
 
 	return ret;
 }
diff --git testcases/kernel/syscalls/fsconfig/fsconfig02.c testcases/kernel/syscalls/fsconfig/fsconfig02.c
index d51a869ac..c3722691a 100644
--- testcases/kernel/syscalls/fsconfig/fsconfig02.c
+++ testcases/kernel/syscalls/fsconfig/fsconfig02.c
@@ -7,8 +7,8 @@
 #include "tst_test.h"
 #include "lapi/fsmount.h"
 
-int fd = -1, temp_fd = -1, invalid_fd = -1;
-int aux_0 = 0, aux_1 = 1, aux_fdcwd = AT_FDCWD, aux_minus1 = -1;
+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;
@@ -51,9 +51,7 @@ static void setup(void)
 {
 	fsopen_supported_by_kernel();
 
-	TEST(fsopen(tst_device->fs_type, 0));
-	fd = TST_RET;
-
+	TEST(fd = fsopen(tst_device->fs_type, 0));
 	if (fd == -1)
 		tst_brk(TBROK | TERRNO, "fsopen() failed");
 
diff --git testcases/kernel/syscalls/fsmount/fsmount01.c testcases/kernel/syscalls/fsmount/fsmount01.c
index b7810df2f..6774a43ff 100644
--- testcases/kernel/syscalls/fsmount/fsmount01.c
+++ testcases/kernel/syscalls/fsmount/fsmount01.c
@@ -41,13 +41,12 @@ static void run(unsigned int n)
 	struct tcase *tc = &tcases[n];
 	int sfd, mfd;
 
-	TEST(fsopen(tst_device->fs_type, FSOPEN_CLOEXEC));
-	if (TST_RET == -1) {
+	TEST(sfd = fsopen(tst_device->fs_type, FSOPEN_CLOEXEC));
+	if (sfd == -1) {
 		tst_res(TBROK | TTERRNO, "fsopen() on %s failed",
 			tst_device->fs_type);
 		return;
 	}
-	sfd = TST_RET;
 
 	TEST(fsconfig(sfd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
 	if (TST_RET < 0) {
diff --git testcases/kernel/syscalls/fsmount/fsmount02.c testcases/kernel/syscalls/fsmount/fsmount02.c
index c61caa8e8..90dbff1fd 100644
--- testcases/kernel/syscalls/fsmount/fsmount02.c
+++ testcases/kernel/syscalls/fsmount/fsmount02.c
@@ -33,9 +33,7 @@ static void setup(void)
 {
 	fsopen_supported_by_kernel();
 
-	TEST(fsopen(tst_device->fs_type, 0));
-	fd = TST_RET;
-
+	TEST(fd = fsopen(tst_device->fs_type, 0));
 	if (fd == -1)
 		tst_brk(TBROK | TERRNO, "fsopen() failed");
 
diff --git testcases/kernel/syscalls/fsopen/fsopen01.c testcases/kernel/syscalls/fsopen/fsopen01.c
index 51fab25d9..84cb38fcf 100644
--- testcases/kernel/syscalls/fsopen/fsopen01.c
+++ testcases/kernel/syscalls/fsopen/fsopen01.c
@@ -25,9 +25,7 @@ static void run(unsigned int n)
 	struct tcase *tc = &tcases[n];
 	int fd, fsmfd;
 
-	TEST(fsopen(tst_device->fs_type, tc->flags));
-	fd = TST_RET;
-
+	TEST(fd = fsopen(tst_device->fs_type, tc->flags));
 	if (fd == -1) {
 		tst_res(TFAIL | TERRNO, "fsopen() failed");
 		return;
diff --git testcases/kernel/syscalls/fspick/fspick.h testcases/kernel/syscalls/fspick/fspick.h
new file mode 100644
index 000000000..8d42eff05
--- /dev/null
+++ 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 (TST_RET == -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 testcases/kernel/syscalls/fspick/fspick01.c testcases/kernel/syscalls/fspick/fspick01.c
index 9d4b28a6a..217cb1f12 100644
--- testcases/kernel/syscalls/fspick/fspick01.c
+++ testcases/kernel/syscalls/fspick/fspick01.c
@@ -6,8 +6,7 @@
  */
 #include "tst_test.h"
 #include "lapi/fsmount.h"
-
-#define MNTPOINT	"mntpoint"
+#include "fspick.h"
 
 #define TCASE_ENTRY(_flags)	{.name = "Flag " #_flags, .flags = _flags}
 
@@ -21,58 +20,6 @@ static struct tcase {
 	TCASE_ENTRY(FSPICK_EMPTY_PATH),
 };
 
-static int ismounted;
-
-static void cleanup(void)
-{
-	if (ismounted)
-		SAFE_UMOUNT(MNTPOINT);
-}
-
-static void setup(void)
-{
-	int fd, fsmfd;
-
-	fsopen_supported_by_kernel();
-
-	TEST(fsopen(tst_device->fs_type, 0));
-	fd = TST_RET;
-
-	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(fsmount(fd, 0, 0));
-	SAFE_CLOSE(fd);
-
-	if (TST_RET == -1)
-		tst_brk(TBROK | TERRNO, "fsmount() failed");
-
-	fsmfd = TST_RET;
-	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];
diff --git testcases/kernel/syscalls/fspick/fspick02.c testcases/kernel/syscalls/fspick/fspick02.c
index c702140ec..dc552c754 100644
--- testcases/kernel/syscalls/fspick/fspick02.c
+++ testcases/kernel/syscalls/fspick/fspick02.c
@@ -6,8 +6,7 @@
  */
 #include "tst_test.h"
 #include "lapi/fsmount.h"
-
-#define MNTPOINT	"mntpoint"
+#include "fspick.h"
 
 static struct tcase {
 	char *name;
@@ -21,59 +20,6 @@ static struct tcase {
 	{"invalid-flags", AT_FDCWD, MNTPOINT, 0x10, 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(fsopen(tst_device->fs_type, 0));
-	fd = TST_RET;
-
-	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(fsmount(fd, 0, 0));
-	SAFE_CLOSE(fd);
-
-	if (TST_RET == -1)
-		tst_brk(TBROK | TERRNO, "fsmount() failed");
-
-	fsmfd = TST_RET;
-
-	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];
diff --git testcases/kernel/syscalls/move_mount/move_mount01.c testcases/kernel/syscalls/move_mount/move_mount01.c
index aef0e15e1..b5513dd18 100644
--- testcases/kernel/syscalls/move_mount/move_mount01.c
+++ testcases/kernel/syscalls/move_mount/move_mount01.c
@@ -28,25 +28,23 @@ static void run(unsigned int n)
 	struct tcase *tc = &tcases[n];
 	int fsmfd, fd;
 
-	TEST(fsopen(tst_device->fs_type, 0));
-	fd = TST_RET;
-
+	TEST(fd = fsopen(tst_device->fs_type, 0));
 	if (fd == -1) {
-		tst_res(TBROK | TERRNO, "fsopen() failed");
+		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(TBROK | TERRNO, "fsconfig() failed");
+		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(TBROK | TERRNO, "fsconfig() failed");
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
 		return;
 	}
 
@@ -54,7 +52,7 @@ static void run(unsigned int n)
 	SAFE_CLOSE(fd);
 
 	if (TST_RET == -1) {
-		tst_res(TBROK | TERRNO, "fsmount() failed");
+		tst_res(TFAIL | TERRNO, "fsmount() failed");
 		return;
 	}
 
diff --git testcases/kernel/syscalls/move_mount/move_mount02.c testcases/kernel/syscalls/move_mount/move_mount02.c
index 98b7be77f..d85381e90 100644
--- testcases/kernel/syscalls/move_mount/move_mount02.c
+++ testcases/kernel/syscalls/move_mount/move_mount02.c
@@ -32,25 +32,23 @@ static void run(unsigned int n)
 	struct tcase *tc = &tcases[n];
 	int fd;
 
-	TEST(fsopen(tst_device->fs_type, 0));
-	fd = TST_RET;
-
+	TEST(fd = fsopen(tst_device->fs_type, 0));
 	if (fd == -1) {
-		tst_res(TBROK | TERRNO, "fsopen() failed");
+		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(TBROK | TERRNO, "fsconfig() failed");
+		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(TBROK | TERRNO, "fsconfig() failed");
+		tst_res(TFAIL | TERRNO, "fsconfig() failed");
 		return;
 	}
 
@@ -58,7 +56,7 @@ static void run(unsigned int n)
 	SAFE_CLOSE(fd);
 
 	if (TST_RET == -1) {
-		tst_res(TBROK | TERRNO, "fsmount() failed");
+		tst_res(TFAIL | TERRNO, "fsmount() failed");
 		return;
 	}
 
diff --git testcases/kernel/syscalls/open_tree/open_tree01.c testcases/kernel/syscalls/open_tree/open_tree01.c
index ef603cc27..191d3196d 100644
--- testcases/kernel/syscalls/open_tree/open_tree01.c
+++ testcases/kernel/syscalls/open_tree/open_tree01.c
@@ -52,18 +52,18 @@ static void run(unsigned int n)
 	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
 	if (TST_RET == -1) {
 		SAFE_CLOSE(fd);
-		tst_res(TBROK | TERRNO, "fsconfig failed");
+		tst_res(TBROK | TERRNO, "fsconfig() failed");
 		return;
 	}
 
 	TEST(fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0));
 	if (TST_RET == -1) {
 		SAFE_CLOSE(fd);
-		tst_res(TBROK | TERRNO, "fsconfig failed");
+		tst_res(TBROK | TERRNO, "fsconfig() failed");
 		return;
 	}
 
-	TEST(fsmount(fd, 0, 0));
+	TEST(fsmfd = fsmount(fd, 0, 0));
 	SAFE_CLOSE(fd);
 
 	if (TST_RET == -1) {
@@ -71,8 +71,6 @@ static void run(unsigned int n)
 		return;
 	}
 
-	fsmfd = TST_RET;
-
 	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
 			MOVE_MOUNT_F_EMPTY_PATH));
 	SAFE_CLOSE(fsmfd);
diff --git testcases/kernel/syscalls/open_tree/open_tree02.c testcases/kernel/syscalls/open_tree/open_tree02.c
index 5d0ffcf4e..2823531cb 100644
--- testcases/kernel/syscalls/open_tree/open_tree02.c
+++ testcases/kernel/syscalls/open_tree/open_tree02.c
@@ -44,22 +44,21 @@ static void setup(void)
 	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
 	if (TST_RET == -1) {
 		SAFE_CLOSE(fd);
-		tst_brk(TBROK | TERRNO, "fsconfig failed");
+		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");
+		tst_brk(TBROK | TERRNO, "fsconfig() failed");
 	}
 
-	TEST(fsmount(fd, 0, 0));
+	TEST(fsmfd = fsmount(fd, 0, 0));
 	SAFE_CLOSE(fd);
 
 	if (TST_RET == -1)
 		tst_brk(TBROK | TERRNO, "fsmount() failed");
 
-	fsmfd = TST_RET;
 	TEST(move_mount(fsmfd, "", AT_FDCWD, MNTPOINT,
 			MOVE_MOUNT_F_EMPTY_PATH));
 	SAFE_CLOSE(fsmfd);

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

* [LTP] [PATCH V3 10/10] syscalls/open_tree: New tests
  2020-02-25  6:40 ` [LTP] [PATCH V3 10/10] syscalls/open_tree: " Viresh Kumar
@ 2020-02-25 14:25   ` Petr Vorel
  0 siblings, 0 replies; 33+ messages in thread
From: Petr Vorel @ 2020-02-25 14:25 UTC (permalink / raw)
  To: ltp

Hi,

> +++ b/testcases/kernel/syscalls/open_tree/open_tree01.c
...
> +static void cleanup(void)
> +{
> +	if (dir_created)
> +		SAFE_RMDIR(OT_MNTPOINT);
> +}
BTW: This might not be needed (whole tmp directory gets deleted after test exits by
the library).

Kind regards,
Petr

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

* [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests
  2020-02-25 13:57   ` Petr Vorel
@ 2020-02-26  2:27     ` Viresh Kumar
  2020-02-26  3:34       ` Yang Xu
  2020-02-26  7:47       ` Petr Vorel
  0 siblings, 2 replies; 33+ messages in thread
From: Viresh Kumar @ 2020-02-26  2:27 UTC (permalink / raw)
  To: ltp

On 25-02-20, 14:57, Petr Vorel wrote:
> > +	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
> > +	if (TST_RET == -1) {
> > +		SAFE_CLOSE(fd);
> > +		tst_res(TBROK | TERRNO, "fsconfig() failed");
> These should be TFAIL otherwise it 1) breaks all tests 2) does not report any
> result:
> 
> move_mount02.c:37: BROK: fsopen() failed: SUCCESS (0)
> tst_test.c:1051: BROK: Test 0 haven't reported results!

I am a bit confused about TBROK and TFAIL to be honest. The test
writing guideline says this:

| 'TFAIL' | Test has failed.
| 'TBROK' | Something has failed in test preparation phase.

And so in my code I have been using TFAIL only for the failures for the
actual syscalls that we are testing, like move_mount here. And I have
been using TBROK for pretty much everything else.

Would be good if you and Cyril can explain what's the correct usage
model for these is.

-- 
viresh

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

* [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests
  2020-02-26  2:27     ` Viresh Kumar
@ 2020-02-26  3:34       ` Yang Xu
  2020-02-26  4:34         ` Viresh Kumar
  2020-02-26  7:47       ` Petr Vorel
  1 sibling, 1 reply; 33+ messages in thread
From: Yang Xu @ 2020-02-26  3:34 UTC (permalink / raw)
  To: ltp

Hi

> On 25-02-20, 14:57, Petr Vorel wrote:
>>> +	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
>>> +	if (TST_RET == -1) {
>>> +		SAFE_CLOSE(fd);
>>> +		tst_res(TBROK | TERRNO, "fsconfig() failed");
>> These should be TFAIL otherwise it 1) breaks all tests 2) does not report any
>> result:
>>
>> move_mount02.c:37: BROK: fsopen() failed: SUCCESS (0)
>> tst_test.c:1051: BROK: Test 0 haven't reported results!
> 
> I am a bit confused about TBROK and TFAIL to be honest. The test
> writing guideline says this:
> 
> | 'TFAIL' | Test has failed.
> | 'TBROK' | Something has failed in test preparation phase.
> 
> And so in my code I have been using TFAIL only for the failures for the
> actual syscalls that we are testing, like move_mount here. And I have
> been using TBROK for pretty much everything else
If using TBROK, it will terminate the program. So how can test the 
remaining test?
Like move_mount02.c, if test "invalid-from-fd"and fsconfig failed, it 
will exit. So how do you test "invalid-from-path"?

Also, using return after TBROK is meaningless.

Best Regards
Yang Xu
> 
> Would be good if you and Cyril can explain what's the correct usage
> model for these is.
> 




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

* [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests
  2020-02-26  3:34       ` Yang Xu
@ 2020-02-26  4:34         ` Viresh Kumar
  0 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2020-02-26  4:34 UTC (permalink / raw)
  To: ltp

On 26-02-20, 11:34, Yang Xu wrote:
> If using TBROK, it will terminate the program. So how can test the remaining
> test?
> Like move_mount02.c, if test "invalid-from-fd"and fsconfig failed, it will
> exit. So how do you test "invalid-from-path"?
> 
> Also, using return after TBROK is meaningless.

I think one of us has surely misunderstood it all :)

What my understanding says is that if you use tst_brk(), then it
breaks the test and returns early. TBROK or TFAIL, both can be used in
tst_brk() or tst_ret() and they only imply what has failed, the test
or some preparatory thing.

-- 
viresh

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

* [LTP] [PATCH V4 1/10] tst_device: Add tst_is_mounted() helper
  2020-02-25 10:39   ` [LTP] [PATCH V4 1/10] tst_device: Add tst_is_mounted() helper Viresh Kumar
@ 2020-02-26  5:14     ` Zorro Lang
  2020-02-26  5:58       ` Yang Xu
  2020-02-26  6:28       ` Viresh Kumar
  0 siblings, 2 replies; 33+ messages in thread
From: Zorro Lang @ 2020-02-26  5:14 UTC (permalink / raw)
  To: ltp

On Tue, Feb 25, 2020 at 04:09:31PM +0530, Viresh Kumar wrote:
> 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>
> ---
> V3->V4:
> - s/TWARN/TINFO
> - Fix commit log.
> 
>  include/tst_device.h                          |  6 +++++
>  lib/tst_device.c                              | 23 +++++++++++++++++
>  testcases/kernel/syscalls/fsmount/fsmount01.c | 25 +------------------
>  3 files changed, 30 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..d99fb8bc554a 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -386,6 +386,29 @@ int tst_umount(const char *path)
>  	return -1;
>  }
>  
> +int tst_is_mounted(const char *path)
> +{
> +	char line[256];
> +	FILE *file;
> +	int ret = 0;
> +
> +	file = SAFE_FOPEN(NULL, "/proc/mounts", "r");
> +
> +	while (fgets(line, sizeof(line), file)) {
> +		if (strstr(line, path) != NULL) {

This code moving is fine. But if we'd like to make this function to be common
function, we'd better think more about that. I think the current code logic
isn't so well.

For example, if path is "/mnt/test", and we get a line as "/mnt/test/mnt1 ..."
or "/mnt/mnt/test", Then this function thinks "/mnt/test" is a mountpoint.

We can refer to util-linux/sys-utils/mountpoint.c a little, but it's complicated,
or we can call mountpoint command directly?

> +			ret = 1;
> +			break;
> +		}
> +	}
> +
> +	SAFE_FCLOSE(NULL, file);
> +
> +	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	[flat|nested] 33+ messages in thread

* [LTP] [PATCH V3 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel()
  2020-02-25  6:40 ` [LTP] [PATCH V3 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel() Viresh Kumar
@ 2020-02-26  5:51   ` Zorro Lang
  0 siblings, 0 replies; 33+ messages in thread
From: Zorro Lang @ 2020-02-26  5:51 UTC (permalink / raw)
  To: ltp

On Tue, Feb 25, 2020 at 12:10:40PM +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>
> 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)

Looks good to me.

> +{
> +	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	[flat|nested] 33+ messages in thread

* [LTP] [PATCH V3 03/10] lapi/fsmount.h: Include "lapi/fcntl.h"
  2020-02-25  6:40 ` [LTP] [PATCH V3 03/10] lapi/fsmount.h: Include "lapi/fcntl.h" Viresh Kumar
@ 2020-02-26  5:51   ` Zorro Lang
  0 siblings, 0 replies; 33+ messages in thread
From: Zorro Lang @ 2020-02-26  5:51 UTC (permalink / raw)
  To: ltp

On Tue, Feb 25, 2020 at 12:10:41PM +0530, Viresh Kumar wrote:
> 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>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---

Good to me

>  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	[flat|nested] 33+ messages in thread

* [LTP] [PATCH V4 1/10] tst_device: Add tst_is_mounted() helper
  2020-02-26  5:14     ` Zorro Lang
@ 2020-02-26  5:58       ` Yang Xu
  2020-02-26  6:28       ` Viresh Kumar
  1 sibling, 0 replies; 33+ messages in thread
From: Yang Xu @ 2020-02-26  5:58 UTC (permalink / raw)
  To: ltp

Hi


> On Tue, Feb 25, 2020 at 04:09:31PM +0530, Viresh Kumar wrote:
>> 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>
>> ---
>> V3->V4:
>> - s/TWARN/TINFO
>> - Fix commit log.
>>
>>   include/tst_device.h                          |  6 +++++
>>   lib/tst_device.c                              | 23 +++++++++++++++++
>>   testcases/kernel/syscalls/fsmount/fsmount01.c | 25 +------------------
>>   3 files changed, 30 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..d99fb8bc554a 100644
>> --- a/lib/tst_device.c
>> +++ b/lib/tst_device.c
>> @@ -386,6 +386,29 @@ int tst_umount(const char *path)
>>   	return -1;
>>   }
>>   
>> +int tst_is_mounted(const char *path)
>> +{
>> +	char line[256];
>> +	FILE *file;
>> +	int ret = 0;
>> +
>> +	file = SAFE_FOPEN(NULL, "/proc/mounts", "r");
>> +
>> +	while (fgets(line, sizeof(line), file)) {
>> +		if (strstr(line, path) != NULL) {
> 
> This code moving is fine. But if we'd like to make this function to be common
> function, we'd better think more about that. I think the current code logic
> isn't so well.
> 
> For example, if path is "/mnt/test", and we get a line as "/mnt/test/mnt1 ..."
> or "/mnt/mnt/test", Then this function thinks "/mnt/test" is a mountpoint.
> 
> We can refer to util-linux/sys-utils/mountpoint.c a little, but it's complicated,
> or we can call mountpoint command directly?
> 
I think we can use a fixed format to extract it like kernel code
tools/lib/api/fs/fs.c
static bool fs__read_mounts(struct fs *fs)
{
         bool found = false;
         char type[100];
         FILE *fp;

         fp = fopen("/proc/mounts", "r");
         if (fp == NULL)
                 return NULL;

         while (!found &&
                fscanf(fp, "%*s %" STR(PATH_MAX) "s %99s %*s %*d %*d\n",
                       fs->path, type) == 2) {

                 if (strcmp(type, fs->name) == 0)
                         found = true;
         }

         fclose(fp);
         return fs->found = found;
}

But this way maybe wrong if kernel updates mount info format in future.

Best Regards
Yang Xu
>> +			ret = 1;
>> +			break;
>> +		}
>> +	}
>> +
>> +	SAFE_FCLOSE(NULL, file);
>> +
>> +	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	[flat|nested] 33+ messages in thread

* [LTP] [PATCH V4 1/10] tst_device: Add tst_is_mounted() helper
  2020-02-26  5:14     ` Zorro Lang
  2020-02-26  5:58       ` Yang Xu
@ 2020-02-26  6:28       ` Viresh Kumar
  1 sibling, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2020-02-26  6:28 UTC (permalink / raw)
  To: ltp

On 26-02-20, 13:14, Zorro Lang wrote:
> For example, if path is "/mnt/test", and we get a line as "/mnt/test/mnt1 ..."
> or "/mnt/mnt/test", Then this function thinks "/mnt/test" is a mountpoint.
> 
> We can refer to util-linux/sys-utils/mountpoint.c a little, but it's complicated,
> or we can call mountpoint command directly?

This is working fine for me, does this looks okay now ?

diff --git a/lib/tst_device.c b/lib/tst_device.c
index d99fb8bc554a..8bf6dacf5973 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -388,25 +388,16 @@ int tst_umount(const char *path)
 
 int tst_is_mounted(const char *path)
 {
-       char line[256];
-       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;
-               }
-       }
+       char cmd[PATH_MAX];
+       int ret;
 
-       SAFE_FCLOSE(NULL, file);
+       snprintf(cmd, sizeof(cmd), "mountpoint -q %s", path);
+       ret = tst_system(cmd);
 
-       if (!ret)
+       if (ret)
                tst_resm(TINFO, "No device is mounted at %s", path);
 
-       return ret;
+       return !ret;
 }

-- 
viresh

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

* [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests
  2020-02-26  2:27     ` Viresh Kumar
  2020-02-26  3:34       ` Yang Xu
@ 2020-02-26  7:47       ` Petr Vorel
  2020-02-26  8:23         ` Viresh Kumar
  1 sibling, 1 reply; 33+ messages in thread
From: Petr Vorel @ 2020-02-26  7:47 UTC (permalink / raw)
  To: ltp

Hi Viresh,

> On 25-02-20, 14:57, Petr Vorel wrote:
> > > +	TEST(fsconfig(fd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0));
> > > +	if (TST_RET == -1) {
> > > +		SAFE_CLOSE(fd);
> > > +		tst_res(TBROK | TERRNO, "fsconfig() failed");
> > These should be TFAIL otherwise it 1) breaks all tests 2) does not report any
> > result:

> > move_mount02.c:37: BROK: fsopen() failed: SUCCESS (0)
> > tst_test.c:1051: BROK: Test 0 haven't reported results!

> I am a bit confused about TBROK and TFAIL to be honest. The test
> writing guideline says this:

> | 'TFAIL' | Test has failed.
> | 'TBROK' | Something has failed in test preparation phase.

> And so in my code I have been using TFAIL only for the failures for the
> actual syscalls that we are testing, like move_mount here. And I have
> been using TBROK for pretty much everything else.
Your idea is correct, but IMHO it's not good to skip all the tests, which is
done due
tst_test.c:1051: BROK: Test 0 haven't reported results!
if you use tst_res(TBROK ...).

You can use tst_brk(TBROK) to avoid no reported results, but that's obviously
exit the test either.

tst_brk(TBROK) is used for setup, where you create some resource, which is then
reused by all test runs, but this preparation fails.

NOTE: There are some tests which are using tst_res(TBROK). At least some of them are
wrong. IMHO in testcases/kernel/syscalls/request_key/request_key04.c
	TST_ERR = saved_errno;
	if (TST_ERR == EACCES) {
		tst_res(TPASS, "request_key() failed with EACCES as expected");
	} else {
		tst_res(TBROK | TTERRNO,
			"request_key() failed with unexpected error code");
	}

IMHO it should be
	TST_ERR = saved_errno;
	if (TST_ERR == EACCES) {
		tst_res(TPASS, "request_key() failed with EACCES as expected");
	} else {
		tst_res(TFAIL | TTERRNO,
			"request_key() failed with unexpected error code");
	}
Otherwise if it fails at unexpected error code, you get:
request_key04.c:68: BROK: request_key() failed with unexpected error code: EACCES (13)
tst_test.c:1036: BROK: Test haven't reported results!

Thus I'd try to avoid TBROK with tst_res().

> Would be good if you and Cyril can explain what's the correct usage
> model for these is.

Kind regards,
Petr

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

* [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests
  2020-02-26  7:47       ` Petr Vorel
@ 2020-02-26  8:23         ` Viresh Kumar
  2020-02-26  8:53           ` Petr Vorel
  0 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2020-02-26  8:23 UTC (permalink / raw)
  To: ltp

On 26-02-20, 08:47, Petr Vorel wrote:
> Your idea is correct, but IMHO it's not good to skip all the tests, which is
> done due
> tst_test.c:1051: BROK: Test 0 haven't reported results!
> if you use tst_res(TBROK ...).

I don't think that is the case. tst_res(TBROK, ...) shouldn't (and
isn't for me) result in skipping of tests.

> tst_brk(TBROK) is used for setup, where you create some resource, which is then
> reused by all test runs, but this preparation fails.

Right, but in my case I can't put all setup stuff in setup() callback
and some of the setup bits stay in run() callback as well.

-- 
viresh

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

* [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests
  2020-02-26  8:23         ` Viresh Kumar
@ 2020-02-26  8:53           ` Petr Vorel
  2020-02-26  8:59             ` Viresh Kumar
  0 siblings, 1 reply; 33+ messages in thread
From: Petr Vorel @ 2020-02-26  8:53 UTC (permalink / raw)
  To: ltp

Hi Viresh,

> On 26-02-20, 08:47, Petr Vorel wrote:
> > Your idea is correct, but IMHO it's not good to skip all the tests, which is
> > done due
> > tst_test.c:1051: BROK: Test 0 haven't reported results!
> > if you use tst_res(TBROK ...).

> I don't think that is the case. tst_res(TBROK, ...) shouldn't (and
> isn't for me) result in skipping of tests.
Correct, tst_res() itself doesn't exit the test. But the fact it uses TBROK
without reporting any result previously leads to tst_brk from the library:

tst_test.c:1036: BROK: Test haven't reported results!

tst_brk(TBROK, "Test %i haven't reported results!", i);

That's why I consider tst_res(TBROK problematic.

This is a separate discussion, see the patch I sent today
https://patchwork.ozlabs.org/patch/1244781/

Kind regards,
Petr

> > tst_brk(TBROK) is used for setup, where you create some resource, which is then
> > reused by all test runs, but this preparation fails.

> Right, but in my case I can't put all setup stuff in setup() callback
> and some of the setup bits stay in run() callback as well.
Sure. But as a result of it I'd personally use tst_res(TFAIL).
But maybe I'm wrong and others will correct me.

Kind regards,
Petr

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

* [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests
  2020-02-26  8:53           ` Petr Vorel
@ 2020-02-26  8:59             ` Viresh Kumar
  2020-02-26  9:20               ` Petr Vorel
  0 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2020-02-26  8:59 UTC (permalink / raw)
  To: ltp

On 26-02-20, 09:53, Petr Vorel wrote:
> Hi Viresh,
> 
> > On 26-02-20, 08:47, Petr Vorel wrote:
> > > Your idea is correct, but IMHO it's not good to skip all the tests, which is
> > > done due
> > > tst_test.c:1051: BROK: Test 0 haven't reported results!
> > > if you use tst_res(TBROK ...).
> 
> > I don't think that is the case. tst_res(TBROK, ...) shouldn't (and
> > isn't for me) result in skipping of tests.
> Correct, tst_res() itself doesn't exit the test. But the fact it uses TBROK
> without reporting any result previously leads to tst_brk from the library:
> 
> tst_test.c:1036: BROK: Test haven't reported results!

Ahh, I missed this part then.

> > Right, but in my case I can't put all setup stuff in setup() callback
> > and some of the setup bits stay in run() callback as well.
> Sure. But as a result of it I'd personally use tst_res(TFAIL).
> But maybe I'm wrong and others will correct me.

FWIW, I am the least educated guy here in terms of LTP stuff :)

I just want to make sure I don't need to do it again and so wanted to
better understand TBROK vs TFAIL thing.

So the conclusion is that in the run() callback we should always use
TFAIL ?

-- 
viresh

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

* [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests
  2020-02-26  8:59             ` Viresh Kumar
@ 2020-02-26  9:20               ` Petr Vorel
  0 siblings, 0 replies; 33+ messages in thread
From: Petr Vorel @ 2020-02-26  9:20 UTC (permalink / raw)
  To: ltp

Hi Viresh,

> > > I don't think that is the case. tst_res(TBROK, ...) shouldn't (and
> > > isn't for me) result in skipping of tests.
> > Correct, tst_res() itself doesn't exit the test. But the fact it uses TBROK
> > without reporting any result previously leads to tst_brk from the library:

> > tst_test.c:1036: BROK: Test haven't reported results!

> Ahh, I missed this part then.

> > > Right, but in my case I can't put all setup stuff in setup() callback
> > > and some of the setup bits stay in run() callback as well.
> > Sure. But as a result of it I'd personally use tst_res(TFAIL).
> > But maybe I'm wrong and others will correct me.

> FWIW, I am the least educated guy here in terms of LTP stuff :)
You're doing well :).

> I just want to make sure I don't need to do it again and so wanted to
> better understand TBROK vs TFAIL thing.
+1.

> So the conclusion is that in the run() callback we should always use
> TFAIL ?
I'd wait on the outcome of the discussion here
https://patchwork.ozlabs.org/patch/1244781/

Kind regards,
Petr

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

* [LTP] [PATCH V3 05/10] syscalls/fsconfig: New tests
  2020-02-25 13:46   ` Petr Vorel
@ 2020-02-27  4:54     ` Viresh Kumar
  0 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2020-02-27  4:54 UTC (permalink / raw)
  To: ltp

On 25-02-20, 14:46, Petr Vorel wrote:
> > diff --git a/testcases/kernel/syscalls/fsconfig/fsconfig02.c b/testcases/kernel/syscalls/fsconfig/fsconfig02.c
> > new file mode 100644
> > index 000000000000..d51a869ac3ff
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/fsconfig/fsconfig02.c
> > @@ -0,0 +1,98 @@
> > +// 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"
> > +
> > +int fd = -1, temp_fd = -1, invalid_fd = -1;
> > +int aux_0 = 0, aux_1 = 1, aux_fdcwd = AT_FDCWD, aux_minus1 = -1;
> These 2 should be static (also fd could be default 0, but who cares :)).

No, fd was intentionally set to -1 here. If we return early due to
tst_brk() from fsopen_supported_by_kernel(), then we need to handle
that case in cleanup().

-- 
viresh

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

end of thread, other threads:[~2020-02-27  4:54 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25  6:40 [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Viresh Kumar
2020-02-25  6:40 ` [LTP] [PATCH V3 01/10] tst_device: Add tst_ismount() helper Viresh Kumar
2020-02-25 10:39   ` [LTP] [PATCH V4 1/10] tst_device: Add tst_is_mounted() helper Viresh Kumar
2020-02-26  5:14     ` Zorro Lang
2020-02-26  5:58       ` Yang Xu
2020-02-26  6:28       ` Viresh Kumar
2020-02-25  6:40 ` [LTP] [PATCH V3 02/10] lapi/fsmount.h: Add fsopen_supported_by_kernel() Viresh Kumar
2020-02-26  5:51   ` Zorro Lang
2020-02-25  6:40 ` [LTP] [PATCH V3 03/10] lapi/fsmount.h: Include "lapi/fcntl.h" Viresh Kumar
2020-02-26  5:51   ` Zorro Lang
2020-02-25  6:40 ` [LTP] [PATCH V3 04/10] syscalls/fsopen: New tests Viresh Kumar
2020-02-25  6:40 ` [LTP] [PATCH V3 05/10] syscalls/fsconfig: " Viresh Kumar
2020-02-25 13:46   ` Petr Vorel
2020-02-27  4:54     ` Viresh Kumar
2020-02-25  6:40 ` [LTP] [PATCH V3 06/10] syscalls/fsmount: Improve fsmount01 test Viresh Kumar
2020-02-25  6:40 ` [LTP] [PATCH V3 07/10] syscalls/fsmount: Add failure tests Viresh Kumar
2020-02-25  6:40 ` [LTP] [PATCH V3 08/10] syscalls/move_mount: New tests Viresh Kumar
2020-02-25 13:57   ` Petr Vorel
2020-02-26  2:27     ` Viresh Kumar
2020-02-26  3:34       ` Yang Xu
2020-02-26  4:34         ` Viresh Kumar
2020-02-26  7:47       ` Petr Vorel
2020-02-26  8:23         ` Viresh Kumar
2020-02-26  8:53           ` Petr Vorel
2020-02-26  8:59             ` Viresh Kumar
2020-02-26  9:20               ` Petr Vorel
2020-02-25  6:40 ` [LTP] [PATCH V3 09/10] syscalls/fspick: " Viresh Kumar
2020-02-25  6:40 ` [LTP] [PATCH V3 10/10] syscalls/open_tree: " Viresh Kumar
2020-02-25 14:25   ` Petr Vorel
2020-02-25 10:00 ` [LTP] [PATCH V3 00/10] Add new LTP tests related to fsmount family of syscalls Li Wang
2020-02-25 10:32   ` Petr Vorel
2020-02-25 10:48   ` Viresh Kumar
2020-02-25 14:23 ` Petr Vorel

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.