On Thu, Jul 07, 2022 at 02:55:07PM -0700, Alli wrote:
> On Thu, 2022-07-07 at 11:06 -0700, Darrick J. Wong wrote:
> > I guess I should've cc'd Allison and Catherine on this one.
> >
> > Could either of you review these test changes, please?
> >
> > --D
> >
> > On Tue, Jul 05, 2022 at 03:02:14PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> > >
> > > Fix this test to work properly when the filesystem block size is
> > > less
> > > than 4k. Tripping the error injection points on shape changes in
> > > the
> > > xattr structure must be done dynamically.
> > >
> > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > > ---
> > > tests/xfs/018 | 52
> > > +++++++++++++++++++++++++++++++++++++++++++++++-----
> > > tests/xfs/018.out | 16 ++++------------
> > > 2 files changed, 51 insertions(+), 17 deletions(-)
> > >
> > >
> > > diff --git a/tests/xfs/018 b/tests/xfs/018
> > > index 041a3b24..14a6f716 100755
> > > --- a/tests/xfs/018
> > > +++ b/tests/xfs/018
> > > @@ -54,6 +54,45 @@ test_attr_replay()
> > > echo ""
> > > }
> > >
> > > +test_attr_replay_loop()
> > > +{
> > > + testfile=$testdir/$1
> > > + attr_name=$2
> > > + attr_value=$3
> > > + flag=$4
> > > + error_tag=$5
> > > +
> > > + # Inject error
> > > + _scratch_inject_error $error_tag
> > > +
> > > + # Set attribute; hopefully 1000 of them is enough to cause
> > > whatever
> > > + # attr structure shape change that the caller wants to test.
> > > + for ((i = 0; i < 1024; i++)); do
> > > + echo "$attr_value" | \
> > > + ${ATTR_PROG} -$flag "$attr_name$i" $testfile >
> > > $tmp.out 2> $tmp.err
> > > + cat $tmp.out $tmp.err >> $seqres.full
> > > + cat $tmp.err | _filter_scratch | sed -e 's/attr_name[0-
> > > 9]*/attr_nameXXXX/g'
> > > + touch $testfile &>/dev/null || break
> > > + done
> > > +
> > > + # FS should be shut down, touch will fail
> > > + touch $testfile 2>&1 | _filter_scratch
> > > +
> > > + # Remount to replay log
> > > + _scratch_remount_dump_log >> $seqres.full
> > > +
> > > + # FS should be online, touch should succeed
> > > + touch $testfile
> > > +
> > > + # Verify attr recovery
> > > + $ATTR_PROG -l $testfile >> $seqres.full
> > > + echo "Checking contents of $attr_name$i" >> $seqres.full
> > > + echo -n "${attr_name}XXXX: "
> > > + $ATTR_PROG -q -g $attr_name$i $testfile 2> /dev/null | md5sum;
> > > +
> > > + echo ""
> > > +}
> > > +
>
>
> Ok, I think I see what you are trying to do, but I think we can do it
> with less duplicated code and looping functions. What about something
> like this:
>
> diff --git a/tests/xfs/018 b/tests/xfs/018
> index 041a3b24..dc1324b1 100755
> --- a/tests/xfs/018
> +++ b/tests/xfs/018
> @@ -95,6 +95,9 @@ attr16k="$attr8k$attr8k"
> attr32k="$attr16k$attr16k"
> attr64k="$attr32k$attr32k"
>
> +blk_sz=$(_scratch_xfs_get_sb_field blocksize)
> +multiplier=$(( $blk_sz / 256 ))
The scratch fs hasn't been formatted yet, but if you use _get_block_size
after it's mounted, then, yes, I'm with you so far.
> +
> echo "*** mkfs"
> _scratch_mkfs >/dev/null
>
> @@ -140,7 +143,7 @@ test_attr_replay extent_file1 "attr_name2" $attr1k
> "s" "larp"
> test_attr_replay extent_file1 "attr_name2" $attr1k "r" "larp"
>
> # extent, inject error on split
> -create_test_file extent_file2 3 $attr1k
> +create_test_file extent_file2 $(( $multiplier - 1 )) $attr256
Hm. The calculations seem slightly off here -- name is ~8 bytes long,
the value is 256 bytes, which means there's at least 264 bytes per attr.
I guess you do only create multiplier-1 attrs, though, so that probably
works for all the blocksizes I can think of...
> test_attr_replay extent_file2 "attr_name4" $attr1k "s" "da_leaf_split"
If we keep the 1k attr value here, do we still trip the leaf split even
if that 1k value ends up in a remote block?
> # extent, inject error on fork transition
>
>
>
> Same idea right? We bring the attr fork right up and to the edge of
> the block boundary and then pop it? And then of course we apply the
> same pattern to the rest of the tests. I think that sort of reads
> cleaner too.
Right, I think that would work in principle. Does the same sort of fix
apply to the "extent, inject error on fork transition" case too?
--D
> Allison
>
> > > create_test_file()
> > > {
> > > filename=$testdir/$1
> > > @@ -88,6 +127,7 @@ echo 1 > /sys/fs/xfs/debug/larp
> > > attr16="0123456789ABCDEF"
> > > attr64="$attr16$attr16$attr16$attr16"
> > > attr256="$attr64$attr64$attr64$attr64"
> > > +attr512="$attr256$attr256"
> > > attr1k="$attr256$attr256$attr256$attr256"
> > > attr4k="$attr1k$attr1k$attr1k$attr1k"
> > > attr8k="$attr4k$attr4k"
> > > @@ -140,12 +180,14 @@ test_attr_replay extent_file1 "attr_name2"
> > > $attr1k "s" "larp"
> > > test_attr_replay extent_file1 "attr_name2" $attr1k "r" "larp"
> > >
> > > # extent, inject error on split
> > > -create_test_file extent_file2 3 $attr1k
> > > -test_attr_replay extent_file2 "attr_name4" $attr1k "s"
> > > "da_leaf_split"
> > > +create_test_file extent_file2 0 $attr1k
> > > +test_attr_replay_loop extent_file2 "attr_name" $attr1k "s"
> > > "da_leaf_split"
> > >
> > > -# extent, inject error on fork transition
> > > -create_test_file extent_file3 3 $attr1k
> > > -test_attr_replay extent_file3 "attr_name4" $attr1k "s"
> > > "attr_leaf_to_node"
> > > +# extent, inject error on fork transition. The attr value must be
> > > less than
> > > +# a full filesystem block so that the attrs don't use remote xattr
> > > values,
> > > +# which means we miss the leaf to node transition.
> > > +create_test_file extent_file3 0 $attr1k
> > > +test_attr_replay_loop extent_file3 "attr_name" $attr512 "s"
> > > "attr_leaf_to_node"
> > >
> > > # extent, remote
> > > create_test_file extent_file4 1 $attr1k
> > > diff --git a/tests/xfs/018.out b/tests/xfs/018.out
> > > index 022b0ca3..c3021ee3 100644
> > > --- a/tests/xfs/018.out
> > > +++ b/tests/xfs/018.out
> > > @@ -87,22 +87,14 @@ Attribute "attr_name1" has a 1024 byte value
> > > for SCRATCH_MNT/testdir/extent_file
> > > attr_name2: d41d8cd98f00b204e9800998ecf8427e -
> > >
> > > attr_set: Input/output error
> > > -Could not set "attr_name4" for SCRATCH_MNT/testdir/extent_file2
> > > +Could not set "attr_nameXXXX" for SCRATCH_MNT/testdir/extent_file2
> > > touch: cannot touch 'SCRATCH_MNT/testdir/extent_file2':
> > > Input/output error
> > > -Attribute "attr_name4" has a 1025 byte value for
> > > SCRATCH_MNT/testdir/extent_file2
> > > -Attribute "attr_name2" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file2
> > > -Attribute "attr_name3" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file2
> > > -Attribute "attr_name1" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file2
> > > -attr_name4: 9fd415c49d67afc4b78fad4055a3a376 -
> > > +attr_nameXXXX: 9fd415c49d67afc4b78fad4055a3a376 -
> > >
> > > attr_set: Input/output error
> > > -Could not set "attr_name4" for SCRATCH_MNT/testdir/extent_file3
> > > +Could not set "attr_nameXXXX" for SCRATCH_MNT/testdir/extent_file3
> > > touch: cannot touch 'SCRATCH_MNT/testdir/extent_file3':
> > > Input/output error
> > > -Attribute "attr_name4" has a 1025 byte value for
> > > SCRATCH_MNT/testdir/extent_file3
> > > -Attribute "attr_name2" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file3
> > > -Attribute "attr_name3" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file3
> > > -Attribute "attr_name1" has a 1024 byte value for
> > > SCRATCH_MNT/testdir/extent_file3
> > > -attr_name4: 9fd415c49d67afc4b78fad4055a3a376 -
> > > +attr_nameXXXX: a597dc41e4574873516420a7e4e5a3e0 -
> > >
> > > attr_set: Input/output error
> > > Could not set "attr_name2" for SCRATCH_MNT/testdir/extent_file4
> > >
>
