| Message ID | 20221004090810.9023-1-pvorel@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] df01.sh: Use own fsfreeze implementation for XFS | expand |
On Tue, Oct 04, 2022 at 11:08:10AM +0200, Petr Vorel wrote: > df01.sh started to fail on XFS on certain configuration since mkfs.xfs > and kernel 5.19. Implement fsfreeze instead of introducing external ...since the introduction of background garbage collection on XFS in kernel 5.19. ;) > dependency. NOTE: implementation could fail on other filesystems > (EOPNOTSUPP on exfat, ntfs, vfat). > > Suggested-by: Darrick J. Wong <djwong@kernel.org> > Suggested-by: Eric Sandeen <sandeen@redhat.com> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > Hi, > > FYI the background of this issue: > https://lore.kernel.org/ltp/Yv5oaxsX6z2qxxF3@magnolia/ > https://lore.kernel.org/ltp/974cc110-d47e-5fae-af5f-e2e610720e2d@redhat.com/ > > @LTP developers: not sure if the consensus is to avoid LTP API > completely (even use it just with TST_NO_DEFAULT_MAIN), if required I > can rewrite to use it just to get SAFE_*() macros (like > testcases/lib/tst_checkpoint.c) or even with tst_test workarounds > (testcases/lib/tst_get_free_pids.c). > > Kind regards, > Petr > > testcases/commands/df/Makefile | 4 +- > testcases/commands/df/df01.sh | 3 ++ > testcases/commands/df/df01_fsfreeze.c | 55 +++++++++++++++++++++++++++ > 3 files changed, 61 insertions(+), 1 deletion(-) > create mode 100644 testcases/commands/df/df01_fsfreeze.c > > diff --git a/testcases/commands/df/Makefile b/testcases/commands/df/Makefile > index 2787bb43a..1e0b4283a 100644 > --- a/testcases/commands/df/Makefile > +++ b/testcases/commands/df/Makefile > @@ -1,11 +1,13 @@ > # SPDX-License-Identifier: GPL-2.0-or-later > +# Copyright (c) Linux Test Project, 2021-2022 > # Copyright (c) 2015 Fujitsu Ltd. > -# Author:Zhang Jin <jy_zhangjin@cn.fujitsu.com> > +# Author: Zhang Jin <jy_zhangjin@cn.fujitsu.com> > > top_srcdir ?= ../../.. > > include $(top_srcdir)/include/mk/env_pre.mk > > INSTALL_TARGETS := df01.sh > +MAKE_TARGETS := df01_fsfreeze > > include $(top_srcdir)/include/mk/generic_leaf_target.mk > diff --git a/testcases/commands/df/df01.sh b/testcases/commands/df/df01.sh > index ae0449c3c..c59d2a01d 100755 > --- a/testcases/commands/df/df01.sh > +++ b/testcases/commands/df/df01.sh > @@ -46,6 +46,9 @@ df_test() > > ROD_SILENT rm -rf $TST_MNTPOINT/testimg > > + # ensure free space change can be seen by statfs > + [ "$fs" = "xfs" ] && ROD_SILENT df01_fsfreeze $TST_MNTPOINT > + > # flush file system buffers, then we can get the actual sizes. > sync > } > diff --git a/testcases/commands/df/df01_fsfreeze.c b/testcases/commands/df/df01_fsfreeze.c > new file mode 100644 > index 000000000..d47e1b01a > --- /dev/null > +++ b/testcases/commands/df/df01_fsfreeze.c > @@ -0,0 +1,55 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2010 Hajime Taira <htaira@redhat.com> > + * Copyright (c) 2010 Masatake Yamato <yamato@redhat.com> > + * Copyright (c) 2022 Petr Vorel <pvorel@suse.cz> > + */ > + > +#include <errno.h> > +#include <fcntl.h> > +#include <linux/fs.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <sys/ioctl.h> > +#include <sys/stat.h> > +#include <unistd.h> > + > +#define err_exit(...) ({ \ > + fprintf(stderr, __VA_ARGS__); \ > + if (errno) \ > + fprintf(stderr, ": %s (%d)", strerror(errno), errno); \ > + fprintf(stderr, "\n"); \ > + exit(EXIT_FAILURE); \ > +}) > + > +int main(int argc, char *argv[]) > +{ > + int fd; > + struct stat sb; > + > + if (argc < 2) > + err_exit("USAGE: df01_fsfreeze <mountpoint>"); > + > + fd = open(argv[1], O_RDONLY); > + if (fd < 0) > + err_exit("open '%s' failed", argv[1]); > + > + if (fstat(fd, &sb) == -1) > + err_exit("stat of '%s' failed", argv[1]); > + > + if (!S_ISDIR(sb.st_mode)) Note that XFS is perfectly happy to let you call FIFREEZE on a nondirectory. > + err_exit("%s: is not a directory", argv[1]); > + > + if (ioctl(fd, FIFREEZE, 0) < 0) > + err_exit("ioctl FIFREEZE on '%s' failed", argv[1]); > + > + usleep(100); The usleep shouldn't be necessary here. All the work necessary (background gc flushing, log quiescing, etc.) to stabilize the free space counters are performed synchronously before the FIFREEZE ioctl returns. If that's not been your experience, please let us know. --D > + if (ioctl(fd, FITHAW, 0) < 0) > + err_exit("ioctl FITHAW on '%s' failed", argv[1]); > + > + close(fd); > + > + return EXIT_SUCCESS; > +} > -- > 2.37.3 >
Hi Darrick, all, > On Tue, Oct 04, 2022 at 11:08:10AM +0200, Petr Vorel wrote: > > df01.sh started to fail on XFS on certain configuration since mkfs.xfs > > and kernel 5.19. Implement fsfreeze instead of introducing external > ...since the introduction of background garbage collection on XFS in > kernel 5.19. ;) Thank you, I'll update the commit message before merge. > > dependency. NOTE: implementation could fail on other filesystems > > (EOPNOTSUPP on exfat, ntfs, vfat). > > Suggested-by: Darrick J. Wong <djwong@kernel.org> > > Suggested-by: Eric Sandeen <sandeen@redhat.com> > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > > --- > > Hi, > > FYI the background of this issue: > > https://lore.kernel.org/ltp/Yv5oaxsX6z2qxxF3@magnolia/ > > https://lore.kernel.org/ltp/974cc110-d47e-5fae-af5f-e2e610720e2d@redhat.com/ > > @LTP developers: not sure if the consensus is to avoid LTP API > > completely (even use it just with TST_NO_DEFAULT_MAIN), if required I > > can rewrite to use it just to get SAFE_*() macros (like > > testcases/lib/tst_checkpoint.c) or even with tst_test workarounds > > (testcases/lib/tst_get_free_pids.c). > > Kind regards, > > Petr > > testcases/commands/df/Makefile | 4 +- > > testcases/commands/df/df01.sh | 3 ++ > > testcases/commands/df/df01_fsfreeze.c | 55 +++++++++++++++++++++++++++ > > 3 files changed, 61 insertions(+), 1 deletion(-) > > create mode 100644 testcases/commands/df/df01_fsfreeze.c > > diff --git a/testcases/commands/df/Makefile b/testcases/commands/df/Makefile > > index 2787bb43a..1e0b4283a 100644 > > --- a/testcases/commands/df/Makefile > > +++ b/testcases/commands/df/Makefile > > @@ -1,11 +1,13 @@ > > # SPDX-License-Identifier: GPL-2.0-or-later > > +# Copyright (c) Linux Test Project, 2021-2022 > > # Copyright (c) 2015 Fujitsu Ltd. > > -# Author:Zhang Jin <jy_zhangjin@cn.fujitsu.com> > > +# Author: Zhang Jin <jy_zhangjin@cn.fujitsu.com> > > top_srcdir ?= ../../.. > > include $(top_srcdir)/include/mk/env_pre.mk > > INSTALL_TARGETS := df01.sh > > +MAKE_TARGETS := df01_fsfreeze > > include $(top_srcdir)/include/mk/generic_leaf_target.mk > > diff --git a/testcases/commands/df/df01.sh b/testcases/commands/df/df01.sh > > index ae0449c3c..c59d2a01d 100755 > > --- a/testcases/commands/df/df01.sh > > +++ b/testcases/commands/df/df01.sh > > @@ -46,6 +46,9 @@ df_test() > > ROD_SILENT rm -rf $TST_MNTPOINT/testimg > > + # ensure free space change can be seen by statfs > > + [ "$fs" = "xfs" ] && ROD_SILENT df01_fsfreeze $TST_MNTPOINT > > + > > # flush file system buffers, then we can get the actual sizes. > > sync > > } > > diff --git a/testcases/commands/df/df01_fsfreeze.c b/testcases/commands/df/df01_fsfreeze.c > > new file mode 100644 > > index 000000000..d47e1b01a > > --- /dev/null > > +++ b/testcases/commands/df/df01_fsfreeze.c > > @@ -0,0 +1,55 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright (c) 2010 Hajime Taira <htaira@redhat.com> > > + * Copyright (c) 2010 Masatake Yamato <yamato@redhat.com> > > + * Copyright (c) 2022 Petr Vorel <pvorel@suse.cz> > > + */ > > + > > +#include <errno.h> > > +#include <fcntl.h> > > +#include <linux/fs.h> > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > +#include <sys/ioctl.h> > > +#include <sys/stat.h> > > +#include <unistd.h> > > + > > +#define err_exit(...) ({ \ > > + fprintf(stderr, __VA_ARGS__); \ > > + if (errno) \ > > + fprintf(stderr, ": %s (%d)", strerror(errno), errno); \ > > + fprintf(stderr, "\n"); \ > > + exit(EXIT_FAILURE); \ > > +}) > > + > > +int main(int argc, char *argv[]) > > +{ > > + int fd; > > + struct stat sb; > > + > > + if (argc < 2) > > + err_exit("USAGE: df01_fsfreeze <mountpoint>"); > > + > > + fd = open(argv[1], O_RDONLY); > > + if (fd < 0) > > + err_exit("open '%s' failed", argv[1]); > > + > > + if (fstat(fd, &sb) == -1) > > + err_exit("stat of '%s' failed", argv[1]); > > + > > + if (!S_ISDIR(sb.st_mode)) > Note that XFS is perfectly happy to let you call FIFREEZE on a > nondirectory. Thanks for info. I added this because the tool is intended to be used on $TST_MNTPOINT, which is directory. But as not needed, I might remove it. > > + err_exit("%s: is not a directory", argv[1]); > > + > > + if (ioctl(fd, FIFREEZE, 0) < 0) > > + err_exit("ioctl FIFREEZE on '%s' failed", argv[1]); > > + > > + usleep(100); > The usleep shouldn't be necessary here. All the work necessary > (background gc flushing, log quiescing, etc.) to stabilize the free > space counters are performed synchronously before the FIFREEZE ioctl > returns. > If that's not been your experience, please let us know. Indeed usleep() is not needed. I added it when I was debugging failures on other filesystems. Kind regards, Petr > --D > > + if (ioctl(fd, FITHAW, 0) < 0) > > + err_exit("ioctl FITHAW on '%s' failed", argv[1]); > > + > > + close(fd); > > + > > + return EXIT_SUCCESS; > > +} > > -- > > 2.37.3
Hello, Petr Vorel <pvorel@suse.cz> writes: > df01.sh started to fail on XFS on certain configuration since mkfs.xfs > and kernel 5.19. Implement fsfreeze instead of introducing external > dependency. NOTE: implementation could fail on other filesystems > (EOPNOTSUPP on exfat, ntfs, vfat). > > Suggested-by: Darrick J. Wong <djwong@kernel.org> > Suggested-by: Eric Sandeen <sandeen@redhat.com> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > Hi, > > FYI the background of this issue: > https://lore.kernel.org/ltp/Yv5oaxsX6z2qxxF3@magnolia/ > https://lore.kernel.org/ltp/974cc110-d47e-5fae-af5f-e2e610720e2d@redhat.com/ > > @LTP developers: not sure if the consensus is to avoid LTP API > completely (even use it just with TST_NO_DEFAULT_MAIN), if required I Why would that be the consensus? :-) > can rewrite to use it just to get SAFE_*() macros (like > testcases/lib/tst_checkpoint.c) or even with tst_test workarounds > (testcases/lib/tst_get_free_pids.c). Yes, it should work fine with TST_NO_DEFAULT_MAIN > > Kind regards, > Petr > > testcases/commands/df/Makefile | 4 +- > testcases/commands/df/df01.sh | 3 ++ > testcases/commands/df/df01_fsfreeze.c | 55 +++++++++++++++++++++++++++ > 3 files changed, 61 insertions(+), 1 deletion(-) > create mode 100644 testcases/commands/df/df01_fsfreeze.c > > diff --git a/testcases/commands/df/Makefile b/testcases/commands/df/Makefile > index 2787bb43a..1e0b4283a 100644 > --- a/testcases/commands/df/Makefile > +++ b/testcases/commands/df/Makefile > @@ -1,11 +1,13 @@ > # SPDX-License-Identifier: GPL-2.0-or-later > +# Copyright (c) Linux Test Project, 2021-2022 > # Copyright (c) 2015 Fujitsu Ltd. > -# Author:Zhang Jin <jy_zhangjin@cn.fujitsu.com> > +# Author: Zhang Jin <jy_zhangjin@cn.fujitsu.com> > > top_srcdir ?= ../../.. > > include $(top_srcdir)/include/mk/env_pre.mk > > INSTALL_TARGETS := df01.sh > +MAKE_TARGETS := df01_fsfreeze > > include $(top_srcdir)/include/mk/generic_leaf_target.mk > diff --git a/testcases/commands/df/df01.sh b/testcases/commands/df/df01.sh > index ae0449c3c..c59d2a01d 100755 > --- a/testcases/commands/df/df01.sh > +++ b/testcases/commands/df/df01.sh > @@ -46,6 +46,9 @@ df_test() > > ROD_SILENT rm -rf $TST_MNTPOINT/testimg > > + # ensure free space change can be seen by statfs > + [ "$fs" = "xfs" ] && ROD_SILENT df01_fsfreeze $TST_MNTPOINT > + > # flush file system buffers, then we can get the actual sizes. > sync > } > diff --git a/testcases/commands/df/df01_fsfreeze.c b/testcases/commands/df/df01_fsfreeze.c > new file mode 100644 > index 000000000..d47e1b01a > --- /dev/null > +++ b/testcases/commands/df/df01_fsfreeze.c > @@ -0,0 +1,55 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2010 Hajime Taira <htaira@redhat.com> > + * Copyright (c) 2010 Masatake Yamato <yamato@redhat.com> > + * Copyright (c) 2022 Petr Vorel <pvorel@suse.cz> > + */ > + > +#include <errno.h> > +#include <fcntl.h> > +#include <linux/fs.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <sys/ioctl.h> > +#include <sys/stat.h> > +#include <unistd.h> > + > +#define err_exit(...) ({ \ > + fprintf(stderr, __VA_ARGS__); \ > + if (errno) \ > + fprintf(stderr, ": %s (%d)", strerror(errno), errno); \ > + fprintf(stderr, "\n"); \ > + exit(EXIT_FAILURE); \ > +}) > + > +int main(int argc, char *argv[]) > +{ > + int fd; > + struct stat sb; > + > + if (argc < 2) > + err_exit("USAGE: df01_fsfreeze <mountpoint>"); > + > + fd = open(argv[1], O_RDONLY); > + if (fd < 0) > + err_exit("open '%s' failed", argv[1]); > + > + if (fstat(fd, &sb) == -1) > + err_exit("stat of '%s' failed", argv[1]); > + > + if (!S_ISDIR(sb.st_mode)) > + err_exit("%s: is not a directory", argv[1]); > + > + if (ioctl(fd, FIFREEZE, 0) < 0) > + err_exit("ioctl FIFREEZE on '%s' failed", argv[1]); > + > + usleep(100); > + > + if (ioctl(fd, FITHAW, 0) < 0) > + err_exit("ioctl FITHAW on '%s' failed", argv[1]); > + > + close(fd); > + > + return EXIT_SUCCESS; > +} > -- > 2.37.3
Hi Richie, > Hello, > Petr Vorel <pvorel@suse.cz> writes: > > df01.sh started to fail on XFS on certain configuration since mkfs.xfs > > and kernel 5.19. Implement fsfreeze instead of introducing external > > dependency. NOTE: implementation could fail on other filesystems > > (EOPNOTSUPP on exfat, ntfs, vfat). > > Suggested-by: Darrick J. Wong <djwong@kernel.org> > > Suggested-by: Eric Sandeen <sandeen@redhat.com> > > Signed-off-by: Petr Vorel <pvorel@suse.cz> > > --- > > Hi, > > FYI the background of this issue: > > https://lore.kernel.org/ltp/Yv5oaxsX6z2qxxF3@magnolia/ > > https://lore.kernel.org/ltp/974cc110-d47e-5fae-af5f-e2e610720e2d@redhat.com/ > > @LTP developers: not sure if the consensus is to avoid LTP API > > completely (even use it just with TST_NO_DEFAULT_MAIN), if required I > Why would that be the consensus? :-) $ ls testcases/lib/*.c |wc -l 19 $ git grep -l TST_NO_DEFAULT_MAIN testcases/lib/*.c |wc -l 9 => 10 tests not use tst_test.h at all. => none is *not* defining TST_NO_DEFAULT_MAIN (not a big surprise), but 2 of them (testcases/lib/tst_device.c, testcases/lib/tst_get_free_pids.c) implement workaround to force messages to be printed from the new library (tst_test.c). static struct tst_test test = { }; tst_test = &test; My opinion also was based on Cyril's comments on nfs05_make_tree.c patch, but he probably meant to just use TST_NO_DEFAULT_MAIN instead of struct tst_test test: https://lore.kernel.org/ltp/YqxFo1iFzHatNRIl@yuki/ > > can rewrite to use it just to get SAFE_*() macros (like > > testcases/lib/tst_checkpoint.c) or even with tst_test workarounds > > (testcases/lib/tst_get_free_pids.c). > Yes, it should work fine with TST_NO_DEFAULT_MAIN Both versions IMHO work well, the question what we prefer more. Do you vote for rewriting? Kind regards, Petr > > Kind regards, > > Petr > > testcases/commands/df/Makefile | 4 +- > > testcases/commands/df/df01.sh | 3 ++ > > testcases/commands/df/df01_fsfreeze.c | 55 +++++++++++++++++++++++++++ > > 3 files changed, 61 insertions(+), 1 deletion(-) > > create mode 100644 testcases/commands/df/df01_fsfreeze.c > > diff --git a/testcases/commands/df/Makefile b/testcases/commands/df/Makefile > > index 2787bb43a..1e0b4283a 100644 > > --- a/testcases/commands/df/Makefile > > +++ b/testcases/commands/df/Makefile > > @@ -1,11 +1,13 @@ > > # SPDX-License-Identifier: GPL-2.0-or-later > > +# Copyright (c) Linux Test Project, 2021-2022 > > # Copyright (c) 2015 Fujitsu Ltd. > > -# Author:Zhang Jin <jy_zhangjin@cn.fujitsu.com> > > +# Author: Zhang Jin <jy_zhangjin@cn.fujitsu.com> > > top_srcdir ?= ../../.. > > include $(top_srcdir)/include/mk/env_pre.mk > > INSTALL_TARGETS := df01.sh > > +MAKE_TARGETS := df01_fsfreeze > > include $(top_srcdir)/include/mk/generic_leaf_target.mk > > diff --git a/testcases/commands/df/df01.sh b/testcases/commands/df/df01.sh > > index ae0449c3c..c59d2a01d 100755 > > --- a/testcases/commands/df/df01.sh > > +++ b/testcases/commands/df/df01.sh > > @@ -46,6 +46,9 @@ df_test() > > ROD_SILENT rm -rf $TST_MNTPOINT/testimg > > + # ensure free space change can be seen by statfs > > + [ "$fs" = "xfs" ] && ROD_SILENT df01_fsfreeze $TST_MNTPOINT > > + > > # flush file system buffers, then we can get the actual sizes. > > sync > > } > > diff --git a/testcases/commands/df/df01_fsfreeze.c b/testcases/commands/df/df01_fsfreeze.c > > new file mode 100644 > > index 000000000..d47e1b01a > > --- /dev/null > > +++ b/testcases/commands/df/df01_fsfreeze.c > > @@ -0,0 +1,55 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +/* > > + * Copyright (c) 2010 Hajime Taira <htaira@redhat.com> > > + * Copyright (c) 2010 Masatake Yamato <yamato@redhat.com> > > + * Copyright (c) 2022 Petr Vorel <pvorel@suse.cz> > > + */ > > + > > +#include <errno.h> > > +#include <fcntl.h> > > +#include <linux/fs.h> > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > +#include <sys/ioctl.h> > > +#include <sys/stat.h> > > +#include <unistd.h> > > + > > +#define err_exit(...) ({ \ > > + fprintf(stderr, __VA_ARGS__); \ > > + if (errno) \ > > + fprintf(stderr, ": %s (%d)", strerror(errno), errno); \ > > + fprintf(stderr, "\n"); \ > > + exit(EXIT_FAILURE); \ > > +}) > > + > > +int main(int argc, char *argv[]) > > +{ > > + int fd; > > + struct stat sb; > > + > > + if (argc < 2) > > + err_exit("USAGE: df01_fsfreeze <mountpoint>"); > > + > > + fd = open(argv[1], O_RDONLY); > > + if (fd < 0) > > + err_exit("open '%s' failed", argv[1]); > > + > > + if (fstat(fd, &sb) == -1) > > + err_exit("stat of '%s' failed", argv[1]); > > + > > + if (!S_ISDIR(sb.st_mode)) > > + err_exit("%s: is not a directory", argv[1]); > > + > > + if (ioctl(fd, FIFREEZE, 0) < 0) > > + err_exit("ioctl FIFREEZE on '%s' failed", argv[1]); > > + > > + usleep(100); > > + > > + if (ioctl(fd, FITHAW, 0) < 0) > > + err_exit("ioctl FITHAW on '%s' failed", argv[1]); > > + > > + close(fd); > > + > > + return EXIT_SUCCESS; > > +} > > -- > > 2.37.3
Hello, Petr Vorel <pvorel@suse.cz> writes: > Hi Richie, > >> Hello, > >> Petr Vorel <pvorel@suse.cz> writes: > >> > df01.sh started to fail on XFS on certain configuration since mkfs.xfs >> > and kernel 5.19. Implement fsfreeze instead of introducing external >> > dependency. NOTE: implementation could fail on other filesystems >> > (EOPNOTSUPP on exfat, ntfs, vfat). > >> > Suggested-by: Darrick J. Wong <djwong@kernel.org> >> > Suggested-by: Eric Sandeen <sandeen@redhat.com> >> > Signed-off-by: Petr Vorel <pvorel@suse.cz> >> > --- >> > Hi, > >> > FYI the background of this issue: >> > https://lore.kernel.org/ltp/Yv5oaxsX6z2qxxF3@magnolia/ >> > https://lore.kernel.org/ltp/974cc110-d47e-5fae-af5f-e2e610720e2d@redhat.com/ > >> > @LTP developers: not sure if the consensus is to avoid LTP API >> > completely (even use it just with TST_NO_DEFAULT_MAIN), if required I > >> Why would that be the consensus? :-) > > $ ls testcases/lib/*.c |wc -l > 19 > > $ git grep -l TST_NO_DEFAULT_MAIN testcases/lib/*.c |wc -l > 9 > > => 10 tests not use tst_test.h at all. > => none is *not* defining TST_NO_DEFAULT_MAIN (not a big surprise), > but 2 of them (testcases/lib/tst_device.c, testcases/lib/tst_get_free_pids.c) > implement workaround to force messages to be printed from the new library > (tst_test.c). Possibly the reason for this is that it's not clear whether some core library functions will work as expected if we create an executable with TST_NO_DEFAULT_MAIN. However most stuff works fine. > > static struct tst_test test = { > }; > tst_test = &test; > > My opinion also was based on Cyril's comments on nfs05_make_tree.c patch, but he > probably meant to just use TST_NO_DEFAULT_MAIN instead of struct tst_test test: > https://lore.kernel.org/ltp/YqxFo1iFzHatNRIl@yuki/ Certainly we shouldn't put a test struct in anything which is not a test. Possibly we could create a util struct > >> > can rewrite to use it just to get SAFE_*() macros (like >> > testcases/lib/tst_checkpoint.c) or even with tst_test workarounds >> > (testcases/lib/tst_get_free_pids.c). > >> Yes, it should work fine with TST_NO_DEFAULT_MAIN > Both versions IMHO work well, the question what we prefer more. > Do you vote for rewriting? Yes, avoiding the LTP library caused a number of problems in sparse-ltp and the ltx prototype. Then I found linking in the LTP libs with TST_NO_DEFAULT_MAIN to ltx and using tst_res(TBROK, ...) etc. worked fine.
> Hello, > Petr Vorel <pvorel@suse.cz> writes: > > Hi Richie, > >> Hello, > >> Petr Vorel <pvorel@suse.cz> writes: > >> > df01.sh started to fail on XFS on certain configuration since mkfs.xfs > >> > and kernel 5.19. Implement fsfreeze instead of introducing external > >> > dependency. NOTE: implementation could fail on other filesystems > >> > (EOPNOTSUPP on exfat, ntfs, vfat). > >> > Suggested-by: Darrick J. Wong <djwong@kernel.org> > >> > Suggested-by: Eric Sandeen <sandeen@redhat.com> > >> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > >> > --- > >> > Hi, > >> > FYI the background of this issue: > >> > https://lore.kernel.org/ltp/Yv5oaxsX6z2qxxF3@magnolia/ > >> > https://lore.kernel.org/ltp/974cc110-d47e-5fae-af5f-e2e610720e2d@redhat.com/ > >> > @LTP developers: not sure if the consensus is to avoid LTP API > >> > completely (even use it just with TST_NO_DEFAULT_MAIN), if required I > >> Why would that be the consensus? :-) > > $ ls testcases/lib/*.c |wc -l > > 19 > > $ git grep -l TST_NO_DEFAULT_MAIN testcases/lib/*.c |wc -l > > 9 > > => 10 tests not use tst_test.h at all. > > => none is *not* defining TST_NO_DEFAULT_MAIN (not a big surprise), > > but 2 of them (testcases/lib/tst_device.c, testcases/lib/tst_get_free_pids.c) > > implement workaround to force messages to be printed from the new library > > (tst_test.c). > Possibly the reason for this is that it's not clear whether some core > library functions will work as expected if we create an executable with > TST_NO_DEFAULT_MAIN. > However most stuff works fine. > > static struct tst_test test = { > > }; > > tst_test = &test; > > My opinion also was based on Cyril's comments on nfs05_make_tree.c patch, but he > > probably meant to just use TST_NO_DEFAULT_MAIN instead of struct tst_test test: > > https://lore.kernel.org/ltp/YqxFo1iFzHatNRIl@yuki/ > Certainly we shouldn't put a test struct in anything which is not a > test. Possibly we could create a util struct > >> > can rewrite to use it just to get SAFE_*() macros (like > >> > testcases/lib/tst_checkpoint.c) or even with tst_test workarounds > >> > (testcases/lib/tst_get_free_pids.c). > >> Yes, it should work fine with TST_NO_DEFAULT_MAIN > > Both versions IMHO work well, the question what we prefer more. > > Do you vote for rewriting? > Yes, avoiding the LTP library caused a number of problems in sparse-ltp > and the ltx prototype. Then I found linking in the LTP libs with > TST_NO_DEFAULT_MAIN to ltx and using tst_res(TBROK, ...) etc. worked > fine. Well, this simple utility works without LTP library. It's more a matter of style. OK, I'll send TST_NO_DEFAULT_MAIN version and let the community decide. Kind regards, Petr
diff --git a/testcases/commands/df/Makefile b/testcases/commands/df/Makefile index 2787bb43a..1e0b4283a 100644 --- a/testcases/commands/df/Makefile +++ b/testcases/commands/df/Makefile @@ -1,11 +1,13 @@ # SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (c) Linux Test Project, 2021-2022 # Copyright (c) 2015 Fujitsu Ltd. -# Author:Zhang Jin <jy_zhangjin@cn.fujitsu.com> +# Author: Zhang Jin <jy_zhangjin@cn.fujitsu.com> top_srcdir ?= ../../.. include $(top_srcdir)/include/mk/env_pre.mk INSTALL_TARGETS := df01.sh +MAKE_TARGETS := df01_fsfreeze include $(top_srcdir)/include/mk/generic_leaf_target.mk diff --git a/testcases/commands/df/df01.sh b/testcases/commands/df/df01.sh index ae0449c3c..c59d2a01d 100755 --- a/testcases/commands/df/df01.sh +++ b/testcases/commands/df/df01.sh @@ -46,6 +46,9 @@ df_test() ROD_SILENT rm -rf $TST_MNTPOINT/testimg + # ensure free space change can be seen by statfs + [ "$fs" = "xfs" ] && ROD_SILENT df01_fsfreeze $TST_MNTPOINT + # flush file system buffers, then we can get the actual sizes. sync } diff --git a/testcases/commands/df/df01_fsfreeze.c b/testcases/commands/df/df01_fsfreeze.c new file mode 100644 index 000000000..d47e1b01a --- /dev/null +++ b/testcases/commands/df/df01_fsfreeze.c @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2010 Hajime Taira <htaira@redhat.com> + * Copyright (c) 2010 Masatake Yamato <yamato@redhat.com> + * Copyright (c) 2022 Petr Vorel <pvorel@suse.cz> + */ + +#include <errno.h> +#include <fcntl.h> +#include <linux/fs.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/ioctl.h> +#include <sys/stat.h> +#include <unistd.h> + +#define err_exit(...) ({ \ + fprintf(stderr, __VA_ARGS__); \ + if (errno) \ + fprintf(stderr, ": %s (%d)", strerror(errno), errno); \ + fprintf(stderr, "\n"); \ + exit(EXIT_FAILURE); \ +}) + +int main(int argc, char *argv[]) +{ + int fd; + struct stat sb; + + if (argc < 2) + err_exit("USAGE: df01_fsfreeze <mountpoint>"); + + fd = open(argv[1], O_RDONLY); + if (fd < 0) + err_exit("open '%s' failed", argv[1]); + + if (fstat(fd, &sb) == -1) + err_exit("stat of '%s' failed", argv[1]); + + if (!S_ISDIR(sb.st_mode)) + err_exit("%s: is not a directory", argv[1]); + + if (ioctl(fd, FIFREEZE, 0) < 0) + err_exit("ioctl FIFREEZE on '%s' failed", argv[1]); + + usleep(100); + + if (ioctl(fd, FITHAW, 0) < 0) + err_exit("ioctl FITHAW on '%s' failed", argv[1]); + + close(fd); + + return EXIT_SUCCESS; +}
df01.sh started to fail on XFS on certain configuration since mkfs.xfs and kernel 5.19. Implement fsfreeze instead of introducing external dependency. NOTE: implementation could fail on other filesystems (EOPNOTSUPP on exfat, ntfs, vfat). Suggested-by: Darrick J. Wong <djwong@kernel.org> Suggested-by: Eric Sandeen <sandeen@redhat.com> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- Hi, FYI the background of this issue: https://lore.kernel.org/ltp/Yv5oaxsX6z2qxxF3@magnolia/ https://lore.kernel.org/ltp/974cc110-d47e-5fae-af5f-e2e610720e2d@redhat.com/ @LTP developers: not sure if the consensus is to avoid LTP API completely (even use it just with TST_NO_DEFAULT_MAIN), if required I can rewrite to use it just to get SAFE_*() macros (like testcases/lib/tst_checkpoint.c) or even with tst_test workarounds (testcases/lib/tst_get_free_pids.c). Kind regards, Petr testcases/commands/df/Makefile | 4 +- testcases/commands/df/df01.sh | 3 ++ testcases/commands/df/df01_fsfreeze.c | 55 +++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 testcases/commands/df/df01_fsfreeze.c