Re: [PATCH 3/7] btrfs/137: Make this compatible with all block sizes

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2025/7/29 15:51, Nirjhar Roy (IBM) 写道:
> For large blocksizes like 64k on powerpc with 64k pagesize
> it failed simply because this test was written with 4k
> block size in mind.
> The first few lines of the error logs are as follows:
> 
>       d3dc847171f9081bd75d7a2d3b53d322  SCRATCH_MNT/snap2/bar
> 
>       File snap1/foo fiemap results in the original filesystem:
>      -0: [0..7]: data
>      +0: [0..127]: data
> 
>       File snap1/bar fiemap results in the original filesystem:
>      ...
> 
> Fix this by making the test choose offsets based on
> the blocksize.

I'm wondering, why not just use a fixed 64K block size?

So that all supported btrfs block sizes can result the same file contents.

> Also, now that the file hashes and
> the extent/block numbers will change depending on the
> blocksize, calculate the hashes and the block mappings,
> store them in temporary files and then calculate their diff
> between the new and the original filesystem.
> This allows us to remove all the block mapping and hashes
> from the .out file.

Although I agree we should remove the block mappings from the golden 
output, as compression can add extra flags and pollute the golden output.

But that can also be done with _require_btrfs_no_compress() helper.

> 
> Reported-by: Disha Goel <disgoel@linux.ibm.com>
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> ---
>   tests/btrfs/137     | 135 +++++++++++++++++++++++++++++---------------
>   tests/btrfs/137.out |  59 ++-----------------
>   2 files changed, 94 insertions(+), 100 deletions(-)
> 
> diff --git a/tests/btrfs/137 b/tests/btrfs/137
> index 7710dc18..61e983cb 100755
> --- a/tests/btrfs/137
> +++ b/tests/btrfs/137
> @@ -27,53 +27,74 @@ _require_xfs_io_command "fiemap"
>   send_files_dir=$TEST_DIR/btrfs-test-$seq
>   
>   rm -fr $send_files_dir
> -mkdir $send_files_dir
> +mkdir $send_files_dir $tmp

Just a small nitpick, it's more common to use $tmp.<suffix>, that's why 
the default _cleanup() template goes with "rm -f $tmp.*"

Thanks,
Qu