* [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() @ 2020-02-07 14:41 Petr Vorel 2020-02-07 14:41 ` [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 Petr Vorel 2020-02-07 15:24 ` [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() Cyril Hrubis 0 siblings, 2 replies; 18+ messages in thread From: Petr Vorel @ 2020-02-07 14:41 UTC (permalink / raw) To: ltp To get retry workaround for gvfsd-trash background daemon. Suggested-by: Cyril Hrubis <chrubis@suse.cz> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- Hi, this should be ok, but better to double check. Used in fsmount01. Kind regards, Petr lib/safe_macros.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/safe_macros.c b/lib/safe_macros.c index 41fa4ca83..deb19d2df 100644 --- a/lib/safe_macros.c +++ b/lib/safe_macros.c @@ -772,7 +772,7 @@ int safe_umount(const char *file, const int lineno, void (*cleanup_fn)(void), { int rval; - rval = umount(target); + rval = tst_umount(target); if (rval == -1) { tst_brkm(TBROK | TERRNO, cleanup_fn, -- 2.24.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 2020-02-07 14:41 [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() Petr Vorel @ 2020-02-07 14:41 ` Petr Vorel 2020-02-16 7:32 ` Li Wang 2020-02-07 15:24 ` [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() Cyril Hrubis 1 sibling, 1 reply; 18+ messages in thread From: Petr Vorel @ 2020-02-07 14:41 UTC (permalink / raw) To: ltp From: Zorro Lang <zlang@redhat.com> Add basic tests tests for new mount API from kernel v5.2. Testing mount and umount filesystems with fsopen(), fsconfig(), fsmount() and move_mount(). Signed-off-by: Zorro Lang <zlang@redhat.com> Reported-by: Cyril Hrubis <chrubis@suse.cz> [ pvorel: cleanup autotools and other fixes ] Signed-off-by: Petr Vorel <pvorel@suse.cz> --- configure.ac | 4 + include/lapi/newmount.h | 95 +++++++++++++++++++ include/lapi/syscalls/powerpc64.in | 4 + runtest/syscalls | 2 + testcases/kernel/syscalls/fsmount/.gitignore | 1 + testcases/kernel/syscalls/fsmount/Makefile | 8 ++ testcases/kernel/syscalls/fsmount/fsmount01.c | 94 ++++++++++++++++++ 7 files changed, 208 insertions(+) create mode 100644 include/lapi/newmount.h create mode 100644 testcases/kernel/syscalls/fsmount/.gitignore create mode 100644 testcases/kernel/syscalls/fsmount/Makefile create mode 100644 testcases/kernel/syscalls/fsmount/fsmount01.c diff --git a/configure.ac b/configure.ac index df4e8c832..05b7d0a72 100644 --- a/configure.ac +++ b/configure.ac @@ -80,6 +80,9 @@ AC_CHECK_FUNCS([ \ execveat \ fallocate \ fchownat \ + fsconfig \ + fsmount \ + fsopen \ fstatat \ getdents \ getdents64 \ @@ -88,6 +91,7 @@ AC_CHECK_FUNCS([ \ mkdirat \ mknodat \ modify_ldt \ + move_mount \ name_to_handle_at \ openat \ pidfd_open \ diff --git a/include/lapi/newmount.h b/include/lapi/newmount.h new file mode 100644 index 000000000..d4efdb300 --- /dev/null +++ b/include/lapi/newmount.h @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2019 Red Hat, Inc. All rights reserved. + * Author: Zorro Lang <zlang@redhat.com> + */ + +#ifndef NEWMOUNT_H__ +#define NEWMOUNT_H__ + +#include <stdint.h> +#include <unistd.h> +#include "config.h" +#include "lapi/syscalls.h" + +#ifndef HAVE_FSOPEN +static inline int fsopen(const char *fs_name, unsigned int flags) +{ + return tst_syscall(__NR_fsopen, fs_name, flags); +} + +/* + * fsopen() flags. + */ +#define FSOPEN_CLOEXEC 0x00000001 +#endif /* HAVE_FSOPEN */ + +#ifndef HAVE_FSCONFIG +static inline int fsconfig(int fsfd, unsigned int cmd, + const char *key, const void *val, int aux) +{ + return tst_syscall(__NR_fsconfig, fsfd, cmd, key, val, aux); +} + +/* + * The type of fsconfig() call made. + */ +enum fsconfig_command { + FSCONFIG_SET_FLAG = 0, /* Set parameter, supplying no value */ + FSCONFIG_SET_STRING = 1, /* Set parameter, supplying a string value */ + FSCONFIG_SET_BINARY = 2, /* Set parameter, supplying a binary blob value */ + FSCONFIG_SET_PATH = 3, /* Set parameter, supplying an object by path */ + FSCONFIG_SET_PATH_EMPTY = 4, /* Set parameter, supplying an object by (empty) path */ + FSCONFIG_SET_FD = 5, /* Set parameter, supplying an object by fd */ + FSCONFIG_CMD_CREATE = 6, /* Invoke superblock creation */ + FSCONFIG_CMD_RECONFIGURE = 7, /* Invoke superblock reconfiguration */ +}; +#endif /* HAVE_FSCONFIG */ + +#ifndef HAVE_FSMOUNT +static inline int fsmount(int fsfd, unsigned int flags, unsigned int ms_flags) +{ + return tst_syscall(__NR_fsmount, fsfd, flags, ms_flags); +} + +/* + * fsmount() flags. + */ +#define FSMOUNT_CLOEXEC 0x00000001 + +/* + * Mount attributes. + */ +#define MOUNT_ATTR_RDONLY 0x00000001 /* Mount read-only */ +#define MOUNT_ATTR_NOSUID 0x00000002 /* Ignore suid and sgid bits */ +#define MOUNT_ATTR_NODEV 0x00000004 /* Disallow access to device special files */ +#define MOUNT_ATTR_NOEXEC 0x00000008 /* Disallow program execution */ +#define MOUNT_ATTR__ATIME 0x00000070 /* Setting on how atime should be updated */ +#define MOUNT_ATTR_RELATIME 0x00000000 /* - Update atime relative to mtime/ctime. */ +#define MOUNT_ATTR_NOATIME 0x00000010 /* - Do not update access times. */ +#define MOUNT_ATTR_STRICTATIME 0x00000020 /* - Always perform atime updates */ +#define MOUNT_ATTR_NODIRATIME 0x00000080 /* Do not update directory access times */ +#endif /* HAVE_FSMOUNT */ + +#ifndef HAVE_MOVE_MOUNT +static inline int move_mount(int from_dfd, const char *from_pathname, + int to_dfd, const char *to_pathname, + unsigned int flags) +{ + return tst_syscall(__NR_move_mount, from_dfd, from_pathname, to_dfd, + to_pathname, flags); +} + +/* + * move_mount() flags. + */ +#define MOVE_MOUNT_F_SYMLINKS 0x00000001 /* Follow symlinks on from path */ +#define MOVE_MOUNT_F_AUTOMOUNTS 0x00000002 /* Follow automounts on from path */ +#define MOVE_MOUNT_F_EMPTY_PATH 0x00000004 /* Empty from path permitted */ +#define MOVE_MOUNT_T_SYMLINKS 0x00000010 /* Follow symlinks on to path */ +#define MOVE_MOUNT_T_AUTOMOUNTS 0x00000020 /* Follow automounts on to path */ +#define MOVE_MOUNT_T_EMPTY_PATH 0x00000040 /* Empty to path permitted */ +#define MOVE_MOUNT__MASK 0x00000077 +#endif /* HAVE_MOVE_MOUNT */ + +#endif /* NEWMOUNT_H__ */ diff --git a/include/lapi/syscalls/powerpc64.in b/include/lapi/syscalls/powerpc64.in index 2c1f105c1..beb0e6812 100644 --- a/include/lapi/syscalls/powerpc64.in +++ b/include/lapi/syscalls/powerpc64.in @@ -371,3 +371,7 @@ pidfd_open 434 pkey_mprotect 386 pkey_alloc 384 pkey_free 385 +move_mount 429 +fsopen 430 +fsconfig 431 +fsmount 432 diff --git a/runtest/syscalls b/runtest/syscalls index 0743cf4e3..72e729c1c 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -341,6 +341,8 @@ fpathconf01 fpathconf01 fremovexattr01 fremovexattr01 fremovexattr02 fremovexattr02 +fsmount01 fsmount01 + fstat02 fstat02 fstat02_64 fstat02_64 fstat03 fstat03 diff --git a/testcases/kernel/syscalls/fsmount/.gitignore b/testcases/kernel/syscalls/fsmount/.gitignore new file mode 100644 index 000000000..e2f01ea17 --- /dev/null +++ b/testcases/kernel/syscalls/fsmount/.gitignore @@ -0,0 +1 @@ +/fsmount01 diff --git a/testcases/kernel/syscalls/fsmount/Makefile b/testcases/kernel/syscalls/fsmount/Makefile new file mode 100644 index 000000000..cc80d2efd --- /dev/null +++ b/testcases/kernel/syscalls/fsmount/Makefile @@ -0,0 +1,8 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (C) 2019 Red Hat, Inc. All rights reserved. + +top_srcdir ?= ../../../.. + +include $(top_srcdir)/include/mk/testcases.mk + +include $(top_srcdir)/include/mk/generic_leaf_target.mk diff --git a/testcases/kernel/syscalls/fsmount/fsmount01.c b/testcases/kernel/syscalls/fsmount/fsmount01.c new file mode 100644 index 000000000..99e7a1720 --- /dev/null +++ b/testcases/kernel/syscalls/fsmount/fsmount01.c @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2019 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. + */ + +#include <sys/mount.h> + +#include "tst_test.h" +#include "lapi/newmount.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) + SAFE_UMOUNT(MNTPOINT); +} + +static void test_newmount(void) +{ + TEST(fsopen(tst_device->fs_type, FSOPEN_CLOEXEC)); + if (TST_RET < 0) + tst_brk(TBROK | TTERRNO, "fsopen %s", tst_device->fs_type); + sfd = TST_RET; + tst_res(TPASS, "fsopen %s", tst_device->fs_type); + + TEST(fsconfig(sfd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0)); + if (TST_RET < 0) + tst_brk(TBROK | TTERRNO, + "fsconfig set source to %s", tst_device->dev); + tst_res(TPASS, "fsconfig set source to %s", tst_device->dev); + + + TEST(fsconfig(sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0)); + if (TST_RET < 0) + tst_brk(TBROK | TTERRNO, "fsconfig create superblock"); + tst_res(TPASS, "fsconfig create superblock"); + + TEST(fsmount(sfd, FSMOUNT_CLOEXEC, 0)); + if (TST_RET < 0) + tst_brk(TBROK | TTERRNO, "fsmount"); + mfd = TST_RET; + tst_res(TPASS, "fsmount"); + SAFE_CLOSE(sfd); + + TEST(move_mount(mfd, "", AT_FDCWD, MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH)); + if (TST_RET < 0) + tst_brk(TBROK | TTERRNO, "move_mount attach to mount point"); + is_mounted = 1; + tst_res(TPASS, "move_mount attach to mount point"); + SAFE_CLOSE(mfd); + + if (ismount(MNTPOINT)) { + tst_res(TPASS, "new mount API from v5.2 works"); + SAFE_UMOUNT(MNTPOINT); + is_mounted = 0; + } else + tst_res(TFAIL, "new mount API from v5.2 works"); +} + +static struct tst_test test = { + .test_all = test_newmount, + .cleanup = cleanup, + .needs_root = 1, + .mntpoint = MNTPOINT, + .format_device = 1, + .all_filesystems = 1, + .dev_fs_flags = TST_FS_SKIP_FUSE, +}; -- 2.24.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 2020-02-07 14:41 ` [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 Petr Vorel @ 2020-02-16 7:32 ` Li Wang 2020-02-16 13:17 ` Petr Vorel 2020-02-17 8:04 ` Viresh Kumar 0 siblings, 2 replies; 18+ messages in thread From: Li Wang @ 2020-02-16 7:32 UTC (permalink / raw) To: ltp Hi, [CC Viresh Kumar] Patch v6 looks almost good besides tiny issues: I don't like the commit summary with kernel version number, can we just note as: "syscalls/fsmount01: Add test for fsmount series API"? On Fri, Feb 7, 2020 at 10:41 PM Petr Vorel <pvorel@suse.cz> wrote: > From: Zorro Lang <zlang@redhat.com> > > Add basic tests tests for new mount API from kernel v5.2. > Testing mount and umount filesystems with fsopen(), fsconfig(), > fsmount() and move_mount(). > > Signed-off-by: Zorro Lang <zlang@redhat.com> > Reported-by: Cyril Hrubis <chrubis@suse.cz> > [ pvorel: cleanup autotools and other fixes ] > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > configure.ac | 4 + > include/lapi/newmount.h | 95 +++++++++++++++++++ > include/lapi/syscalls/powerpc64.in | 4 + > Is there any reason why only add syscall num for ppc64? > > diff --git a/configure.ac b/configure.ac > index df4e8c832..05b7d0a72 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -80,6 +80,9 @@ AC_CHECK_FUNCS([ \ > execveat \ > fallocate \ > fchownat \ > + fsconfig \ > + fsmount \ > + fsopen \ > fstatat \ > getdents \ > getdents64 \ > @@ -88,6 +91,7 @@ AC_CHECK_FUNCS([ \ > mkdirat \ > mknodat \ > modify_ldt \ > + move_mount \ > name_to_handle_at \ > openat \ > pidfd_open \ > diff --git a/include/lapi/newmount.h b/include/lapi/newmount.h > new file mode 100644 > index 000000000..d4efdb300 > --- /dev/null > +++ b/include/lapi/newmount.h > Maybe rename to fsmount.h is better? Now we think it new since mainline v5.2 is new to us, one year later it probably not new actually, to use a name can indicate the functionality is wiser I guess. BTW, I like the way Viresh Kumar gives in his fsmount.h, it looks more tidy and clean. http://lists.linux.it/pipermail/ltp/2020-February/015413.html > --- /dev/null > +++ b/testcases/kernel/syscalls/fsmount/fsmount01.c > @@ -0,0 +1,94 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2019 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. > + */ > + > +#include <sys/mount.h> > + > +#include "tst_test.h" > +#include "lapi/newmount.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) > + SAFE_UMOUNT(MNTPOINT); > +} > + > +static void test_newmount(void) > static void test_fsmount(void)? Or, static void run(void). > +{ > + TEST(fsopen(tst_device->fs_type, FSOPEN_CLOEXEC)); > + if (TST_RET < 0) > + tst_brk(TBROK | TTERRNO, "fsopen %s", tst_device->fs_type); > + sfd = TST_RET; > + tst_res(TPASS, "fsopen %s", tst_device->fs_type); > + > + TEST(fsconfig(sfd, FSCONFIG_SET_STRING, "source", tst_device->dev, > 0)); > + if (TST_RET < 0) > + tst_brk(TBROK | TTERRNO, > + "fsconfig set source to %s", tst_device->dev); > + tst_res(TPASS, "fsconfig set source to %s", tst_device->dev); > + > + > + TEST(fsconfig(sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0)); > + if (TST_RET < 0) > + tst_brk(TBROK | TTERRNO, "fsconfig create superblock"); > + tst_res(TPASS, "fsconfig create superblock"); > + > + TEST(fsmount(sfd, FSMOUNT_CLOEXEC, 0)); > + if (TST_RET < 0) > + tst_brk(TBROK | TTERRNO, "fsmount"); > + mfd = TST_RET; > + tst_res(TPASS, "fsmount"); > + SAFE_CLOSE(sfd); > + > + TEST(move_mount(mfd, "", AT_FDCWD, MNTPOINT, > MOVE_MOUNT_F_EMPTY_PATH)); > + if (TST_RET < 0) > + tst_brk(TBROK | TTERRNO, "move_mount attach to mount > point"); > + is_mounted = 1; > + tst_res(TPASS, "move_mount attach to mount point"); > + SAFE_CLOSE(mfd); > + > + if (ismount(MNTPOINT)) { > + tst_res(TPASS, "new mount API from v5.2 works"); > Can we avoid appearance the v5.2? I guess many Enterprise Linux Distributions will backport the fsmount() series API, v5.2 in test log looks strange if the kernel is older than it. > + SAFE_UMOUNT(MNTPOINT); > + is_mounted = 0; > + } else > + tst_res(TFAIL, "new mount API from v5.2 works"); > no v5.2 > +} > + > +static struct tst_test test = { > + .test_all = test_newmount, > + .cleanup = cleanup, > + .needs_root = 1, > + .mntpoint = MNTPOINT, > + .format_device = 1, > + .all_filesystems = 1, > + .dev_fs_flags = TST_FS_SKIP_FUSE, > +}; > -- > 2.24.1 > > -- Regards, Li Wang -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.linux.it/pipermail/ltp/attachments/20200216/3ca8f298/attachment.htm> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 2020-02-16 7:32 ` Li Wang @ 2020-02-16 13:17 ` Petr Vorel 2020-02-17 6:48 ` Li Wang 2020-02-17 7:17 ` Li Wang 2020-02-17 8:04 ` Viresh Kumar 1 sibling, 2 replies; 18+ messages in thread From: Petr Vorel @ 2020-02-16 13:17 UTC (permalink / raw) To: ltp Hi Li, > Patch v6 looks almost good besides tiny issues: > I don't like the commit summary with kernel version number, can we just > note as: > "syscalls/fsmount01: Add test for fsmount series API"? Sure. > > configure.ac | 4 + > > include/lapi/newmount.h | 95 +++++++++++++++++++ > > include/lapi/syscalls/powerpc64.in | 4 + > Is there any reason why only add syscall num for ppc64? The other numbers are already there: 01e4dc222 lapi/syscalls: Add MIPS support c2f27f6e9 Add syscall numbers for new file-system related syscalls Not sure if we should add a note in the commit message to prevent confusion later (probably not needed). > > +++ b/include/lapi/newmount.h > Maybe rename to fsmount.h is better? Now we think it new since mainline > v5.2 is new to us, one year later it probably not new actually, to use a > name can indicate the functionality is wiser I guess. +1 > BTW, I like the way Viresh Kumar gives in his fsmount.h, it looks more tidy > and clean. > http://lists.linux.it/pipermail/ltp/2020-February/015413.html Hm, competing implementations. Both tries to handle preventing redefinitions (e.g. FSOPEN_CLOEXEC) once the API hits libc headers (at least in musl it might be soon). Zorro tries to bind them to function check (e.g. #ifndef HAVE_FSMOUNT, #ifndef HAVE_MOVE_MOUNT), Viresh just use single check #ifndef OPEN_TREE_CLONE. I slightly prefer Viresh way (it's unlikely that libc headers would include just part of the new mount API definitions, although obviously the most safest way would be to either guard with #ifndef each definition or just give up on testing header and copy whole include/uapi/linux/mount.h (+ avoid using sys/mount.h; that's the way used for include/lapi/bpf.h). > > +++ b/testcases/kernel/syscalls/fsmount/fsmount01.c ... > > +static void test_newmount(void) > static void test_fsmount(void)? Or, static void run(void). +1. > > + if (ismount(MNTPOINT)) { > > + tst_res(TPASS, "new mount API from v5.2 works"); > Can we avoid appearance the v5.2? I guess many Enterprise Linux > Distributions will backport the fsmount() series API, v5.2 in test log > looks strange if the kernel is older than it. +1. That was added by myself. I've prepared fixed version, we just need to decide how to handle LAPI header. Kind regards, Petr ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 2020-02-16 13:17 ` Petr Vorel @ 2020-02-17 6:48 ` Li Wang 2020-02-17 7:51 ` Petr Vorel 2020-02-17 9:16 ` Jan Stancek 2020-02-17 7:17 ` Li Wang 1 sibling, 2 replies; 18+ messages in thread From: Li Wang @ 2020-02-17 6:48 UTC (permalink / raw) To: ltp Hi Petr, On Sun, Feb 16, 2020 at 9:17 PM Petr Vorel <pvorel@suse.cz> wrote: > ... > > > include/lapi/syscalls/powerpc64.in | 4 + > > Is there any reason why only add syscall num for ppc64? > The other numbers are already there: > 01e4dc222 lapi/syscalls: Add MIPS support > c2f27f6e9 Add syscall numbers for new file-system related syscalls > Good to know this. Not sure if we should add a note in the commit message to prevent confusion > later (probably not needed). > Or just mentionion that commit(c2f27f6e9 Add syscall numbers ...) message. > BTW, I like the way Viresh Kumar gives in his fsmount.h, it looks more > tidy > > and clean. > > http://lists.linux.it/pipermail/ltp/2020-February/015413.html > Hm, competing implementations. > Both tries to handle preventing redefinitions (e.g. FSOPEN_CLOEXEC) once > the API hits libc headers (at least in musl it might be soon). > Zorro tries to bind them to function check (e.g. #ifndef HAVE_FSMOUNT, > #ifndef > HAVE_MOVE_MOUNT), Viresh just use single check #ifndef OPEN_TREE_CLONE. > I slightly prefer Viresh way (it's unlikely that libc headers would > include just > +1 Viresh way. > part of the new mount API definitions, although obviously the most safest > way > would be to either guard with #ifndef each definition or just give up on > testing > header and copy whole include/uapi/linux/mount.h (+ avoid using > sys/mount.h; > that's the way used for include/lapi/bpf.h). > @Cyril, @Jan, any else suggestion? Btw, we have to include "lapi/fcntl.h" in the test to get rid of build error from old(RHEL5.11) distro: fsmount01.c:71: error: ?AT_FDCWD? undeclared (first use in this function) fsmount01.c:71: error: (Each undeclared identifier is reported only once fsmount01.c:71: error: for each function it appears in.) -- Regards, Li Wang -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.linux.it/pipermail/ltp/attachments/20200217/f295f6a4/attachment-0001.htm> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 2020-02-17 6:48 ` Li Wang @ 2020-02-17 7:51 ` Petr Vorel 2020-02-17 9:16 ` Jan Stancek 1 sibling, 0 replies; 18+ messages in thread From: Petr Vorel @ 2020-02-17 7:51 UTC (permalink / raw) To: ltp Hi Li, > On Sun, Feb 16, 2020 at 9:17 PM Petr Vorel <pvorel@suse.cz> wrote: > > ... > > > > include/lapi/syscalls/powerpc64.in | 4 + > > > Is there any reason why only add syscall num for ppc64? > > The other numbers are already there: > > 01e4dc222 lapi/syscalls: Add MIPS support > > c2f27f6e9 Add syscall numbers for new file-system related syscalls > Good to know this. > > Not sure if we should add a note in the commit message to prevent confusion > > later (probably not needed). > Or just mentionion that commit(c2f27f6e9 Add syscall numbers ...) message. I'll add it (I'll mention both commits). > > BTW, I like the way Viresh Kumar gives in his fsmount.h, it looks more > > tidy > > > and clean. > > > http://lists.linux.it/pipermail/ltp/2020-February/015413.html > > Hm, competing implementations. > > Both tries to handle preventing redefinitions (e.g. FSOPEN_CLOEXEC) once > > the API hits libc headers (at least in musl it might be soon). > > Zorro tries to bind them to function check (e.g. #ifndef HAVE_FSMOUNT, > > #ifndef > > HAVE_MOVE_MOUNT), Viresh just use single check #ifndef OPEN_TREE_CLONE. > > I slightly prefer Viresh way (it's unlikely that libc headers would > > include just > +1 Viresh way. I'm also for merging 1st commit from Viresh's patchset and than Zorro's fsmount01 test with dropped lapi commit. I'll send PR so others can see the fixes. > > part of the new mount API definitions, although obviously the most safest > > way > > would be to either guard with #ifndef each definition or just give up on > > testing > > header and copy whole include/uapi/linux/mount.h (+ avoid using > > sys/mount.h; > > that's the way used for include/lapi/bpf.h). > @Cyril, @Jan, any else suggestion? > Btw, we have to include "lapi/fcntl.h" in the test to get rid of build > error from old(RHEL5.11) distro: Thanks for notifying, I'll add it. BTW is there a way to add this old RHEL to travis? Or is there any old Fedora version which would imitate that? Or I can try to search about old SLES 11 (or old openSUSE version which would imitate that). > fsmount01.c:71: error: ?AT_FDCWD? undeclared (first use in this function) > fsmount01.c:71: error: (Each undeclared identifier is reported only once > fsmount01.c:71: error: for each function it appears in.) Kind regards, Petr ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 2020-02-17 6:48 ` Li Wang 2020-02-17 7:51 ` Petr Vorel @ 2020-02-17 9:16 ` Jan Stancek 2020-02-17 9:43 ` Petr Vorel 1 sibling, 1 reply; 18+ messages in thread From: Jan Stancek @ 2020-02-17 9:16 UTC (permalink / raw) To: ltp ----- Original Message ----- > Hi Petr, > > On Sun, Feb 16, 2020 at 9:17 PM Petr Vorel <pvorel@suse.cz> wrote: > > > ... > > > > include/lapi/syscalls/powerpc64.in | 4 + > > > Is there any reason why only add syscall num for ppc64? > > The other numbers are already there: > > 01e4dc222 lapi/syscalls: Add MIPS support > > c2f27f6e9 Add syscall numbers for new file-system related syscalls > > > > Good to know this. > > Not sure if we should add a note in the commit message to prevent confusion > > later (probably not needed). > > > > Or just mentionion that commit(c2f27f6e9 Add syscall numbers ...) message. > > > BTW, I like the way Viresh Kumar gives in his fsmount.h, it looks more > > tidy > > > and clean. > > > http://lists.linux.it/pipermail/ltp/2020-February/015413.html > > Hm, competing implementations. > > Both tries to handle preventing redefinitions (e.g. FSOPEN_CLOEXEC) once > > the API hits libc headers (at least in musl it might be soon). > > Zorro tries to bind them to function check (e.g. #ifndef HAVE_FSMOUNT, > > #ifndef > > HAVE_MOVE_MOUNT), Viresh just use single check #ifndef OPEN_TREE_CLONE. > > I slightly prefer Viresh way (it's unlikely that libc headers would > > include just > > > +1 Viresh way. > > > > part of the new mount API definitions, although obviously the most safest > > way > > would be to either guard with #ifndef each definition or just give up on > > testing > > header and copy whole include/uapi/linux/mount.h (+ avoid using > > sys/mount.h; > > that's the way used for include/lapi/bpf.h). > > > > @Cyril, @Jan, any else suggestion? I'd go with additions to lapi, and avoid copying entire linux/mount.h. And use #ifndef for each definition. v7 is currently not doing that, but it's easy to add if we run into problems later, when/if there are additions to mount API. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 2020-02-17 9:16 ` Jan Stancek @ 2020-02-17 9:43 ` Petr Vorel 0 siblings, 0 replies; 18+ messages in thread From: Petr Vorel @ 2020-02-17 9:43 UTC (permalink / raw) To: ltp Hi Jan, ... > > > BTW, I like the way Viresh Kumar gives in his fsmount.h, it looks more > > > tidy > > > > and clean. > > > > http://lists.linux.it/pipermail/ltp/2020-February/015413.html > > > Hm, competing implementations. > > > Both tries to handle preventing redefinitions (e.g. FSOPEN_CLOEXEC) once > > > the API hits libc headers (at least in musl it might be soon). > > > Zorro tries to bind them to function check (e.g. #ifndef HAVE_FSMOUNT, > > > #ifndef > > > HAVE_MOVE_MOUNT), Viresh just use single check #ifndef OPEN_TREE_CLONE. > > > I slightly prefer Viresh way (it's unlikely that libc headers would > > > include just > > +1 Viresh way. > > > part of the new mount API definitions, although obviously the most safest > > > way > > > would be to either guard with #ifndef each definition or just give up on > > > testing > > > header and copy whole include/uapi/linux/mount.h (+ avoid using > > > sys/mount.h; > > > that's the way used for include/lapi/bpf.h). > > @Cyril, @Jan, any else suggestion? > I'd go with additions to lapi, and avoid copying entire linux/mount.h. And use > #ifndef for each definition. v7 is currently not doing that, but it's easy > to add if we run into problems later, when/if there are additions to mount API. OK, I'm also for single guard with OPEN_TREE_CLONE until anything else is needed. I'll add your ack for lapi commit. Kind regards, Petr ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 2020-02-16 13:17 ` Petr Vorel 2020-02-17 6:48 ` Li Wang @ 2020-02-17 7:17 ` Li Wang 1 sibling, 0 replies; 18+ messages in thread From: Li Wang @ 2020-02-17 7:17 UTC (permalink / raw) To: ltp Petr Vorel <pvorel@suse.cz> wrote: ... > > > include/lapi/syscalls/powerpc64.in | 4 + > > Is there any reason why only add syscall num for ppc64? > The other numbers are already there: > 01e4dc222 lapi/syscalls: Add MIPS support > c2f27f6e9 Add syscall numbers for new file-system related syscalls > [sorry, forget to mention in last email] I have checked the powerpc64.in, the series syscall number already exists there, so this patch also includes the redundant work for powerpc64 :). -- Regards, Li Wang -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.linux.it/pipermail/ltp/attachments/20200217/4c86c347/attachment.htm> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 2020-02-16 7:32 ` Li Wang 2020-02-16 13:17 ` Petr Vorel @ 2020-02-17 8:04 ` Viresh Kumar 1 sibling, 0 replies; 18+ messages in thread From: Viresh Kumar @ 2020-02-17 8:04 UTC (permalink / raw) To: ltp > On Fri, Feb 7, 2020 at 10:41 PM Petr Vorel <pvorel@suse.cz> wrote: > Add basic tests tests for new mount API from kernel v5.2. > Testing mount and umount filesystems with fsopen(), fsconfig(), > fsmount() and move_mount(). > > Signed-off-by: Zorro Lang <zlang@redhat.com> > Reported-by: Cyril Hrubis <chrubis@suse.cz> > [ pvorel: cleanup autotools and other fixes ] > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > configure.ac | 4 + > include/lapi/newmount.h | 95 +++++++++++++++++++ > include/lapi/syscalls/powerpc64.in | 4 + > runtest/syscalls | 2 + > testcases/kernel/syscalls/fsmount/.gitignore | 1 + > testcases/kernel/syscalls/fsmount/Makefile | 8 ++ > testcases/kernel/syscalls/fsmount/fsmount01.c | 94 ++++++++++++++++++ > 7 files changed, 208 insertions(+) > create mode 100644 include/lapi/newmount.h > create mode 100644 testcases/kernel/syscalls/fsmount/.gitignore > create mode 100644 testcases/kernel/syscalls/fsmount/Makefile > create mode 100644 testcases/kernel/syscalls/fsmount/fsmount01.c > > diff --git a/configure.ac b/configure.ac > index df4e8c832..05b7d0a72 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -80,6 +80,9 @@ AC_CHECK_FUNCS([ \ > execveat \ > fallocate \ > fchownat \ > + fsconfig \ > + fsmount \ > + fsopen \ > fstatat \ > getdents \ > getdents64 \ > @@ -88,6 +91,7 @@ AC_CHECK_FUNCS([ \ > mkdirat \ > mknodat \ > modify_ldt \ > + move_mount \ > name_to_handle_at \ > openat \ > pidfd_open \ > diff --git a/include/lapi/newmount.h b/include/lapi/newmount.h > new file mode 100644 > index 000000000..d4efdb300 > --- /dev/null > +++ b/include/lapi/newmount.h > @@ -0,0 +1,95 @@ newmount doesn't look correct to me, what if there is another mount API in future :) fsmount sounds okay to me.. > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2019 Red Hat, Inc. All rights reserved. Should be 2020 now ? > + * Author: Zorro Lang <zlang@redhat.com> > + */ > + > +#ifndef NEWMOUNT_H__ > +#define NEWMOUNT_H__ > + > +#include <stdint.h> > +#include <unistd.h> > +#include "config.h" > +#include "lapi/syscalls.h" > + > +#ifndef HAVE_FSOPEN > +static inline int fsopen(const char *fs_name, unsigned int flags) > +{ > + return tst_syscall(__NR_fsopen, fs_name, flags); > +} > + > +/* > + * fsopen() flags. > + */ > +#define FSOPEN_CLOEXEC 0x00000001 > +#endif /* HAVE_FSOPEN */ > + > +#ifndef HAVE_FSCONFIG > +static inline int fsconfig(int fsfd, unsigned int cmd, > + const char *key, const void *val, int aux) > +{ > + return tst_syscall(__NR_fsconfig, fsfd, cmd, key, val, aux); > +} > + > +/* > + * The type of fsconfig() call made. > + */ > +enum fsconfig_command { > + FSCONFIG_SET_FLAG = 0, /* Set parameter, supplying no value */ > + FSCONFIG_SET_STRING = 1, /* Set parameter, supplying a string value */ > + FSCONFIG_SET_BINARY = 2, /* Set parameter, supplying a binary blob value */ > + FSCONFIG_SET_PATH = 3, /* Set parameter, supplying an object by path */ > + FSCONFIG_SET_PATH_EMPTY = 4, /* Set parameter, supplying an object by (empty) path */ > + FSCONFIG_SET_FD = 5, /* Set parameter, supplying an object by fd */ > + FSCONFIG_CMD_CREATE = 6, /* Invoke superblock creation */ > + FSCONFIG_CMD_RECONFIGURE = 7, /* Invoke superblock reconfiguration */ > +}; > +#endif /* HAVE_FSCONFIG */ > + > +#ifndef HAVE_FSMOUNT > +static inline int fsmount(int fsfd, unsigned int flags, unsigned int ms_flags) > +{ > + return tst_syscall(__NR_fsmount, fsfd, flags, ms_flags); > +} > + > +/* > + * fsmount() flags. > + */ > +#define FSMOUNT_CLOEXEC 0x00000001 > + > +/* > + * Mount attributes. > + */ > +#define MOUNT_ATTR_RDONLY 0x00000001 /* Mount read-only */ > +#define MOUNT_ATTR_NOSUID 0x00000002 /* Ignore suid and sgid bits */ > +#define MOUNT_ATTR_NODEV 0x00000004 /* Disallow access to device special files */ > +#define MOUNT_ATTR_NOEXEC 0x00000008 /* Disallow program execution */ > +#define MOUNT_ATTR__ATIME 0x00000070 /* Setting on how atime should be updated */ > +#define MOUNT_ATTR_RELATIME 0x00000000 /* - Update atime relative to mtime/ctime. */ > +#define MOUNT_ATTR_NOATIME 0x00000010 /* - Do not update access times. */ > +#define MOUNT_ATTR_STRICTATIME 0x00000020 /* - Always perform atime updates */ > +#define MOUNT_ATTR_NODIRATIME 0x00000080 /* Do not update directory access times */ > +#endif /* HAVE_FSMOUNT */ > + > +#ifndef HAVE_MOVE_MOUNT > +static inline int move_mount(int from_dfd, const char *from_pathname, > + int to_dfd, const char *to_pathname, > + unsigned int flags) > +{ > + return tst_syscall(__NR_move_mount, from_dfd, from_pathname, to_dfd, > + to_pathname, flags); > +} > + > +/* > + * move_mount() flags. > + */ > +#define MOVE_MOUNT_F_SYMLINKS 0x00000001 /* Follow symlinks on from path */ > +#define MOVE_MOUNT_F_AUTOMOUNTS 0x00000002 /* Follow automounts on from path */ > +#define MOVE_MOUNT_F_EMPTY_PATH 0x00000004 /* Empty from path permitted */ > +#define MOVE_MOUNT_T_SYMLINKS 0x00000010 /* Follow symlinks on to path */ > +#define MOVE_MOUNT_T_AUTOMOUNTS 0x00000020 /* Follow automounts on to path */ > +#define MOVE_MOUNT_T_EMPTY_PATH 0x00000040 /* Empty to path permitted */ > +#define MOVE_MOUNT__MASK 0x00000077 > +#endif /* HAVE_MOVE_MOUNT */ > + > +#endif /* NEWMOUNT_H__ */ > diff --git a/include/lapi/syscalls/powerpc64.in b/include/lapi/syscalls/powerpc64.in > index 2c1f105c1..beb0e6812 100644 > --- a/include/lapi/syscalls/powerpc64.in > +++ b/include/lapi/syscalls/powerpc64.in > @@ -371,3 +371,7 @@ pidfd_open 434 > pkey_mprotect 386 > pkey_alloc 384 > pkey_free 385 > +move_mount 429 > +fsopen 430 > +fsconfig 431 > +fsmount 432 I have already sent a patch for this earlier and it got merged then. Please rebase on the latest code. > diff --git a/runtest/syscalls b/runtest/syscalls > index 0743cf4e3..72e729c1c 100644 > --- a/runtest/syscalls > +++ b/runtest/syscalls > @@ -341,6 +341,8 @@ fpathconf01 fpathconf01 > fremovexattr01 fremovexattr01 > fremovexattr02 fremovexattr02 > > +fsmount01 fsmount01 > + > fstat02 fstat02 > fstat02_64 fstat02_64 > fstat03 fstat03 > diff --git a/testcases/kernel/syscalls/fsmount/.gitignore b/testcases/kernel/syscalls/fsmount/.gitignore > new file mode 100644 > index 000000000..e2f01ea17 > --- /dev/null > +++ b/testcases/kernel/syscalls/fsmount/.gitignore > @@ -0,0 +1 @@ > +/fsmount01 > diff --git a/testcases/kernel/syscalls/fsmount/Makefile b/testcases/kernel/syscalls/fsmount/Makefile > new file mode 100644 > index 000000000..cc80d2efd > --- /dev/null > +++ b/testcases/kernel/syscalls/fsmount/Makefile > @@ -0,0 +1,8 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +# Copyright (C) 2019 Red Hat, Inc. All rights reserved. > + > +top_srcdir ?= ../../../.. > + > +include $(top_srcdir)/include/mk/testcases.mk > + > +include $(top_srcdir)/include/mk/generic_leaf_target.mk > diff --git a/testcases/kernel/syscalls/fsmount/fsmount01.c b/testcases/kernel/syscalls/fsmount/fsmount01.c > new file mode 100644 > index 000000000..99e7a1720 > --- /dev/null > +++ b/testcases/kernel/syscalls/fsmount/fsmount01.c > @@ -0,0 +1,94 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2019 Red Hat, Inc. All rights reserved. 2020 ? > + * 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. > + */ > + > +#include <sys/mount.h> > + > +#include "tst_test.h" > +#include "lapi/newmount.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) > + SAFE_UMOUNT(MNTPOINT); > +} > + > +static void test_newmount(void) What exactly are we trying to test here? fsmount or all of them ? Asking because we may need to use TFAIL for the API we are testing and TBROK for others, if my understanding is correct. > +{ > + TEST(fsopen(tst_device->fs_type, FSOPEN_CLOEXEC)); > + if (TST_RET < 0) > + tst_brk(TBROK | TTERRNO, "fsopen %s", tst_device->fs_type); May be write explicitly so it is readable without reading the code as well. Like "fsopen() failed for %s filesystem" > + sfd = TST_RET; > + tst_res(TPASS, "fsopen %s", tst_device->fs_type); Okay, so looks like we are testing all syscalls here. Then it should have been TFAIL earlier. > + > + TEST(fsconfig(sfd, FSCONFIG_SET_STRING, "source", tst_device->dev, 0)); > + if (TST_RET < 0) > + tst_brk(TBROK | TTERRNO, > + "fsconfig set source to %s", tst_device->dev); > + tst_res(TPASS, "fsconfig set source to %s", tst_device->dev); same thing with error messages here and below as well. > + > + > + TEST(fsconfig(sfd, FSCONFIG_CMD_CREATE, NULL, NULL, 0)); > + if (TST_RET < 0) > + tst_brk(TBROK | TTERRNO, "fsconfig create superblock"); > + tst_res(TPASS, "fsconfig create superblock"); > + > + TEST(fsmount(sfd, FSMOUNT_CLOEXEC, 0)); > + if (TST_RET < 0) > + tst_brk(TBROK | TTERRNO, "fsmount"); > + mfd = TST_RET; > + tst_res(TPASS, "fsmount"); > + SAFE_CLOSE(sfd); > + > + TEST(move_mount(mfd, "", AT_FDCWD, MNTPOINT, MOVE_MOUNT_F_EMPTY_PATH)); > + if (TST_RET < 0) > + tst_brk(TBROK | TTERRNO, "move_mount attach to mount point"); > + is_mounted = 1; > + tst_res(TPASS, "move_mount attach to mount point"); > + SAFE_CLOSE(mfd); > + > + if (ismount(MNTPOINT)) { > + tst_res(TPASS, "new mount API from v5.2 works"); > + SAFE_UMOUNT(MNTPOINT); > + is_mounted = 0; > + } else > + tst_res(TFAIL, "new mount API from v5.2 works"); > +} > + > +static struct tst_test test = { > + .test_all = test_newmount, > + .cleanup = cleanup, > + .needs_root = 1, > + .mntpoint = MNTPOINT, > + .format_device = 1, > + .all_filesystems = 1, > + .dev_fs_flags = TST_FS_SKIP_FUSE, > +}; -- viresh ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() 2020-02-07 14:41 [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() Petr Vorel 2020-02-07 14:41 ` [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 Petr Vorel @ 2020-02-07 15:24 ` Cyril Hrubis 2020-02-07 15:47 ` Jan Stancek 1 sibling, 1 reply; 18+ messages in thread From: Cyril Hrubis @ 2020-02-07 15:24 UTC (permalink / raw) To: ltp Hi! > To get retry workaround for gvfsd-trash background daemon. > > Suggested-by: Cyril Hrubis <chrubis@suse.cz> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > Hi, > > this should be ok, but better to double check. > Used in fsmount01. My expectation is that retrying on EBUSY will not break anything. Jan what do you think? > lib/safe_macros.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/safe_macros.c b/lib/safe_macros.c > index 41fa4ca83..deb19d2df 100644 > --- a/lib/safe_macros.c > +++ b/lib/safe_macros.c > @@ -772,7 +772,7 @@ int safe_umount(const char *file, const int lineno, void (*cleanup_fn)(void), > { > int rval; > > - rval = umount(target); > + rval = tst_umount(target); > > if (rval == -1) { > tst_brkm(TBROK | TERRNO, cleanup_fn, > -- > 2.24.1 > -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() 2020-02-07 15:24 ` [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() Cyril Hrubis @ 2020-02-07 15:47 ` Jan Stancek 2020-02-07 15:53 ` Petr Vorel 2020-02-07 15:57 ` Cyril Hrubis 0 siblings, 2 replies; 18+ messages in thread From: Jan Stancek @ 2020-02-07 15:47 UTC (permalink / raw) To: ltp ----- Original Message ----- > Hi! > > To get retry workaround for gvfsd-trash background daemon. > > > > Suggested-by: Cyril Hrubis <chrubis@suse.cz> > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > > --- > > Hi, > > > > this should be ok, but better to double check. > > Used in fsmount01. > > My expectation is that retrying on EBUSY will not break anything. Jan > what do you think? I agree. Though I'd like to point out that tst_umount() currently loops on any error, not just EBUSY. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() 2020-02-07 15:47 ` Jan Stancek @ 2020-02-07 15:53 ` Petr Vorel 2020-02-07 15:56 ` Jan Stancek 2020-02-07 15:57 ` Cyril Hrubis 1 sibling, 1 reply; 18+ messages in thread From: Petr Vorel @ 2020-02-07 15:53 UTC (permalink / raw) To: ltp Hi, > > My expectation is that retrying on EBUSY will not break anything. Jan > > what do you think? > I agree. Though I'd like to point out that tst_umount() currently loops > on any error, not just EBUSY. I suggest to give it a try and if problematic, we change it to EBUSY. Ack? Kind regards, Petr ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() 2020-02-07 15:53 ` Petr Vorel @ 2020-02-07 15:56 ` Jan Stancek 0 siblings, 0 replies; 18+ messages in thread From: Jan Stancek @ 2020-02-07 15:56 UTC (permalink / raw) To: ltp ----- Original Message ----- > Hi, > > > > My expectation is that retrying on EBUSY will not break anything. Jan > > > what do you think? > > > I agree. Though I'd like to point out that tst_umount() currently loops > > on any error, not just EBUSY. > > I suggest to give it a try and if problematic, we change it to EBUSY. > Ack? Ack. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() 2020-02-07 15:47 ` Jan Stancek 2020-02-07 15:53 ` Petr Vorel @ 2020-02-07 15:57 ` Cyril Hrubis 2020-02-07 16:10 ` Petr Vorel 2020-02-10 8:19 ` Jan Stancek 1 sibling, 2 replies; 18+ messages in thread From: Cyril Hrubis @ 2020-02-07 15:57 UTC (permalink / raw) To: ltp Hi! > > My expectation is that retrying on EBUSY will not break anything. Jan > > what do you think? > > I agree. Though I'd like to point out that tst_umount() currently loops > on any error, not just EBUSY. Right, I guess that we should change that to be on the safe side, it was never intended to retry anything else than EBUSY. What about? diff --git a/lib/tst_device.c b/lib/tst_device.c index 89b9c96de..52a5b37fd 100644 --- a/lib/tst_device.c +++ b/lib/tst_device.c @@ -371,6 +371,9 @@ int tst_umount(const char *path) "mounted fs, kill it to speed up tests."); } + if (err != EBUSY) { + errno = err; + return ret; + } + usleep(100000); } -- Cyril Hrubis chrubis@suse.cz ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() 2020-02-07 15:57 ` Cyril Hrubis @ 2020-02-07 16:10 ` Petr Vorel 2020-02-10 8:19 ` Jan Stancek 1 sibling, 0 replies; 18+ messages in thread From: Petr Vorel @ 2020-02-07 16:10 UTC (permalink / raw) To: ltp Hi, > > I agree. Though I'd like to point out that tst_umount() currently loops > > on any error, not just EBUSY. > Right, I guess that we should change that to be on the safe side, it was > never intended to retry anything else than EBUSY. > What about? Acked-by: Petr Vorel <pvorel@suse.cz> > diff --git a/lib/tst_device.c b/lib/tst_device.c > index 89b9c96de..52a5b37fd 100644 > --- a/lib/tst_device.c > +++ b/lib/tst_device.c > @@ -371,6 +371,9 @@ int tst_umount(const char *path) > "mounted fs, kill it to speed up tests."); > } > + if (err != EBUSY) { > + errno = err; > + return ret; > + } > + > usleep(100000); > } Kind regards, Petr ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() 2020-02-07 15:57 ` Cyril Hrubis 2020-02-07 16:10 ` Petr Vorel @ 2020-02-10 8:19 ` Jan Stancek 2020-02-10 12:44 ` Petr Vorel 1 sibling, 1 reply; 18+ messages in thread From: Jan Stancek @ 2020-02-10 8:19 UTC (permalink / raw) To: ltp ----- Original Message ----- > Hi! > > > My expectation is that retrying on EBUSY will not break anything. Jan > > > what do you think? > > > > I agree. Though I'd like to point out that tst_umount() currently loops > > on any error, not just EBUSY. > > Right, I guess that we should change that to be on the safe side, it was > never intended to retry anything else than EBUSY. > > What about? > > diff --git a/lib/tst_device.c b/lib/tst_device.c > index 89b9c96de..52a5b37fd 100644 > --- a/lib/tst_device.c > +++ b/lib/tst_device.c > @@ -371,6 +371,9 @@ int tst_umount(const char *path) > "mounted fs, kill it to speed up tests."); > } > > + if (err != EBUSY) { > + errno = err; > + return ret; > + } > + > usleep(100000); > } Works for me too, I don't recall seeing anything else than EBUSY. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() 2020-02-10 8:19 ` Jan Stancek @ 2020-02-10 12:44 ` Petr Vorel 0 siblings, 0 replies; 18+ messages in thread From: Petr Vorel @ 2020-02-10 12:44 UTC (permalink / raw) To: ltp Hi, > > + if (err != EBUSY) { > > + errno = err; > > + return ret; > > + } > > + > > usleep(100000); > > } > Works for me too, I don't recall seeing anything else than EBUSY. Thanks! Just in case I've sent this commit as v2, followed with "safe_macros: Use tst_umount() in safe_umount()" (without Zorro's fsmount commit). IMHO now is docs about tst_umount() correct. https://patchwork.ozlabs.org/patch/1235811/ Kind regards, Petr ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-02-17 9:43 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-07 14:41 [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() Petr Vorel 2020-02-07 14:41 ` [LTP] [PATCH v6 2/2] syscalls/fsmount01: Add test for new mount API v5.2 Petr Vorel 2020-02-16 7:32 ` Li Wang 2020-02-16 13:17 ` Petr Vorel 2020-02-17 6:48 ` Li Wang 2020-02-17 7:51 ` Petr Vorel 2020-02-17 9:16 ` Jan Stancek 2020-02-17 9:43 ` Petr Vorel 2020-02-17 7:17 ` Li Wang 2020-02-17 8:04 ` Viresh Kumar 2020-02-07 15:24 ` [LTP] [PATCH v6 1/2] safe_macros: Use tst_umount() in safe_umount() Cyril Hrubis 2020-02-07 15:47 ` Jan Stancek 2020-02-07 15:53 ` Petr Vorel 2020-02-07 15:56 ` Jan Stancek 2020-02-07 15:57 ` Cyril Hrubis 2020-02-07 16:10 ` Petr Vorel 2020-02-10 8:19 ` Jan Stancek 2020-02-10 12:44 ` 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.