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

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

[Bill O'Donnell]

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.

Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
---
 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
 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
 
 # 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
-- 
2.41.0

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

[Eric Sandeen]

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@redhat.com>
> ---
>  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