diff mbox series

[V2,04/10] syscalls/fsopen: New tests

Message ID aab7ae7e324c40e8e995ff381384a402d2aba8f5.1582104018.git.viresh.kumar@linaro.org
State Changes Requested
Headers show
Series Add new LTP tests related to fsmount family of syscalls | expand

Commit Message

Viresh Kumar Feb. 19, 2020, 9:28 a.m. UTC 
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 | 85 +++++++++++++++++++++
 testcases/kernel/syscalls/fsopen/fsopen02.c | 57 ++++++++++++++
 5 files changed, 153 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

Comments

Li Wang Feb. 20, 2020, 5:23 a.m. UTC | #1 
On Wed, Feb 19, 2020 at 5:28 PM Viresh Kumar <viresh.kumar@linaro.org>
wrote:

> ...
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fsopen/fsopen01.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * Description:
> + * Basic fsopen() test which tries to configure and mount the filesystem
> as
> + * well.
> + */
> +#include "tst_test.h"
> +#include "lapi/fsmount.h"
> +
> +#define MNTPOINT       "mntpoint"
> +
> +static struct tcase {
> +       char *name;
> +       unsigned int flags;
> +} tcases[] = {
> +       {"Flag 0", 0},
> +       {"Flag FSOPEN_CLOEXEC", FSOPEN_CLOEXEC},
>

Maybe better to remove the 'Flag' keywords from here and add it to the
print sentence.


> +static void run(unsigned int n)
> +{
> ...
> +
> +       if (!tst_ismount(MNTPOINT))
> +               tst_res(TPASS, "%s: fsopen() passed", tc->name);
>

tst_res(TPASS, "Flag %s: fsopen() passed", tc->name);
Petr Vorel Feb. 20, 2020, 8:51 a.m. UTC | #2 
Hi,

> > +static struct tcase {
> > +       char *name;
> > +       unsigned int flags;
> > +} tcases[] = {
> > +       {"Flag 0", 0},
> > +       {"Flag FSOPEN_CLOEXEC", FSOPEN_CLOEXEC},


> Maybe better to remove the 'Flag' keywords from here and add it to the
> print sentence.
+1. BTW if you remove bogus text, you can use macro to not repeat the flag,
see testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep01.c
#define TYPE_NAME(x) .ttype = x, .desc = #x
(just a suggestion, nothing really important)

> > +static void run(unsigned int n)
> > +{
> > ...
> > +
> > +       if (!tst_ismount(MNTPOINT))
> > +               tst_res(TPASS, "%s: fsopen() passed", tc->name);


> tst_res(TPASS, "Flag %s: fsopen() passed", tc->name);

Kind regards,
Petr
Petr Vorel Feb. 20, 2020, 5:04 p.m. UTC | #3 
Hi Viresh,

...
> +static struct tcase {
> +	char *name;
> +	unsigned int flags;
> +} tcases[] = {
> +	{"Flag 0", 0},
> +	{"Flag FSOPEN_CLOEXEC", FSOPEN_CLOEXEC},
> +};
Note about removing text from flags apply also for this test.

> +static void setup(void)
> +{
> +	fsopen_supported_by_kernel();
> +}
How about removing this and use as setup function directly
fsopen_supported_by_kernel()?
.setup = fsopen_supported_by_kernel,

Other than that it's OK.
However I have problem on fsopen01 and fsmount02.
mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0', not making
filesystem (use -I to override)
tst_mkfs.c:100: BROK: tst_test.c:830: mkfs.vfat failed with 1

.dev_fs_flags = TST_FS_SKIP_FUSE helps with exfat and ntfs, but this also fails
on vfat.

This is tested on system with CONFIG_VFAT_FS=m and CONFIG_FAT_FS=m, vfat and fat
are loaded by mkfs.vfat. IMHO we need something like TST_FS_SKIP_EXFAT | TST_FS_SKIP_FAT | TST_FS_SKIP_NTFS | TST_FS_SKIP_FUSE
i.e. explicitly say what FS is not wanted no matter whether it's fuse or not.

Or maybe just TST_FS_SKIP_FAT | TST_FS_SKIP_FUSE would be enough.

NOTE: flags TST_FS_SKIP_EXFAT, TST_FS_SKIP_FAT, TST_FS_SKIP_NTFS do not exists yet.

...
> diff --git a/testcases/kernel/syscalls/fsopen/fsopen02.c b/testcases/kernel/syscalls/fsopen/fsopen02.c
> new file mode 100644
> index 000000000000..72cb940c5468
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fsopen/fsopen02.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * Description:
Nit: I'd avoid "Description:".
> + * Basic fsopen() failure tests.
> + */
...

> +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_brk(TFAIL, "%s: fsopen() succeeded unexpectedly (index: %d)",
> +			tc->name, n);
This also need tst_res() and return, or second test get skipped if first fails.
See http://lists.linux.it/pipermail/ltp/2020-February/015505.html

> +	}
> +
> +	if (tc->exp_errno != TST_ERR) {
> +		tst_brk(TFAIL | TTERRNO, "%s: fsopen() should fail with %s",
> +			tc->name, tst_strerrno(tc->exp_errno));
And here too.
> +	}

Kind regards,
Petr
Viresh Kumar Feb. 24, 2020, 3:18 a.m. UTC | #4 
On 20-02-20, 18:04, Petr Vorel wrote:
> However I have problem on fsopen01 and fsmount02.
> mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0', not making
> filesystem (use -I to override)
> tst_mkfs.c:100: BROK: tst_test.c:830: mkfs.vfat failed with 1
> 
> .dev_fs_flags = TST_FS_SKIP_FUSE helps with exfat and ntfs, but this also fails
> on vfat.
> 
> This is tested on system with CONFIG_VFAT_FS=m and CONFIG_FAT_FS=m, vfat and fat
> are loaded by mkfs.vfat. IMHO we need something like TST_FS_SKIP_EXFAT | TST_FS_SKIP_FAT | TST_FS_SKIP_NTFS | TST_FS_SKIP_FUSE
> i.e. explicitly say what FS is not wanted no matter whether it's fuse or not.
> 
> Or maybe just TST_FS_SKIP_FAT | TST_FS_SKIP_FUSE would be enough.
> 
> NOTE: flags TST_FS_SKIP_EXFAT, TST_FS_SKIP_FAT, TST_FS_SKIP_NTFS do not exists yet.

I am not sure what should be done here.

@Cyril ?
Yang Xu Feb. 24, 2020, 3:35 a.m. UTC | #5 
Hi Viresh

> On 20-02-20, 18:04, Petr Vorel wrote:
>> However I have problem on fsopen01 and fsmount02.
>> mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0', not making
>> filesystem (use -I to override)
>> tst_mkfs.c:100: BROK: tst_test.c:830: mkfs.vfat failed with 1
>>
>> .dev_fs_flags = TST_FS_SKIP_FUSE helps with exfat and ntfs, but this also fails
>> on vfat.
>>
>> This is tested on system with CONFIG_VFAT_FS=m and CONFIG_FAT_FS=m, vfat and fat
>> are loaded by mkfs.vfat. IMHO we need something like TST_FS_SKIP_EXFAT | TST_FS_SKIP_FAT | TST_FS_SKIP_NTFS | TST_FS_SKIP_FUSE
>> i.e. explicitly say what FS is not wanted no matter whether it's fuse or not.
>>
>> Or maybe just TST_FS_SKIP_FAT | TST_FS_SKIP_FUSE would be enough.
>>
>> NOTE: flags TST_FS_SKIP_EXFAT, TST_FS_SKIP_FAT, TST_FS_SKIP_NTFS do not exists yet.
> 
> I am not sure what should be done here.
I guess petr may want to let you add TST_FS_SKIP_FAT flag like  adding 
TST_FS_SKIP_FUSE flag in commit dbe56e52bc50("ib: Add flags to 
tst_get_supported_fs_types()").

Best Regards
Yang Xu
> 
> @Cyril ?
>
Petr Vorel Feb. 24, 2020, 6:27 a.m. UTC | #6 
Hi,

> > On 20-02-20, 18:04, Petr Vorel wrote:
> > > However I have problem on fsopen01 and fsmount02.
> > > mkfs.vfat: Partitions or virtual mappings on device '/dev/loop0', not making
> > > filesystem (use -I to override)
> > > tst_mkfs.c:100: BROK: tst_test.c:830: mkfs.vfat failed with 1

> > > .dev_fs_flags = TST_FS_SKIP_FUSE helps with exfat and ntfs, but this also fails
> > > on vfat.

> > > This is tested on system with CONFIG_VFAT_FS=m and CONFIG_FAT_FS=m, vfat and fat
> > > are loaded by mkfs.vfat. IMHO we need something like TST_FS_SKIP_EXFAT | TST_FS_SKIP_FAT | TST_FS_SKIP_NTFS | TST_FS_SKIP_FUSE
> > > i.e. explicitly say what FS is not wanted no matter whether it's fuse or not.

> > > Or maybe just TST_FS_SKIP_FAT | TST_FS_SKIP_FUSE would be enough.

> > > NOTE: flags TST_FS_SKIP_EXFAT, TST_FS_SKIP_FAT, TST_FS_SKIP_NTFS do not exists yet.

> > I am not sure what should be done here.
> I guess petr may want to let you add TST_FS_SKIP_FAT flag like  adding
> TST_FS_SKIP_FUSE flag in commit dbe56e52bc50("ib: Add flags to
> tst_get_supported_fs_types()").
Yes, that's what I meant.
This was meant as a question whether there is simpler solution than this
(it's easy to implement it, but I might have overlook something - it might be
fixed even without it).

> Best Regards
> Yang Xu

> > @Cyril ?
Cyril Hrubis Feb. 24, 2020, 1:08 p.m. UTC | #7 
Hi!
> I am not sure what should be done here.
> 
> @Cyril ?

Hmm, we should fix library to be able to format the device properly,
looks like some kind of bug though:

https://github.com/dosfstools/dosfstools/issues/40
Petr Vorel Feb. 24, 2020, 3:30 p.m. UTC | #8 
Hi,

> > @Cyril ?

> Hmm, we should fix library to be able to format the device properly,
> looks like some kind of bug though:

> https://github.com/dosfstools/dosfstools/issues/40

I'm not able to reproduce the problems on fsopen01 and fsmount02.
Using different tmp directory via TMPDIR fixed that, but it started to work even
without it and even reboot didn't bring back the failure. On different host
(different distro) it worked all the time. I wonder what was wrong.

Kind regards,
Petr
Cyril Hrubis Feb. 24, 2020, 3:32 p.m. UTC | #9 
Hi!
> > Hmm, we should fix library to be able to format the device properly,
> > looks like some kind of bug though:
> 
> > https://github.com/dosfstools/dosfstools/issues/40
> 
> I'm not able to reproduce the problems on fsopen01 and fsmount02.
> Using different tmp directory via TMPDIR fixed that, but it started to work even
> without it and even reboot didn't bring back the failure. On different host
> (different distro) it worked all the time. I wonder what was wrong.

Hmm, that sounds really strange, I doubt that there is something that
would create partitions on a loop device on a background, so it really
smells like a bug.
Petr Vorel Feb. 24, 2020, 3:46 p.m. UTC | #10 
Hi,

> > > Hmm, we should fix library to be able to format the device properly,
> > > looks like some kind of bug though:

> > > https://github.com/dosfstools/dosfstools/issues/40

> > I'm not able to reproduce the problems on fsopen01 and fsmount02.
> > Using different tmp directory via TMPDIR fixed that, but it started to work even
> > without it and even reboot didn't bring back the failure. On different host
> > (different distro) it worked all the time. I wonder what was wrong.

> Hmm, that sounds really strange, I doubt that there is something that
> would create partitions on a loop device on a background, so it really
> smells like a bug.
I haven't found any hint in dmesg. I tested in on a machine, which sometimes has
problems with disc I/O (occasionally it's slow). So maybe it was hw related.

Kind regards,
Petr
diff mbox series

Patch

diff --git a/runtest/syscalls b/runtest/syscalls
index 72e729c1cee9..f113342f0ded 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..2753e5066805
--- /dev/null
+++ b/testcases/kernel/syscalls/fsopen/fsopen01.c
@@ -0,0 +1,85 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Description:
+ * Basic fsopen() test which tries to configure and mount the filesystem as
+ * well.
+ */
+#include "tst_test.h"
+#include "lapi/fsmount.h"
+
+#define MNTPOINT	"mntpoint"
+
+static struct tcase {
+	char *name;
+	unsigned int flags;
+} tcases[] = {
+	{"Flag 0", 0},
+	{"Flag FSOPEN_CLOEXEC", FSOPEN_CLOEXEC},
+};
+
+static void setup(void)
+{
+	fsopen_supported_by_kernel();
+}
+
+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_brk(TFAIL | TERRNO, "fsopen() failed");
+
+	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_ismount(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 = setup,
+	.needs_root = 1,
+	.format_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.dev_fs_flags = TST_FS_SKIP_FUSE,
+};
diff --git a/testcases/kernel/syscalls/fsopen/fsopen02.c b/testcases/kernel/syscalls/fsopen/fsopen02.c
new file mode 100644
index 000000000000..72cb940c5468
--- /dev/null
+++ b/testcases/kernel/syscalls/fsopen/fsopen02.c
@@ -0,0 +1,57 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Description:
+ * 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_brk(TFAIL, "%s: fsopen() succeeded unexpectedly (index: %d)",
+			tc->name, n);
+	}
+
+	if (tc->exp_errno != TST_ERR) {
+		tst_brk(TFAIL | TTERRNO, "%s: fsopen() should fail with %s",
+			tc->name, tst_strerrno(tc->exp_errno));
+	}
+
+	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,
+};