[PATCHSET 0/5] fstests: random fixes for v2023.06.18

[PATCHSET 0/5] fstests: random fixes for v2023.06.18

[Darrick J. Wong]

Hi all,

Here's the usual odd 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
---
 tests/xfs/041     |    3 ++
 tests/xfs/439     |    6 +++-
 tests/xfs/529     |    4 +++
 tests/xfs/569     |    2 +
 tests/xfs/934     |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/934.out |   19 +++++++++++++
 6 files changed, 110 insertions(+), 3 deletions(-)
 create mode 100755 tests/xfs/934
 create mode 100644 tests/xfs/934.out

[PATCH 1/5] xfs/529: fix bogus failure when realtime is configured

[Darrick J. Wong]

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

If I have a realtime volume configured, this test will sometimes trip
over this:

XFS: Assertion failed: nmaps == 1, file: fs/xfs/xfs_dquot.c, line: 360
Call Trace:
 xfs_dquot_disk_alloc+0x3dc/0x400 [xfs 97e1fa8953d397b1fb9732df4de7fa9070bda501]
 xfs_qm_dqread+0xc9/0x190 [xfs 97e1fa8953d397b1fb9732df4de7fa9070bda501]
 xfs_qm_dqget+0xa8/0x230 [xfs 97e1fa8953d397b1fb9732df4de7fa9070bda501]
 xfs_qm_vop_dqalloc+0x160/0x600 [xfs 97e1fa8953d397b1fb9732df4de7fa9070bda501]
 xfs_setattr_nonsize+0x318/0x520 [xfs 97e1fa8953d397b1fb9732df4de7fa9070bda501]
 notify_change+0x30e/0x490
 chown_common+0x13e/0x1f0
 do_fchownat+0x8d/0xe0
 __x64_sys_fchownat+0x1b/0x20
 do_syscall_64+0x2b/0x80
 entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7fa6985e2cae

The test injects the bmap_alloc_minlen_extent error, which refuses to
allocate file space unless it's exactly minlen long.  However, a
precondition of this injection point is that the free space on the data
device must be sufficiently fragmented that there are small free
extents.

However, if realtime and rtinherit are enabled, the punch-alternating
call will operate on a realtime file, which only serves to write 0x55
patterns into the realtime bitmap.  Hence the test preconditions are not
satisfied, so the test is not serving its purpose.

Fix it by disabling rtinherit=1 on the rootdir so that we actually
fragment the bnobt/cntbt as required.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/529 |    4 ++++
 1 file changed, 4 insertions(+)


diff --git a/tests/xfs/529 b/tests/xfs/529
index 83d24da0ac..cd176877f5 100755
--- a/tests/xfs/529
+++ b/tests/xfs/529
@@ -32,6 +32,10 @@ echo "Format and mount fs"
 _scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
 _scratch_mount -o uquota >> $seqres.full
 
+# bmap_alloc_minlen_extent only applies to the datadev space allocator, so
+# we force the filesystem not to use the realtime volume.
+_xfs_force_bdev data $SCRATCH_MNT
+
 bsize=$(_get_file_block_size $SCRATCH_MNT)
 
 echo "* Delalloc to written extent conversion"

[PATCH 2/5] xfs/569: skip post-test fsck run

[Darrick J. Wong]

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

This test examines the behavior of mkfs.xfs with specific filesystem
configuration files by formatting the scratch device directly with those
exact parameters.  IOWs, it doesn't include external log devices or
realtime devices.  If external devices are set up, the post-test fsck
run fails because the filesystem doesnt' use the (allegedly) configured
external devices.  Fix that by adding _require_scratch_nocheck.

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


diff --git a/tests/xfs/569 b/tests/xfs/569
index e8902708bc..b6d5798058 100755
--- a/tests/xfs/569
+++ b/tests/xfs/569
@@ -14,7 +14,7 @@ _begin_fstest mkfs
 
 # Modify as appropriate.
 _supported_fs xfs
-_require_scratch
+_require_scratch_nocheck
 
 ls /usr/share/xfsprogs/mkfs/*.conf &>/dev/null || \
 	_notrun "No mkfs.xfs config files installed"

[PATCH 3/5] xfs/439: amend test to work with new log geometry validation

[Darrick J. Wong]

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

An upcoming patch moves more log validation checks to the superblock
verifier, so update this test as needed.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/439 |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)


diff --git a/tests/xfs/439 b/tests/xfs/439
index b7929493d1..8c69ece655 100755
--- a/tests/xfs/439
+++ b/tests/xfs/439
@@ -21,7 +21,9 @@ _begin_fstest auto quick fuzzers log
 _supported_fs xfs
 _require_scratch_nocheck
 # We corrupt XFS on purpose, and check if assert failures would crash system.
-_require_no_xfs_bug_on_assert
+# This used to be _require_no_xfs_bug_on_assert, but now we've fixed the sb
+# verifier to reject this before xfs_log_mount gets to it:
+_fixed_by_kernel_commit XXXXXXXXXXXX "xfs: journal geometry is not properly bounds checked"
 
 rm -f "$seqres.full"
 
@@ -33,7 +35,7 @@ blksz=$(_scratch_xfs_get_sb_field blocksize)
 _scratch_xfs_set_sb_field logsunit $((blksz - 1)) >> $seqres.full 2>&1
 
 # Check if logsunit is set correctly
-lsunit=$(_scratch_xfs_get_sb_field logsunit)
+lsunit=$(_scratch_xfs_get_sb_field logsunit 2>/dev/null)
 [ $lsunit -ne $((blksz - 1)) ] && _notrun "failed to set sb_logsunit"
 
 # Mount and writing log may trigger a crash on buggy kernel

[PATCH 4/5] xfs/041: force create files on the data device

[Darrick J. Wong]

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

Since we're testing growfs of the data device, we should create the
files there, even if the mkfs configuration enables rtinherit on the
root dir.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/041 |    3 +++
 1 file changed, 3 insertions(+)


diff --git a/tests/xfs/041 b/tests/xfs/041
index 05de5578ff..21b3afe7ce 100755
--- a/tests/xfs/041
+++ b/tests/xfs/041
@@ -46,6 +46,9 @@ bsize=`_scratch_mkfs_xfs -dsize=${agsize}m,agcount=1 2>&1 | _filter_mkfs 2>&1 \
 onemeginblocks=`expr 1048576 / $bsize`
 _scratch_mount
 
+# We're growing the data device, so force new file creation there
+_xfs_force_bdev data $SCRATCH_MNT
+
 echo "done"
 
 # full allocation group -> partial; partial -> expand partial + new partial;

[PATCH 5/5] xfs: test growfs of the realtime device

[Darrick J. Wong]

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

Create a new test to make sure that growfs on the realtime device works
without corrupting anything.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 tests/xfs/934     |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/934.out |   19 +++++++++++++
 2 files changed, 98 insertions(+)
 create mode 100755 tests/xfs/934
 create mode 100644 tests/xfs/934.out


diff --git a/tests/xfs/934 b/tests/xfs/934
new file mode 100755
index 0000000000..f2db4050a7
--- /dev/null
+++ b/tests/xfs/934
@@ -0,0 +1,79 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.
+#
+# FS QA Test No. 934
+#
+# growfs QA tests - repeatedly fill/grow the rt volume of the filesystem check
+# the filesystem contents after each operation.  This is the rt equivalent of
+# xfs/041.
+#
+. ./common/preamble
+_begin_fstest growfs ioctl auto
+
+# Override the default cleanup function.
+_cleanup()
+{
+	cd /
+	_scratch_unmount
+	rm -f $tmp.*
+}
+
+# Import common functions.
+. ./common/filter
+
+# real QA test starts here
+_supported_fs xfs
+
+_require_scratch
+_require_realtime
+_require_no_large_scratch_dev
+_scratch_unmount 2>/dev/null
+
+_fill()
+{
+    if [ $# -ne 1 ]; then echo "Usage: _fill \"path\"" 1>&2 ; exit 1; fi
+    _do "Fill filesystem" \
+	"$here/src/fill2fs --verbose --dir=$1 --seed=0 --filesize=65536 --stddev=32768 --list=- >>$tmp.manifest"
+}
+
+_do_die_on_error=message_only
+rtsize=32
+echo -n "Make $rtsize megabyte rt filesystem on SCRATCH_DEV and mount... "
+_scratch_mkfs_xfs -rsize=${rtsize}m | _filter_mkfs 2> "$tmp.mkfs" >> $seqres.full
+test "${PIPESTATUS[0]}" -eq 0 || _fail "mkfs failed"
+
+. $tmp.mkfs
+onemeginblocks=`expr 1048576 / $dbsize`
+_scratch_mount
+
+# We're growing the realtime device, so force new file creation there
+_xfs_force_bdev realtime $SCRATCH_MNT
+
+echo "done"
+
+# full allocation group -> partial; partial -> expand partial + new partial;
+# partial -> expand partial; partial -> full
+# cycle through 33m -> 67m -> 75m -> 96m
+for size in 33 67 75 96
+do
+    grow_size=`expr $size \* $onemeginblocks`
+    _fill $SCRATCH_MNT/fill_$size
+    _do "Grow filesystem to ${size}m" "xfs_growfs -R $grow_size $SCRATCH_MNT"
+    echo -n "Flush filesystem... "
+    _do "_scratch_unmount"
+    _do "_try_scratch_mount"
+    echo "done"
+    echo -n "Check files... "
+    if ! _do "$here/src/fill2fs_check $tmp.manifest"; then
+      echo "fail (see $seqres.full)"
+      _do "cat $tmp.manifest"
+      _do "ls -altrR $SCRATCH_MNT"
+      status=1 ; exit
+    fi
+    echo "done"
+done
+
+# success, all done
+echo "Growfs tests passed."
+status=0 ; exit
diff --git a/tests/xfs/934.out b/tests/xfs/934.out
new file mode 100644
index 0000000000..07da1eb7cb
--- /dev/null
+++ b/tests/xfs/934.out
@@ -0,0 +1,19 @@
+QA output created by 934
+Make 32 megabyte rt filesystem on SCRATCH_DEV and mount... done
+Fill filesystem... done
+Grow filesystem to 33m... done
+Flush filesystem... done
+Check files... done
+Fill filesystem... done
+Grow filesystem to 67m... done
+Flush filesystem... done
+Check files... done
+Fill filesystem... done
+Grow filesystem to 75m... done
+Flush filesystem... done
+Check files... done
+Fill filesystem... done
+Grow filesystem to 96m... done
+Flush filesystem... done
+Check files... done
+Growfs tests passed.

Re: [PATCH 1/5] xfs/529: fix bogus failure when realtime is configured

[Andrey Albershteyn]

On 2023-07-03 10:03:38, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If I have a realtime volume configured, this test will sometimes trip
> over this:
> 
> XFS: Assertion failed: nmaps == 1, file: fs/xfs/xfs_dquot.c, line: 360
> Call Trace:
>  xfs_dquot_disk_alloc+0x3dc/0x400 [xfs 97e1fa8953d397b1fb9732df4de7fa9070bda501]
>  xfs_qm_dqread+0xc9/0x190 [xfs 97e1fa8953d397b1fb9732df4de7fa9070bda501]
>  xfs_qm_dqget+0xa8/0x230 [xfs 97e1fa8953d397b1fb9732df4de7fa9070bda501]
>  xfs_qm_vop_dqalloc+0x160/0x600 [xfs 97e1fa8953d397b1fb9732df4de7fa9070bda501]
>  xfs_setattr_nonsize+0x318/0x520 [xfs 97e1fa8953d397b1fb9732df4de7fa9070bda501]
>  notify_change+0x30e/0x490
>  chown_common+0x13e/0x1f0
>  do_fchownat+0x8d/0xe0
>  __x64_sys_fchownat+0x1b/0x20
>  do_syscall_64+0x2b/0x80
>  entry_SYSCALL_64_after_hwframe+0x46/0xb0
> RIP: 0033:0x7fa6985e2cae
> 
> The test injects the bmap_alloc_minlen_extent error, which refuses to
> allocate file space unless it's exactly minlen long.  However, a
> precondition of this injection point is that the free space on the data
> device must be sufficiently fragmented that there are small free
> extents.
> 
> However, if realtime and rtinherit are enabled, the punch-alternating
> call will operate on a realtime file, which only serves to write 0x55
> patterns into the realtime bitmap.  Hence the test preconditions are not
> satisfied, so the test is not serving its purpose.
> 
> Fix it by disabling rtinherit=1 on the rootdir so that we actually
> fragment the bnobt/cntbt as required.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/529 |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> 
> diff --git a/tests/xfs/529 b/tests/xfs/529
> index 83d24da0ac..cd176877f5 100755
> --- a/tests/xfs/529
> +++ b/tests/xfs/529
> @@ -32,6 +32,10 @@ echo "Format and mount fs"
>  _scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full
>  _scratch_mount -o uquota >> $seqres.full
>  
> +# bmap_alloc_minlen_extent only applies to the datadev space allocator, so
> +# we force the filesystem not to use the realtime volume.
> +_xfs_force_bdev data $SCRATCH_MNT
> +

Looks good to me:
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

-- 
- Andrey

Re: [PATCH 5/5] xfs: test growfs of the realtime device

[Andrey Albershteyn]

On 2023-07-03 10:04:01, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create a new test to make sure that growfs on the realtime device works
> without corrupting anything.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/934     |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/934.out |   19 +++++++++++++
>  2 files changed, 98 insertions(+)
>  create mode 100755 tests/xfs/934
>  create mode 100644 tests/xfs/934.out
> 
> 
> diff --git a/tests/xfs/934 b/tests/xfs/934
> new file mode 100755
> index 0000000000..f2db4050a7
> --- /dev/null
> +++ b/tests/xfs/934
> @@ -0,0 +1,79 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.

Oracle?

> +#
> +# FS QA Test No. 934
> +#
> +# growfs QA tests - repeatedly fill/grow the rt volume of the filesystem check
> +# the filesystem contents after each operation.  This is the rt equivalent of
> +# xfs/041.
> +#
> +. ./common/preamble
> +_begin_fstest growfs ioctl auto
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> +	cd /
> +	_scratch_unmount
> +	rm -f $tmp.*
> +}
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs xfs
> +
> +_require_scratch
> +_require_realtime
> +_require_no_large_scratch_dev
> +_scratch_unmount 2>/dev/null
> +
> +_fill()
> +{
> +    if [ $# -ne 1 ]; then echo "Usage: _fill \"path\"" 1>&2 ; exit 1; fi

Is this needed for something?

> +    _do "Fill filesystem" \
> +	"$here/src/fill2fs --verbose --dir=$1 --seed=0 --filesize=65536 --stddev=32768 --list=- >>$tmp.manifest"
> +}
> +
> +_do_die_on_error=message_only
> +rtsize=32
> +echo -n "Make $rtsize megabyte rt filesystem on SCRATCH_DEV and mount... "
> +_scratch_mkfs_xfs -rsize=${rtsize}m | _filter_mkfs 2> "$tmp.mkfs" >> $seqres.full
> +test "${PIPESTATUS[0]}" -eq 0 || _fail "mkfs failed"
> +
> +. $tmp.mkfs
> +onemeginblocks=`expr 1048576 / $dbsize`
> +_scratch_mount
> +
> +# We're growing the realtime device, so force new file creation there
> +_xfs_force_bdev realtime $SCRATCH_MNT
> +
> +echo "done"
> +
> +# full allocation group -> partial; partial -> expand partial + new partial;
> +# partial -> expand partial; partial -> full
> +# cycle through 33m -> 67m -> 75m -> 96m
> +for size in 33 67 75 96
> +do
> +    grow_size=`expr $size \* $onemeginblocks`
> +    _fill $SCRATCH_MNT/fill_$size
> +    _do "Grow filesystem to ${size}m" "xfs_growfs -R $grow_size $SCRATCH_MNT"
> +    echo -n "Flush filesystem... "
> +    _do "_scratch_unmount"
> +    _do "_try_scratch_mount"
> +    echo "done"
> +    echo -n "Check files... "
> +    if ! _do "$here/src/fill2fs_check $tmp.manifest"; then
> +      echo "fail (see $seqres.full)"

I think "fail" is already printed by _do

These are nitpicks, anyway looks good to me:
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

-- 
- Andrey

Re: [PATCH 2/5] xfs/569: skip post-test fsck run

[Andrey Albershteyn]

On 2023-07-03 10:03:44, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> This test examines the behavior of mkfs.xfs with specific filesystem
> configuration files by formatting the scratch device directly with those
> exact parameters.  IOWs, it doesn't include external log devices or
> realtime devices.  If external devices are set up, the post-test fsck
> run fails because the filesystem doesnt' use the (allegedly) configured
> external devices.  Fix that by adding _require_scratch_nocheck.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/569 |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/tests/xfs/569 b/tests/xfs/569
> index e8902708bc..b6d5798058 100755
> --- a/tests/xfs/569
> +++ b/tests/xfs/569
> @@ -14,7 +14,7 @@ _begin_fstest mkfs
>  
>  # Modify as appropriate.
>  _supported_fs xfs
> -_require_scratch
> +_require_scratch_nocheck
>  
>  ls /usr/share/xfsprogs/mkfs/*.conf &>/dev/null || \
>  	_notrun "No mkfs.xfs config files installed"
> 

Looks good to me:
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

-- 
- Andrey

Re: [PATCH 4/5] xfs/041: force create files on the data device

[Andrey Albershteyn]

On 2023-07-03 10:03:55, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Since we're testing growfs of the data device, we should create the
> files there, even if the mkfs configuration enables rtinherit on the
> root dir.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/041 |    3 +++
>  1 file changed, 3 insertions(+)
> 
> 
> diff --git a/tests/xfs/041 b/tests/xfs/041
> index 05de5578ff..21b3afe7ce 100755
> --- a/tests/xfs/041
> +++ b/tests/xfs/041
> @@ -46,6 +46,9 @@ bsize=`_scratch_mkfs_xfs -dsize=${agsize}m,agcount=1 2>&1 | _filter_mkfs 2>&1 \
>  onemeginblocks=`expr 1048576 / $bsize`
>  _scratch_mount
>  
> +# We're growing the data device, so force new file creation there
> +_xfs_force_bdev data $SCRATCH_MNT
> +
>  echo "done"
>  
>  # full allocation group -> partial; partial -> expand partial + new partial;
> 

Looks good to me:
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>

-- 
- Andrey

Re: [PATCH 5/5] xfs: test growfs of the realtime device

[Andrey Albershteyn]

oh those are from 041, haven't noticed

-- 
- Andrey

Re: [PATCH 5/5] xfs: test growfs of the realtime device

[Darrick J. Wong]

On Tue, Jul 04, 2023 at 04:03:57PM +0200, Andrey Albershteyn wrote:
> On 2023-07-03 10:04:01, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Create a new test to make sure that growfs on the realtime device works
> > without corrupting anything.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/934     |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/934.out |   19 +++++++++++++
> >  2 files changed, 98 insertions(+)
> >  create mode 100755 tests/xfs/934
> >  create mode 100644 tests/xfs/934.out
> > 
> > 
> > diff --git a/tests/xfs/934 b/tests/xfs/934
> > new file mode 100755
> > index 0000000000..f2db4050a7
> > --- /dev/null
> > +++ b/tests/xfs/934
> > @@ -0,0 +1,79 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.
> 
> Oracle?

Eh, this is really just a tweak of xfs/041 so I retained the old
copyright.

> > +#
> > +# FS QA Test No. 934
> > +#
> > +# growfs QA tests - repeatedly fill/grow the rt volume of the filesystem check
> > +# the filesystem contents after each operation.  This is the rt equivalent of
> > +# xfs/041.
> > +#
> > +. ./common/preamble
> > +_begin_fstest growfs ioctl auto
> > +
> > +# Override the default cleanup function.
> > +_cleanup()
> > +{
> > +	cd /
> > +	_scratch_unmount
> > +	rm -f $tmp.*
> > +}
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs xfs
> > +
> > +_require_scratch
> > +_require_realtime
> > +_require_no_large_scratch_dev
> > +_scratch_unmount 2>/dev/null
> > +
> > +_fill()
> > +{
> > +    if [ $# -ne 1 ]; then echo "Usage: _fill \"path\"" 1>&2 ; exit 1; fi
> 
> Is this needed for something?

...and all the other weird bits are copied verbatim.

Thanks for reviewing!

--D

> > +    _do "Fill filesystem" \
> > +	"$here/src/fill2fs --verbose --dir=$1 --seed=0 --filesize=65536 --stddev=32768 --list=- >>$tmp.manifest"
> > +}
> > +
> > +_do_die_on_error=message_only
> > +rtsize=32
> > +echo -n "Make $rtsize megabyte rt filesystem on SCRATCH_DEV and mount... "
> > +_scratch_mkfs_xfs -rsize=${rtsize}m | _filter_mkfs 2> "$tmp.mkfs" >> $seqres.full
> > +test "${PIPESTATUS[0]}" -eq 0 || _fail "mkfs failed"
> > +
> > +. $tmp.mkfs
> > +onemeginblocks=`expr 1048576 / $dbsize`
> > +_scratch_mount
> > +
> > +# We're growing the realtime device, so force new file creation there
> > +_xfs_force_bdev realtime $SCRATCH_MNT
> > +
> > +echo "done"
> > +
> > +# full allocation group -> partial; partial -> expand partial + new partial;
> > +# partial -> expand partial; partial -> full
> > +# cycle through 33m -> 67m -> 75m -> 96m
> > +for size in 33 67 75 96
> > +do
> > +    grow_size=`expr $size \* $onemeginblocks`
> > +    _fill $SCRATCH_MNT/fill_$size
> > +    _do "Grow filesystem to ${size}m" "xfs_growfs -R $grow_size $SCRATCH_MNT"
> > +    echo -n "Flush filesystem... "
> > +    _do "_scratch_unmount"
> > +    _do "_try_scratch_mount"
> > +    echo "done"
> > +    echo -n "Check files... "
> > +    if ! _do "$here/src/fill2fs_check $tmp.manifest"; then
> > +      echo "fail (see $seqres.full)"
> 
> I think "fail" is already printed by _do
> 
> These are nitpicks, anyway looks good to me:
> Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>
> 
> -- 
> - Andrey
> 

Re: [PATCH 3/5] xfs/439: amend test to work with new log geometry validation

[Zorro Lang]

On Mon, Jul 03, 2023 at 10:03:50AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> An upcoming patch moves more log validation checks to the superblock
> verifier, so update this test as needed.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  tests/xfs/439 |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/tests/xfs/439 b/tests/xfs/439
> index b7929493d1..8c69ece655 100755
> --- a/tests/xfs/439
> +++ b/tests/xfs/439
> @@ -21,7 +21,9 @@ _begin_fstest auto quick fuzzers log
>  _supported_fs xfs
>  _require_scratch_nocheck
>  # We corrupt XFS on purpose, and check if assert failures would crash system.
> -_require_no_xfs_bug_on_assert
> +# This used to be _require_no_xfs_bug_on_assert, but now we've fixed the sb
> +# verifier to reject this before xfs_log_mount gets to it:
> +_fixed_by_kernel_commit XXXXXXXXXXXX "xfs: journal geometry is not properly bounds checked"

This case is a regression test case for:
  9c92ee2 ("xfs: validate sb_logsunit is a multiple of the fs blocksize")

So I think it's better to write this major commit at first, before recording the
secondary one.

>  
>  rm -f "$seqres.full"
>  
> @@ -33,7 +35,7 @@ blksz=$(_scratch_xfs_get_sb_field blocksize)
>  _scratch_xfs_set_sb_field logsunit $((blksz - 1)) >> $seqres.full 2>&1
>  
>  # Check if logsunit is set correctly
> -lsunit=$(_scratch_xfs_get_sb_field logsunit)
> +lsunit=$(_scratch_xfs_get_sb_field logsunit 2>/dev/null)

What kind of error should be ignored at here?

>  [ $lsunit -ne $((blksz - 1)) ] && _notrun "failed to set sb_logsunit"
>  
>  # Mount and writing log may trigger a crash on buggy kernel
> 

Re: [PATCH 3/5] xfs/439: amend test to work with new log geometry validation

[Darrick J. Wong]

On Wed, Jul 05, 2023 at 02:37:09PM +0800, Zorro Lang wrote:
> On Mon, Jul 03, 2023 at 10:03:50AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > An upcoming patch moves more log validation checks to the superblock
> > verifier, so update this test as needed.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  tests/xfs/439 |    6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/tests/xfs/439 b/tests/xfs/439
> > index b7929493d1..8c69ece655 100755
> > --- a/tests/xfs/439
> > +++ b/tests/xfs/439
> > @@ -21,7 +21,9 @@ _begin_fstest auto quick fuzzers log
> >  _supported_fs xfs
> >  _require_scratch_nocheck
> >  # We corrupt XFS on purpose, and check if assert failures would crash system.
> > -_require_no_xfs_bug_on_assert
> > +# This used to be _require_no_xfs_bug_on_assert, but now we've fixed the sb
> > +# verifier to reject this before xfs_log_mount gets to it:
> > +_fixed_by_kernel_commit XXXXXXXXXXXX "xfs: journal geometry is not properly bounds checked"
> 
> This case is a regression test case for:
>   9c92ee2 ("xfs: validate sb_logsunit is a multiple of the fs blocksize")
> 
> So I think it's better to write this major commit at first, before recording the
> secondary one.

Ok.  I guess that's a result of this test predating _fixed_by.

Also I'll update this with the real kernel commit id, now that there is
one.

> >  
> >  rm -f "$seqres.full"
> >  
> > @@ -33,7 +35,7 @@ blksz=$(_scratch_xfs_get_sb_field blocksize)
> >  _scratch_xfs_set_sb_field logsunit $((blksz - 1)) >> $seqres.full 2>&1
> >  
> >  # Check if logsunit is set correctly
> > -lsunit=$(_scratch_xfs_get_sb_field logsunit)
> > +lsunit=$(_scratch_xfs_get_sb_field logsunit 2>/dev/null)
> 
> What kind of error should be ignored at here?

The same new logsunit validation code, when ported to userspace, causes
xfs_db to emit verifier complaints that disturb the golden output.

--D

> 
> >  [ $lsunit -ne $((blksz - 1)) ] && _notrun "failed to set sb_logsunit"
> >  
> >  # Mount and writing log may trigger a crash on buggy kernel
> > 
> 

[PATCH v1.1 3/5] xfs/439: amend test to work with new log geometry validation

[Darrick J. Wong]

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

An upcoming patch moves more log validation checks to the superblock
verifier, so update this test as needed.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
v2: annotate which commits this tests is testing
---
 tests/xfs/439 |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tests/xfs/439 b/tests/xfs/439
index b7929493d1..cb6fb37918 100755
--- a/tests/xfs/439
+++ b/tests/xfs/439
@@ -20,8 +20,14 @@ _begin_fstest auto quick fuzzers log
 # real QA test starts here
 _supported_fs xfs
 _require_scratch_nocheck
-# We corrupt XFS on purpose, and check if assert failures would crash system.
-_require_no_xfs_bug_on_assert
+
+# We corrupt XFS on purpose, and check if assert failures would crash the
+# system when trying to xfs_log_mount.  Hence this is a regression test for:
+_fixed_by_git_commit kernel 9c92ee208b1f "xfs: validate sb_logsunit is a multiple of the fs blocksize"
+
+# This used to be _require_no_xfs_bug_on_assert, but now we've fixed the sb
+# verifier to reject this before xfs_log_mount gets to it:
+_fixed_by_git_commit kernel f1e1765aad7d "xfs: journal geometry is not properly bounds checked"
 
 rm -f "$seqres.full"
 
@@ -33,7 +39,7 @@ blksz=$(_scratch_xfs_get_sb_field blocksize)
 _scratch_xfs_set_sb_field logsunit $((blksz - 1)) >> $seqres.full 2>&1
 
 # Check if logsunit is set correctly
-lsunit=$(_scratch_xfs_get_sb_field logsunit)
+lsunit=$(_scratch_xfs_get_sb_field logsunit 2>/dev/null)
 [ $lsunit -ne $((blksz - 1)) ] && _notrun "failed to set sb_logsunit"
 
 # Mount and writing log may trigger a crash on buggy kernel

Re: [PATCH v1.1 3/5] xfs/439: amend test to work with new log geometry validation

[Zorro Lang]

On Wed, Jul 05, 2023 at 08:38:19AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> An upcoming patch moves more log validation checks to the superblock
> verifier, so update this test as needed.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> v2: annotate which commits this tests is testing
> ---
>  tests/xfs/439 |   12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/xfs/439 b/tests/xfs/439
> index b7929493d1..cb6fb37918 100755
> --- a/tests/xfs/439
> +++ b/tests/xfs/439
> @@ -20,8 +20,14 @@ _begin_fstest auto quick fuzzers log
>  # real QA test starts here
>  _supported_fs xfs
>  _require_scratch_nocheck
> -# We corrupt XFS on purpose, and check if assert failures would crash system.
> -_require_no_xfs_bug_on_assert
> +
> +# We corrupt XFS on purpose, and check if assert failures would crash the
> +# system when trying to xfs_log_mount.  Hence this is a regression test for:
> +_fixed_by_git_commit kernel 9c92ee208b1f "xfs: validate sb_logsunit is a multiple of the fs blocksize"
> +
> +# This used to be _require_no_xfs_bug_on_assert, but now we've fixed the sb
> +# verifier to reject this before xfs_log_mount gets to it:
> +_fixed_by_git_commit kernel f1e1765aad7d "xfs: journal geometry is not properly bounds checked"

Thanks, I'll replace "_fixed_by_git_commit kernel" with "_fixed_by_kernel_commit"
when I merge this patch. Others looks good to me.

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  
>  rm -f "$seqres.full"
>  
> @@ -33,7 +39,7 @@ blksz=$(_scratch_xfs_get_sb_field blocksize)
>  _scratch_xfs_set_sb_field logsunit $((blksz - 1)) >> $seqres.full 2>&1
>  
>  # Check if logsunit is set correctly
> -lsunit=$(_scratch_xfs_get_sb_field logsunit)
> +lsunit=$(_scratch_xfs_get_sb_field logsunit 2>/dev/null)
>  [ $lsunit -ne $((blksz - 1)) ] && _notrun "failed to set sb_logsunit"
>  
>  # Mount and writing log may trigger a crash on buggy kernel
>