Re: [PATCH 4/4] xfs/548: Verify correctness of upgrading an fs to support large extent counters

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jun 07, 2022 at 03:17:01PM +0530, Chandan Babu R wrote:
> On Mon, Jun 06, 2022 at 08:35:19 AM -0700, Darrick J. Wong wrote:
> > On Mon, Jun 06, 2022 at 06:11:01PM +0530, Chandan Babu R wrote:
> >> This commit adds a test to verify upgrade of an existing V5 filesystem to
> >> support large extent counters.
> >> 
> >> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> >> ---
> >>  tests/xfs/548     | 109 ++++++++++++++++++++++++++++++++++++++++++++++
> >>  tests/xfs/548.out |  12 +++++
> >>  2 files changed, 121 insertions(+)
> >>  create mode 100755 tests/xfs/548
> >>  create mode 100644 tests/xfs/548.out
> >> 
> >> diff --git a/tests/xfs/548 b/tests/xfs/548
> >> new file mode 100755
> >> index 00000000..6c577584
> >> --- /dev/null
> >> +++ b/tests/xfs/548
> >> @@ -0,0 +1,109 @@
> >> +#! /bin/bash
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# Copyright (c) 2022 Oracle.  All Rights Reserved.
> >> +#
> >> +# FS QA Test 548
> >> +#
> >> +# Test to verify upgrade of an existing V5 filesystem to support large extent
> >> +# counters.
> >> +#
> >> +. ./common/preamble
> >> +_begin_fstest auto quick metadata
> >> +
> >> +# Import common functions.
> >> +. ./common/filter
> >> +. ./common/attr
> >> +. ./common/inject
> >> +. ./common/populate
> >> +
> >> +# real QA test starts here
> >> +_supported_fs xfs
> >> +_require_scratch
> >> +_require_scratch_xfs_nrext64
> >> +_require_attrs
> >> +_require_xfs_debug
> >> +_require_test_program "punch-alternating"
> >> +_require_xfs_io_error_injection "bmap_alloc_minlen_extent"
> >> +
> >> +_scratch_mkfs -d size=$((512 * 1024 * 1024)) >> $seqres.full
> >> +_scratch_mount >> $seqres.full
> >> +
> >> +bsize=$(_get_file_block_size $SCRATCH_MNT)
> >> +
> >> +testfile=$SCRATCH_MNT/testfile
> >> +
> >> +nr_blks=20
> >> +
> >> +echo "Add blocks to file's data fork"
> >> +$XFS_IO_PROG -f -c "pwrite 0 $((nr_blks * bsize))" $testfile \
> >> +	     >> $seqres.full
> >> +$here/src/punch-alternating $testfile
> >> +
> >> +echo "Consume free space"
> >> +fillerdir=$SCRATCH_MNT/fillerdir
> >> +nr_free_blks=$(stat -f -c '%f' $SCRATCH_MNT)
> >> +nr_free_blks=$((nr_free_blks * 90 / 100))
> >> +
> >> +_fill_fs $((bsize * nr_free_blks)) $fillerdir $bsize 0 \
> >> +	 >> $seqres.full 2>&1
> >> +
> >> +echo "Create fragmented filesystem"
> >> +for dentry in $(ls -1 $fillerdir/); do
> >> +	$here/src/punch-alternating $fillerdir/$dentry >> $seqres.full
> >> +done
> >> +
> >> +echo "Inject bmap_alloc_minlen_extent error tag"
> >> +_scratch_inject_error bmap_alloc_minlen_extent 1
> >> +
> >> +echo "Add blocks to file's attr fork"
> >> +nr_blks=10
> >> +attr_len=255
> >> +nr_attrs=$((nr_blks * bsize / attr_len))
> >> +for i in $(seq 1 $nr_attrs); do
> >> +	attr="$(printf "trusted.%0247d" $i)"
> >> +	$SETFATTR_PROG -n "$attr" $testfile >> $seqres.full 2>&1
> >> +	[[ $? != 0 ]] && break
> >> +done
> >> +
> >> +testino=$(stat -c '%i' $testfile)
> >> +
> >> +echo "Unmount filesystem"
> >> +_scratch_unmount >> $seqres.full
> >> +
> >> +orig_dcnt=$(_scratch_xfs_get_metadata_field core.nextents "inode $testino")
> >> +orig_acnt=$(_scratch_xfs_get_metadata_field core.naextents "inode $testino")
> >> +
> >> +echo "Upgrade filesystem to support large extent counters"
> >> +_scratch_xfs_admin -O nrext64=1 >> $seqres.full 2>&1
> >> +if [[ $? != 0 ]]; then
> >> +	_notrun "Filesystem geometry is not suitable for upgrading"
> >> +fi
> >> +
> >> +
> >> +echo "Mount filesystem"
> >> +_scratch_mount >> $seqres.full
> >> +
> >> +echo "Modify inode core"
> >> +touch $testfile
> >> +
> >> +echo "Unmount filesystem"
> >> +_scratch_unmount >> $seqres.full
> >> +
> >> +dcnt=$(_scratch_xfs_get_metadata_field core.nextents "inode $testino")
> >> +acnt=$(_scratch_xfs_get_metadata_field core.naextents "inode $testino")
> >> +
> >> +echo "Verify inode extent counter values after fs upgrade"
> >
> > Is there a scenario where the inode counters would become corrupt after
> > enabling the superblock feature bit?  IIRC repair doesn't rewrite the
> > inodes during the upgrade... so is this test merely being cautious?  Or
> > is this covering a failure you found somewhere while writing the feature?
> >
> 
> I was just being cautious w.r.t "Large extent counters" functionality working
> correctly. I used this test during my development to make sure that I was able
> to capture failures before I ran the entire xfstests suite.

Fair enough,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> 
> -- 
> chandan