[PATCHSET 0/4] fstests: random fixes

[PATCHSET 0/4] fstests: random fixes

[Darrick J. Wong]

Hi all,

Here are the usual weekly fixes for fstests.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes
---
 check             |    7 ++++++-
 tests/generic/570 |    2 +-
 tests/xfs/106     |    1 +
 tests/xfs/106.out |    5 +++++
 4 files changed, 13 insertions(+), 2 deletions(-)

[PATCH 1/4] xfs/106: fix golden output regression in quota off test

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

In commit 4c5df338, we reshuffled this test a bit in preparation to
disable quotaoff by rearranging the test to testing disabling of quota
by remounting the filesystem.  Unfortunately, extra blank lines were
added to the golden output, leading to test regressions.

The "extra" blank lines are a result of the "echo ; test_off";
test_off() itself doesn't print anything.  Make it print /something/ so
that we know what the test was trying to do when a particular line of
golden output appears, then fix the blank lines.

Fixes: 4c5df338 ("xfs/106: don't test disabling quota accounting")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/106     |    1 +
 tests/xfs/106.out |    5 +++++
 2 files changed, 6 insertions(+)


diff --git a/tests/xfs/106 b/tests/xfs/106
index d8f55441..fc2281af 100755
--- a/tests/xfs/106
+++ b/tests/xfs/106
@@ -155,6 +155,7 @@ test_enable()
 
 test_off()
 {
+	echo "turning quota off by remounting"
 	_scratch_unmount
 	_qmount_option ""
 	_qmount
diff --git a/tests/xfs/106.out b/tests/xfs/106.out
index 3e6805a6..7c7be6b1 100644
--- a/tests/xfs/106.out
+++ b/tests/xfs/106.out
@@ -149,6 +149,7 @@ Blocks grace time: [3 days]
 Inodes grace time: [3 days]
 Realtime Blocks grace time: [7 days]
 
+turning quota off by remounting
 
 checking remove command (type=u)
 User quota are not enabled on SCRATCH_DEV
@@ -327,6 +328,8 @@ Blocks grace time: [3 days]
 Inodes grace time: [3 days]
 Realtime Blocks grace time: [7 days]
 
+turning quota off by remounting
+
 checking remove command (type=g)
 Group quota are not enabled on SCRATCH_DEV
 
@@ -504,6 +507,8 @@ Blocks grace time: [3 days]
 Inodes grace time: [3 days]
 Realtime Blocks grace time: [7 days]
 
+turning quota off by remounting
+
 checking remove command (type=p)
 Project quota are not enabled on SCRATCH_DEV
 

[PATCH 2/4] check: don't leave the scratch filesystem mounted after _notrun

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Unmount the scratch filesystem if a test decides to _notrun itself
because _try_wipe_scratch_devs will not be able to wipe the scratch
device prior to the next test run.  We don't want to let scratch state
from one test leak into subsequent tests if we can help it.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 check |    5 +++++
 1 file changed, 5 insertions(+)


diff --git a/check b/check
index bb7e030c..5d71b74c 100755
--- a/check
+++ b/check
@@ -871,6 +871,11 @@ function run_section()
 			notrun="$notrun $seqnum"
 			n_notrun=`expr $n_notrun + 1`
 			tc_status="notrun"
+
+			# Unmount the scratch fs so that we can wipe the scratch
+			# dev state prior to the next test run.
+			_scratch_unmount 2> /dev/null
+			rm -f ${RESULT_DIR}/require_scratch*
 			continue;
 		fi
 

Re: [PATCH 2/4] check: don't leave the scratch filesystem mounted after _notrun

[Eryu Guan]

On Tue, Jul 27, 2021 at 05:09:48PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Unmount the scratch filesystem if a test decides to _notrun itself
> because _try_wipe_scratch_devs will not be able to wipe the scratch
> device prior to the next test run.  We don't want to let scratch state
> from one test leak into subsequent tests if we can help it.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  check |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> 
> diff --git a/check b/check
> index bb7e030c..5d71b74c 100755
> --- a/check
> +++ b/check
> @@ -871,6 +871,11 @@ function run_section()
>  			notrun="$notrun $seqnum"
>  			n_notrun=`expr $n_notrun + 1`
>  			tc_status="notrun"
> +
> +			# Unmount the scratch fs so that we can wipe the scratch
> +			# dev state prior to the next test run.
> +			_scratch_unmount 2> /dev/null
> +			rm -f ${RESULT_DIR}/require_scratch*

I think _notrun has removed $RESULT_DIR/require_scratch* already, and we
could remove above line. I'll remove it on commit.

Thanks,
Eryu

>  			continue;
>  		fi
>  

Re: [PATCH 2/4] check: don't leave the scratch filesystem mounted after _notrun

[Darrick J. Wong]

On Sun, Aug 01, 2021 at 09:02:13PM +0800, Eryu Guan wrote:
> On Tue, Jul 27, 2021 at 05:09:48PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Unmount the scratch filesystem if a test decides to _notrun itself
> > because _try_wipe_scratch_devs will not be able to wipe the scratch
> > device prior to the next test run.  We don't want to let scratch state
> > from one test leak into subsequent tests if we can help it.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  check |    5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > 
> > diff --git a/check b/check
> > index bb7e030c..5d71b74c 100755
> > --- a/check
> > +++ b/check
> > @@ -871,6 +871,11 @@ function run_section()
> >  			notrun="$notrun $seqnum"
> >  			n_notrun=`expr $n_notrun + 1`
> >  			tc_status="notrun"
> > +
> > +			# Unmount the scratch fs so that we can wipe the scratch
> > +			# dev state prior to the next test run.
> > +			_scratch_unmount 2> /dev/null
> > +			rm -f ${RESULT_DIR}/require_scratch*
> 
> I think _notrun has removed $RESULT_DIR/require_scratch* already, and we
> could remove above line. I'll remove it on commit.

Ok, thanks!

--D

> 
> Thanks,
> Eryu
> 
> >  			continue;
> >  		fi
> >  

[PATCH 3/4] generic/570: fix regression when SCRATCH_DEV is still formatted

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Newer versions of mkswap (or at least the one in util-linux 2.34)
complain to stderr when they're formatting over a device that seems to
contain existing data:

    mkswap: /dev/sdf: warning: wiping old btrfs signature.

This is harmless (since the swap image does get written!) but the extra
golden output is flagged as a regression.  Update the mkswap usage in
this test to dump the stderr output to $seqres.full, and complain if the
exit code is nonzero.

This fixes a regression that the author noticed when testing btrfs and
generic/507 and generic/570 run sequentially.  generic/507 calls
_require_scratch_shutdown to see if the shutdown call is supported.
btrfs does not support that, so the test is _notrun.  This leaves the
scratch filesystem mounted, causing the _try_wipe_scratch_devs between
tests to fail.  When g/570 starts up, the scratch device still contains
leftovers from the failed attempt to run g/507, which is why the mkswap
command outputs the above warning.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/generic/570 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/tests/generic/570 b/tests/generic/570
index 7d03acfe..02c1d333 100755
--- a/tests/generic/570
+++ b/tests/generic/570
@@ -27,7 +27,7 @@ _require_scratch_nocheck
 _require_block_device $SCRATCH_DEV
 test -e /dev/snapshot && _notrun "userspace hibernation to swap is enabled"
 
-$MKSWAP_PROG "$SCRATCH_DEV" >> $seqres.full
+$MKSWAP_PROG -f "$SCRATCH_DEV" &>> $seqres.full || echo "mkswap failed?"
 
 # Can you modify the swap dev via previously open file descriptors?
 for verb in 1 2 3 4; do

Re: [PATCH 3/4] generic/570: fix regression when SCRATCH_DEV is still formatted

[Eryu Guan]

On Tue, Jul 27, 2021 at 05:09:54PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Newer versions of mkswap (or at least the one in util-linux 2.34)
> complain to stderr when they're formatting over a device that seems to
> contain existing data:
> 
>     mkswap: /dev/sdf: warning: wiping old btrfs signature.
> 
> This is harmless (since the swap image does get written!) but the extra
> golden output is flagged as a regression.  Update the mkswap usage in
> this test to dump the stderr output to $seqres.full, and complain if the
> exit code is nonzero.
> 
> This fixes a regression that the author noticed when testing btrfs and
> generic/507 and generic/570 run sequentially.  generic/507 calls
> _require_scratch_shutdown to see if the shutdown call is supported.
> btrfs does not support that, so the test is _notrun.  This leaves the
> scratch filesystem mounted, causing the _try_wipe_scratch_devs between
> tests to fail.  When g/570 starts up, the scratch device still contains

Won't your previous patch "check: don't leave the scratch filesystem
mounted after _notrun" fix this issue as well? As _notrun won't leaves
scratch dev mounted & unwiped after that patch. Would you please confirm?

Thanks,
Eryu

> leftovers from the failed attempt to run g/507, which is why the mkswap
> command outputs the above warning.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/generic/570 |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/tests/generic/570 b/tests/generic/570
> index 7d03acfe..02c1d333 100755
> --- a/tests/generic/570
> +++ b/tests/generic/570
> @@ -27,7 +27,7 @@ _require_scratch_nocheck
>  _require_block_device $SCRATCH_DEV
>  test -e /dev/snapshot && _notrun "userspace hibernation to swap is enabled"
>  
> -$MKSWAP_PROG "$SCRATCH_DEV" >> $seqres.full
> +$MKSWAP_PROG -f "$SCRATCH_DEV" &>> $seqres.full || echo "mkswap failed?"
>  
>  # Can you modify the swap dev via previously open file descriptors?
>  for verb in 1 2 3 4; do

Re: [PATCH 3/4] generic/570: fix regression when SCRATCH_DEV is still formatted

[Darrick J. Wong]

On Sun, Aug 01, 2021 at 09:08:32PM +0800, Eryu Guan wrote:
> On Tue, Jul 27, 2021 at 05:09:54PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Newer versions of mkswap (or at least the one in util-linux 2.34)
> > complain to stderr when they're formatting over a device that seems to
> > contain existing data:
> > 
> >     mkswap: /dev/sdf: warning: wiping old btrfs signature.
> > 
> > This is harmless (since the swap image does get written!) but the extra
> > golden output is flagged as a regression.  Update the mkswap usage in
> > this test to dump the stderr output to $seqres.full, and complain if the
> > exit code is nonzero.
> > 
> > This fixes a regression that the author noticed when testing btrfs and
> > generic/507 and generic/570 run sequentially.  generic/507 calls
> > _require_scratch_shutdown to see if the shutdown call is supported.
> > btrfs does not support that, so the test is _notrun.  This leaves the
> > scratch filesystem mounted, causing the _try_wipe_scratch_devs between
> > tests to fail.  When g/570 starts up, the scratch device still contains
> 
> Won't your previous patch "check: don't leave the scratch filesystem
> mounted after _notrun" fix this issue as well? As _notrun won't leaves
> scratch dev mounted & unwiped after that patch. Would you please confirm?

Yes.  In the end this patch becomes more about failing the test if
mkswap fails than anything else.

--D

> Thanks,
> Eryu
> 
> > leftovers from the failed attempt to run g/507, which is why the mkswap
> > command outputs the above warning.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/generic/570 |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > 
> > diff --git a/tests/generic/570 b/tests/generic/570
> > index 7d03acfe..02c1d333 100755
> > --- a/tests/generic/570
> > +++ b/tests/generic/570
> > @@ -27,7 +27,7 @@ _require_scratch_nocheck
> >  _require_block_device $SCRATCH_DEV
> >  test -e /dev/snapshot && _notrun "userspace hibernation to swap is enabled"
> >  
> > -$MKSWAP_PROG "$SCRATCH_DEV" >> $seqres.full
> > +$MKSWAP_PROG -f "$SCRATCH_DEV" &>> $seqres.full || echo "mkswap failed?"
> >  
> >  # Can you modify the swap dev via previously open file descriptors?
> >  for verb in 1 2 3 4; do

[PATCH 4/4] check: back off the OOM score adjustment to -500

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Dave Chinner complained that fstests really shouldn't be running at
-1000 oom score adjustment because that makes it more "important" than
certain system daemons (e.g. journald, udev).  That's true, so increase
it to -500.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 check |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/check b/check
index 5d71b74c..e493ca11 100755
--- a/check
+++ b/check
@@ -564,7 +564,7 @@ OOM_SCORE_ADJ="/proc/self/oom_score_adj"
 function _adjust_oom_score() {
 	test -w "${OOM_SCORE_ADJ}" && echo "$1" > "${OOM_SCORE_ADJ}"
 }
-_adjust_oom_score -1000
+_adjust_oom_score -500
 
 # ...and make the tests themselves somewhat more attractive to it, so that if
 # the system runs out of memory it'll be the test that gets killed and not the

[PATCH 5/4] xfs/530: skip test if user MKFS_OPTIONS screw up formatting

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Skip this test if the user's MKFS_OPTIONS are not compatible with the
realtime parameters that we're injecting in order to test growfs bugs.
Because this test is trying to trigger a specific kernel bug, we need
mkfs to format a filesystem with very specific geometry parameters.

The first problem stems from the fact that the test performs a default
mkfs, computes a suitable realtime geometry from that filesystem, and
then formats a second time with an explicit blocksize option to mkfs.
If the original MKFS_OPTS contained a blocksize directive, the mkfs will
fail because the option was respecified.  The two blocksize options will
be the same, so we drop the explicit blocksize option.

However, this exposes a second problem: MKFS_OPTIONS might contain
options that are not compatible with any realtime filesystem.  If that
happens, _scratch_do_mkfs will "helpfully" drop MKFS_OPTIONS and try
again with only the options specified by the test.  This gets us a
filesystem with the given rt geometry, but it could be missing critical
parameters from MKFS_OPTIONS (like blocksize).  The test will then fail
to exercise the growfs bugfix, so the second part of the fix is to check
that the filesystem we're going to test actually has the geometry
parameters that we require.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
---
 tests/xfs/530 |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/xfs/530 b/tests/xfs/530
index 99a4d33b..0e12422d 100755
--- a/tests/xfs/530
+++ b/tests/xfs/530
@@ -60,10 +60,22 @@ echo "Format and mount rt volume"
 
 export USE_EXTERNAL=yes
 export SCRATCH_RTDEV=$rtdev
-_scratch_mkfs -d size=$((1024 * 1024 * 1024)) -b size=${dbsize} \
+_scratch_mkfs -d size=$((1024 * 1024 * 1024)) \
 	      -r size=${rtextsz},extsize=${rtextsz} >> $seqres.full
 _try_scratch_mount || _notrun "Couldn't mount fs with synthetic rt volume"
 
+# If we didn't get the desired realtime volume and the same blocksize as the
+# first format (which we used to compute a specific rt geometry), skip the
+# test.  This can happen if the MKFS_OPTIONS conflict with the ones we passed
+# to _scratch_mkfs or do not result in a valid rt fs geometry.  In this case,
+# _scratch_mkfs will try to "succeed" at formatting by dropping MKFS_OPTIONS,
+# giving us the wrong geometry.
+formatted_blksz="$(_get_block_size $SCRATCH_MNT)"
+test "$formatted_blksz" -ne "$dbsize" && \
+	_notrun "Tried to format with $dbsize blocksize, got $formatted_blksz."
+$XFS_INFO_PROG $SCRATCH_MNT | egrep -q 'realtime.*blocks=0' && \
+	_notrun "Filesystem should have a realtime volume"
+
 echo "Consume free space"
 fillerdir=$SCRATCH_MNT/fillerdir
 nr_free_blks=$(stat -f -c '%f' $SCRATCH_MNT)

[PATCH 6/4] xfs/007: fix regressions on V4 filesystems

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Following commit eae40404, I noticed the following regression when
running a V4 fstests run on an 5.13 kernel:

--- /tmp/fstests/tests/xfs/007.out      2021-05-13 11:47:55.793859995 -0700
+++ /var/tmp/fstests/xfs/007.out.bad    2021-07-28 09:23:42.856000000 -0700
@@ -16,4 +16,4 @@
 *** umount
 *** Usage after quotarm ***
 core.nblocks = 0
-core.nblocks = 0
+core.nblocks = 1

The underlying cause of this problem is the fact that we now remount the
filesystem with no quota options because that will (soon) become the
only means to turn off quota accounting on XFS.  Because V4 filesystems
don't support simultaneous project and group quotas and play weird
remapping games with the incore superblock field, we actually have to
issue a remove command for the group quota file if we're trying to
truncate the project quota file on a V4 filesystem.

Due to stupid limitations in xfs_quota we actually have to issue a
separate 'remove' command.

Fixes: eae40404 ("xfs/007: unmount after disabling quota")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/007 |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tests/xfs/007 b/tests/xfs/007
index 796db960..6d6d828b 100755
--- a/tests/xfs/007
+++ b/tests/xfs/007
@@ -43,10 +43,27 @@ do_test()
 	_qmount
 	echo "*** turn off $off_opts quotas"
 	$XFS_QUOTA_PROG -x -c "off -$off_opts" $SCRATCH_MNT
+
+	# Remount the filesystem with no quota options to force quotas off.
+	# This takes care of newer kernels where quotaoff clears the superblock
+	# quota enforcement flags but doesn't shut down accounting.
 	_scratch_unmount
 	_qmount_option ""
 	_scratch_mount
-	$XFS_QUOTA_PROG -x -c "remove -$off_opts" $SCRATCH_MNT
+
+	rm_commands=(-x -c "remove -$off_opts")
+
+	# Remounting a V4 fs with no quota options means that the internal
+	# gquotino -> pquotino remapping does not happen.  If we want to
+	# truncate the "project" quota file we must run remove -g.  However,
+	# xfs_quota has a nasty sharp edge wherein passing '-g' and '-p' only
+	# results in a QUOTARM call for the group quota file, so we must make
+	# a separate remove call.
+	[ $_fs_has_crcs == 0 ] && [ "$off_opts" = "up" ] && \
+		rm_commands+=(-c "remove -g")
+
+	$XFS_QUOTA_PROG "${rm_commands[@]}" $SCRATCH_MNT
+
 	echo "*** umount"
 	_scratch_unmount
 

Re: [PATCHSET 0/4] fstests: random fixes

[Eryu Guan]

On Tue, Jul 27, 2021 at 05:09:37PM -0700, Darrick J. Wong wrote:
> Hi all,
> 
> Here are the usual weekly fixes for fstests.

Thanks for all the fixes! I've queued all patches for update except the
fix for generic/570.

Thanks,
Eryu

> 
> If you're going to start using this mess, you probably ought to just
> pull from my git trees, which are linked below.
> 
> This is an extraordinary way to destroy everything.  Enjoy!
> Comments and questions are, as always, welcome.
> 
> --D
> 
> kernel git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes
> 
> xfsprogs git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes
> 
> fstests git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes
> ---
>  check             |    7 ++++++-
>  tests/generic/570 |    2 +-
>  tests/xfs/106     |    1 +
>  tests/xfs/106.out |    5 +++++
>  4 files changed, 13 insertions(+), 2 deletions(-)