On 8/25/23 3:09 PM, Bill O'Donnell wrote: > xfs_io pwrite issues a series of block size writes, but there is no guarantee > that the resulting extent(s) will be singular or contiguous. This behavior is > acceptable, but the test is flawed in that it expects a single extent for a > pwrite. > > Modify test to accept any layout for the reflinked logical range. I think this is no longer what the change does, right? You've now changed it so that it only pwrites (and reflinks) a single block rather than multiple, to avoid dealing with fragmentation issues. > > Signed-off-by: Bill O'Donnell <bodonnel@xxxxxxxxxx> > --- > tests/generic/352 | 9 +++++---- > tests/generic/352.out | 6 ++---- > 2 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/tests/generic/352 b/tests/generic/352 > index 52ec4850..77570bce 100755 > --- a/tests/generic/352 > +++ b/tests/generic/352 > @@ -28,14 +28,15 @@ _require_xfs_io_command "fiemap" > _scratch_mkfs > /dev/null 2>&1 > _scratch_mount > > -blocksize=$((128 * 1024)) > +blocksize=$(_get_file_block_size $SCRATCH_MNT) > _require_congruent_file_oplen $SCRATCH_MNT $blocksize > file="$SCRATCH_MNT/tmp" > > # Golden output is for $LOAD_FACTOR == 1 case $LOAD_FACTOR == 1 and 4k block size case, now - this is a little tricky so worth documenting IMHO. > orig_nr=8192 > -orig_last_extent=$(($orig_nr * $blocksize / 512)) > -orig_end=$(($orig_last_extent + $blocksize / 512 - 1)) > +orig_blocksize=4096 > +orig_last_extent=$(($orig_nr * $orig_blocksize / 512)) > +orig_end=$(($orig_last_extent + $orig_blocksize / 512 - 1)) > > # Real output > nr=$(($orig_nr * $LOAD_FACTOR)) > @@ -43,7 +44,7 @@ last_extent=$(($nr * $blocksize / 512)) > end=$(($last_extent + $blocksize / 512 - 1)) > > # write the initial block for later reflink > -_pwrite_byte 0xcdcdcdcd 0 $blocksize $file | _filter_xfs_io > +_pwrite_byte 0xcdcdcdcd 0 $blocksize $file | _filter_xfs_io > /dev/null I think it's fine to exclude the non-matching lines of xfs_io output, but there's no reason to run it through the filter if you send it all to /dev/null anyway, so could just drop the filter here. Other than those nitpicks, I think this looks fine now. -Eric > > # use reflink to create the rest of the file, whose all extents are all > # pointing to the first extent > diff --git a/tests/generic/352.out b/tests/generic/352.out > index 4ff66c21..454ff52f 100644 > --- a/tests/generic/352.out > +++ b/tests/generic/352.out > @@ -1,5 +1,3 @@ > QA output created by 352 > -wrote 131072/131072 bytes at offset 0 > -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > -0: [0..2097151]: shared > -1: [2097152..2097407]: shared|last > +0: [0..65535]: shared > +1: [65536..65543]: shared|last