Re: [LTP] [PATCH 1/1] df01.sh: Use own fsfreeze implementation for XFS

[Petr Vorel <pvorel@suse.cz>]

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