Re: [PATCH v2] fstests: generic/353 should accomodate other pwrite behaviors

[Bill O'Donnell <bodonnel@redhat.com>]

ping?

On Fri, Sep 29, 2023 at 08:42:26AM -0500, Bill O'Donnell wrote:
> On Fri, Sep 29, 2023 at 01:06:51PM +0800, Zorro Lang wrote:
> > On Fri, Sep 01, 2023 at 11:18:16AM -0500, 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 use actual blocksize for pwrite and reflink. Also
> > > modify it to accommodate pwrite and reflink that produce different
> > > mapping results.
> > > 
> > > Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
> > > ---
> > 
> > This patch makes sense to me, but this case looks like a regression test
> > test for a known btrfs issue. But this test case doesn't point out which
> > bug/fix does it test for. So I don't know if the 64k blocksize is a
> > necessary condition to reproduce the bug.
> > 
> > If we can prove it still can reproduce that bug with this patch at least,
> > then it's good to me to merge it.
> 
> I'd like Qu to weigh in on this from the btrfs standpoint.
> 
> Thanks-
> Bill
> 
> > 
> > Thanks,
> > Zorro
> > 
> > >  tests/generic/353     | 29 ++++++++++++++++-------------
> > >  tests/generic/353.out | 15 +--------------
> > >  2 files changed, 17 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/tests/generic/353 b/tests/generic/353
> > > index 9a1471bd..c5639725 100755
> > > --- a/tests/generic/353
> > > +++ b/tests/generic/353
> > > @@ -29,31 +29,34 @@ _require_xfs_io_command "fiemap"
> > >  _scratch_mkfs > /dev/null 2>&1
> > >  _scratch_mount
> > >  
> > > -blocksize=64k
> > > +blocksize=$(_get_file_block_size $SCRATCH_MNT)
> > > +
> > >  file1="$SCRATCH_MNT/file1"
> > >  file2="$SCRATCH_MNT/file2"
> > > +extmap1="$SCRATCH_MNT/extmap1"
> > > +extmap2="$SCRATCH_MNT/extmap2"
> > >  
> > >  # write the initial file
> > > -_pwrite_byte 0xcdcdcdcd 0 $blocksize $file1 | _filter_xfs_io
> > > +_pwrite_byte 0xcdcdcdcd 0 $blocksize $file1 > /dev/null
> > >  
> > >  # reflink initial file
> > > -_reflink_range $file1 0 $file2 0 $blocksize | _filter_xfs_io
> > > +_reflink_range $file1 0 $file2 0 $blocksize > /dev/null
> > >  
> > >  # check their fiemap to make sure it's correct
> > > -echo "before sync:"
> > > -echo "$file1" | _filter_scratch
> > > -$XFS_IO_PROG -c "fiemap -v" $file1 | _filter_fiemap_flags
> > > -echo "$file2" | _filter_scratch
> > > -$XFS_IO_PROG -c "fiemap -v" $file2 | _filter_fiemap_flags
> > > +$XFS_IO_PROG -c "fiemap -v" $file1 | _filter_fiemap_flags > $extmap1
> > > +$XFS_IO_PROG -c "fiemap -v" $file2 | _filter_fiemap_flags > $extmap2
> > > +
> > > +cmp -s $extmap1 $extmap2 || echo "mismatched extent maps before sync"
> > >  
> > >  # sync and recheck, to make sure the fiemap doesn't change just
> > >  # due to sync
> > >  sync
> > > -echo "after sync:"
> > > -echo "$file1" | _filter_scratch
> > > -$XFS_IO_PROG -c "fiemap -v" $file1 | _filter_fiemap_flags
> > > -echo "$file2" | _filter_scratch
> > > -$XFS_IO_PROG -c "fiemap -v" $file2 | _filter_fiemap_flags
> > > +$XFS_IO_PROG -c "fiemap -v" $file1 | _filter_fiemap_flags > $extmap1
> > > +$XFS_IO_PROG -c "fiemap -v" $file2 | _filter_fiemap_flags > $extmap2
> > > +
> > > +cmp -s $extmap1 $extmap2 || echo "mismatched extent maps after sync"
> > > +
> > > +echo "Silence is golden"
> > >  
> > >  # success, all done
> > >  status=0
> > > diff --git a/tests/generic/353.out b/tests/generic/353.out
> > > index 4f6e0b92..16ba4f1f 100644
> > > --- a/tests/generic/353.out
> > > +++ b/tests/generic/353.out
> > > @@ -1,15 +1,2 @@
> > >  QA output created by 353
> > > -wrote 65536/65536 bytes at offset 0
> > > -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > > -linked 65536/65536 bytes at offset 0
> > > -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > > -before sync:
> > > -SCRATCH_MNT/file1
> > > -0: [0..127]: shared|last
> > > -SCRATCH_MNT/file2
> > > -0: [0..127]: shared|last
> > > -after sync:
> > > -SCRATCH_MNT/file1
> > > -0: [0..127]: shared|last
> > > -SCRATCH_MNT/file2
> > > -0: [0..127]: shared|last
> > > +Silence is golden
> > > -- 
> > > 2.41.0
> > > 
> > 
>