diff mbox series

common/rc: specifically make new xfs in _test_mkfs

Message ID 20210311121213.1348303-1-zlang@redhat.com (mailing list archive)
State New, archived
Headers show
Series common/rc: specifically make new xfs in _test_mkfs | expand

Commit Message

Zorro Lang March 11, 2021, 12:12 p.m. UTC
When we test different filesystems with different sections in config
file, always hit below failure:
   ...
   our local _test_mkfs routine ...
   mkfs.xfs: /dev/vdb1 appears to contain an existing filesystem (xfs)
   mkfs.xfs: Use the -f option to force overwrite.
   check: failed to mkfs $TEST_DEV using specified options
   ...

To fix this problem, change the _test_mkfs to deal with xfs creation
properly. Due to xfstests uses _test_mkfs rarely, currently only in
check program, so I don't intend to write a whole new series of
_test_mkfs_xfs_* functions likes what _scratch_mkfs_xfs_* does.

Reported-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Zorro Lang <zlang@redhat.com>
---
 common/rc | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Eryu Guan March 14, 2021, 11:42 a.m. UTC | #1
On Thu, Mar 11, 2021 at 08:12:13PM +0800, Zorro Lang wrote:
> When we test different filesystems with different sections in config
> file, always hit below failure:
>    ...
>    our local _test_mkfs routine ...
>    mkfs.xfs: /dev/vdb1 appears to contain an existing filesystem (xfs)
>    mkfs.xfs: Use the -f option to force overwrite.
>    check: failed to mkfs $TEST_DEV using specified options
>    ...
> 
> To fix this problem, change the _test_mkfs to deal with xfs creation
> properly. Due to xfstests uses _test_mkfs rarely, currently only in
> check program, so I don't intend to write a whole new series of
> _test_mkfs_xfs_* functions likes what _scratch_mkfs_xfs_* does.
> 
> Reported-by: Carlos Maiolino <cmaiolino@redhat.com>
> Signed-off-by: Zorro Lang <zlang@redhat.com>

I also have defined different test sections in config file to test
different filesysms, but I didn't hit this failure. And it turns out
that I have "-f" in MKFS_OPTIONS, e.g.

	[xfs_prjquota_crc]
	FSTYP=xfs
	MKFS_OPTIONS="-f -b size=4k -m crc=1"
	MOUNT_OPTIONS="-o prjquota"

	[xfs_allquota_reflink]
	FSTYP=xfs
	MKFS_OPTIONS="-f -b size=4k -m crc=1,reflink=1,rmapbt=1"
	MOUNT_OPTIONS="-o usrquota,grpquota,prjquota"

And I think that's the way to fix it.

Thanks,
Eryu

> ---
>  common/rc | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 0ce3cb0d..c783def7 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -258,6 +258,7 @@ _test_options()
>  
>      case $type in
>      mkfs)
> +	TEST_OPTIONS="$TEST_OPTIONS -f"
>  	rt_opt="-r"
>          log_opt="-l"
>  	;;
> @@ -633,6 +634,15 @@ _test_mkfs()
>      ext2|ext3|ext4)
>  	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* $TEST_DEV
>  	;;
> +    xfs)
> +	local mkfs_opts=$*
> +
> +	if [ -n "$XFS_MKFS_HAS_NO_META_SUPPORT" ]; then
> +		mkfs_opts=`echo $mkfs_opts | sed "s/-m\s\+\S\+//g"`
> +	fi
> +	_test_options mkfs
> +	$MKFS_XFS_PROG $TEST_OPTIONS $MKFS_OPTIONS $mkfs_opts $TEST_DEV
> +	;;
>      *)
>  	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $TEST_DEV
>  	;;
> -- 
> 2.29.2
Zorro Lang March 14, 2021, 2:34 p.m. UTC | #2
On Sun, Mar 14, 2021 at 07:42:06PM +0800, Eryu Guan wrote:
> On Thu, Mar 11, 2021 at 08:12:13PM +0800, Zorro Lang wrote:
> > When we test different filesystems with different sections in config
> > file, always hit below failure:
> >    ...
> >    our local _test_mkfs routine ...
> >    mkfs.xfs: /dev/vdb1 appears to contain an existing filesystem (xfs)
> >    mkfs.xfs: Use the -f option to force overwrite.
> >    check: failed to mkfs $TEST_DEV using specified options
> >    ...
> > 
> > To fix this problem, change the _test_mkfs to deal with xfs creation
> > properly. Due to xfstests uses _test_mkfs rarely, currently only in
> > check program, so I don't intend to write a whole new series of
> > _test_mkfs_xfs_* functions likes what _scratch_mkfs_xfs_* does.
> > 
> > Reported-by: Carlos Maiolino <cmaiolino@redhat.com>
> > Signed-off-by: Zorro Lang <zlang@redhat.com>
> 
> I also have defined different test sections in config file to test
> different filesysms, but I didn't hit this failure. And it turns out
> that I have "-f" in MKFS_OPTIONS, e.g.
> 
> 	[xfs_prjquota_crc]
> 	FSTYP=xfs
> 	MKFS_OPTIONS="-f -b size=4k -m crc=1"
> 	MOUNT_OPTIONS="-o prjquota"
> 
> 	[xfs_allquota_reflink]
> 	FSTYP=xfs
> 	MKFS_OPTIONS="-f -b size=4k -m crc=1,reflink=1,rmapbt=1"
> 	MOUNT_OPTIONS="-o usrquota,grpquota,prjquota"

Yes, set "-f" in MKFS_OPTIONS will help.

BTW, according to below logic:

    if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
        ...
        if ! _test_mkfs >$tmp.err 2>&1

Your TEST_DEV might not be re-created, except you set RECREATE_TEST_DEV=ture.
I hit this failure due to I set:
[ext4]
FSTYP=ext4
...

[xfs]
FSTYP=xfs
...

And ext4 use "-F" by default, but xfs doesn't.

    ext2|ext3|ext4)
        $MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* $TEST_DEV

This patch isn't necessary, due to there's workaround. I'm fine if you'd like
to Nack it.

Thanks,
Zorro

> 
> And I think that's the way to fix it.
> 
> Thanks,
> Eryu
> 
> > ---
> >  common/rc | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/common/rc b/common/rc
> > index 0ce3cb0d..c783def7 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -258,6 +258,7 @@ _test_options()
> >  
> >      case $type in
> >      mkfs)
> > +	TEST_OPTIONS="$TEST_OPTIONS -f"
> >  	rt_opt="-r"
> >          log_opt="-l"
> >  	;;
> > @@ -633,6 +634,15 @@ _test_mkfs()
> >      ext2|ext3|ext4)
> >  	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* $TEST_DEV
> >  	;;
> > +    xfs)
> > +	local mkfs_opts=$*
> > +
> > +	if [ -n "$XFS_MKFS_HAS_NO_META_SUPPORT" ]; then
> > +		mkfs_opts=`echo $mkfs_opts | sed "s/-m\s\+\S\+//g"`
> > +	fi
> > +	_test_options mkfs
> > +	$MKFS_XFS_PROG $TEST_OPTIONS $MKFS_OPTIONS $mkfs_opts $TEST_DEV
> > +	;;
> >      *)
> >  	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $TEST_DEV
> >  	;;
> > -- 
> > 2.29.2
>
Eryu Guan March 14, 2021, 3:58 p.m. UTC | #3
On Sun, Mar 14, 2021 at 10:34:59PM +0800, Zorro Lang wrote:
> On Sun, Mar 14, 2021 at 07:42:06PM +0800, Eryu Guan wrote:
> > On Thu, Mar 11, 2021 at 08:12:13PM +0800, Zorro Lang wrote:
> > > When we test different filesystems with different sections in config
> > > file, always hit below failure:
> > >    ...
> > >    our local _test_mkfs routine ...
> > >    mkfs.xfs: /dev/vdb1 appears to contain an existing filesystem (xfs)
> > >    mkfs.xfs: Use the -f option to force overwrite.
> > >    check: failed to mkfs $TEST_DEV using specified options
> > >    ...
> > > 
> > > To fix this problem, change the _test_mkfs to deal with xfs creation
> > > properly. Due to xfstests uses _test_mkfs rarely, currently only in
> > > check program, so I don't intend to write a whole new series of
> > > _test_mkfs_xfs_* functions likes what _scratch_mkfs_xfs_* does.
> > > 
> > > Reported-by: Carlos Maiolino <cmaiolino@redhat.com>
> > > Signed-off-by: Zorro Lang <zlang@redhat.com>
> > 
> > I also have defined different test sections in config file to test
> > different filesysms, but I didn't hit this failure. And it turns out
> > that I have "-f" in MKFS_OPTIONS, e.g.
> > 
> > 	[xfs_prjquota_crc]
> > 	FSTYP=xfs
> > 	MKFS_OPTIONS="-f -b size=4k -m crc=1"
> > 	MOUNT_OPTIONS="-o prjquota"
> > 
> > 	[xfs_allquota_reflink]
> > 	FSTYP=xfs
> > 	MKFS_OPTIONS="-f -b size=4k -m crc=1,reflink=1,rmapbt=1"
> > 	MOUNT_OPTIONS="-o usrquota,grpquota,prjquota"
> 
> Yes, set "-f" in MKFS_OPTIONS will help.
> 
> BTW, according to below logic:
> 
>     if $RECREATE_TEST_DEV || [ "$OLD_FSTYP" != "$FSTYP" ]; then
>         ...
>         if ! _test_mkfs >$tmp.err 2>&1
> 
> Your TEST_DEV might not be re-created, except you set RECREATE_TEST_DEV=ture.

Yes, I also have RECREATE_TEST_DEV=true set in my local.config

> I hit this failure due to I set:
> [ext4]
> FSTYP=ext4
> ...
> 
> [xfs]
> FSTYP=xfs
> ...
> 
> And ext4 use "-F" by default, but xfs doesn't.
> 
>     ext2|ext3|ext4)
>         $MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* $TEST_DEV
> 
> This patch isn't necessary, due to there's workaround. I'm fine if you'd like
> to Nack it.

Agreed, not taken :)

Thanks,
Eryu
diff mbox series

Patch

diff --git a/common/rc b/common/rc
index 0ce3cb0d..c783def7 100644
--- a/common/rc
+++ b/common/rc
@@ -258,6 +258,7 @@  _test_options()
 
     case $type in
     mkfs)
+	TEST_OPTIONS="$TEST_OPTIONS -f"
 	rt_opt="-r"
         log_opt="-l"
 	;;
@@ -633,6 +634,15 @@  _test_mkfs()
     ext2|ext3|ext4)
 	$MKFS_PROG -t $FSTYP -- -F $MKFS_OPTIONS $* $TEST_DEV
 	;;
+    xfs)
+	local mkfs_opts=$*
+
+	if [ -n "$XFS_MKFS_HAS_NO_META_SUPPORT" ]; then
+		mkfs_opts=`echo $mkfs_opts | sed "s/-m\s\+\S\+//g"`
+	fi
+	_test_options mkfs
+	$MKFS_XFS_PROG $TEST_OPTIONS $MKFS_OPTIONS $mkfs_opts $TEST_DEV
+	;;
     *)
 	yes | $MKFS_PROG -t $FSTYP -- $MKFS_OPTIONS $* $TEST_DEV
 	;;