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 > > > >