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

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

On 8/4/25 09:28, Qu Wenruo wrote:
>
>
> 在 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?

Sorry for delayed response. Yes, if you feel keeping it simple by 
keeping the values aligned to 64k, I can make the change.

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

Okay. In that case, I will have the above pre-condition too.

>
>>
>> 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.*"

Noted. Thanks.

--NR

>
> Thanks,
> Qu
>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore