Re: [PATCH 2/5] btrfs-progs: fsck-tests: Check if current kernel can mount fs with specified sector size

Qu Wenruo
2019-07-23T01:05:31Z

[-- Attachment #1.1: Type: text/plain, Size: 7060 bytes --]

On 2019/7/23 上午1:07, David Sterba wrote:
> On Fri, Jul 05, 2019 at 03:26:48PM +0800, Qu Wenruo wrote:
>> [BUG]
>> When doing test on platforms with page size other than 4K (e.g aarch64
>> can use 64K page size, or ppc64le), certain test wills fail like:
>> [TEST/fsck] 012-leaf-corruption
>> mount: /home/adam/btrfs-progs/tests/mnt: wrong fs type, bad option, bad superblock on /dev/loop4, missing codepage or helper program, or other error.
>> find: '/home/adam/btrfs-progs/tests//mnt/lost+found': No such file or directory
>> inode 1862 not recovered correctly
>> test failed for case 012-leaf-corruption
>>
>> [TEST/fsck] 028-unaligned-super-dev-sizes
>> failed: mount -t btrfs -o loop ./dev_and_super_mismatch_unaligned.raw.restored /home/adam/btrfs-progs/tests//mnt
>> test failed for case 028-unaligned-super-dev-sizes
>>
>> [TEST/fsck] 037-freespacetree-repair
>> failed: /home/adam/btrfs-progs/mkfs.btrfs -f -n 4k /home/adam/btrfs-progs/tests//test.img
>> test failed for case 037-freespacetree-repair
>>
>> [CAUSE]
>>
>> For fsck/012 and fsck/028, it's caused by the lack of subpage size sector
>> size support, thus we require kernel page size to match on-disk sector size:
>> BTRFS error (device loop4): sectorsize 4096 not supported yet, only support 65536
>> BTRFS error (device loop4): superblock contains fatal errors
>> BTRFS error (device loop4): open_ctree failed
>>
>> For fsck/037, it's mkfs causing the problem as we're using 4k nodesize,
>> but on 64K page sized system, we will use 64K sectorsize and cause
>> conflicts.
>>
>> [FIX]
>> Considering it's easier and easier to get aarch64 boards with enough
>> performance (e.g rpi4, rk3399, S922) to compile kernel and run tests,
>> let's skip such tests before widespread complain comes.
>>
>> This patch will introduce a new check, check_prereq_mount_with_sectorsize(),
>> which will test if kernel can mount btrfs with specified sectorsize.
>> So that even one day we support subpage sized sectorsize, we won't need
>> to update test case again.
>>
>> For fsck/037, also specify sector size manually. And since in that case
>> we still need to mount the fs, also add
>> check_prereq_mount_with_sectorsize() call.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> tests/common | 29 +++++++++++++++++++
>> tests/fsck-tests/012-leaf-corruption/test.sh | 1 +
>> .../028-unaligned-super-dev-sizes/test.sh | 1 +
>> .../037-freespacetree-repair/test.sh | 3 +-
>> 4 files changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/common b/tests/common
>> index 79a16f1e187d..5ad16b69b61d 100644
>> --- a/tests/common
>> +++ b/tests/common
>> @@ -306,6 +306,35 @@ check_prereq()
>> fi
>> }
>>
>> +# Require to mount images with speicified sectorsize
>> +# This is to make sure we can run this test on different arch
>> +# (e.g aarch64 with 64K pagesize)
>> +check_prereq_mount_with_sectorsize()
>> +{
>> + prepare_test_dev 128M
>> + check_prereq mkfs.btrfs
>> + setup_root_helper
>> +
>> + local sectorsize=$1
>> + local loop_opt
>> +
>> + if [[ -b "$TEST_DEV" ]]; then
>> + loop_opt=""
>> + elif [[ -f "$TEST_DEV" ]]; then
>> + loop_opt="-o loop"
>> + else
>> + _fail "Invalid \$TEST_DEV: $TEST_DEV"
>> + fi
>> +
>> + run_check_mkfs_test_dev -f -s $sectorsize
>> + $SUDO_HELPER mount -t btrfs $loop_opt "$TEST_DEV" "$TEST_MNT" \
>> + &> /dev/null
>> + if [ $? -ne 0 ]; then
>> + _not_run "kernel doesn't support sectorsize $sectorsize"
>> + fi
>> + run_check_umount_test_dev
>> +}
>
> I think the reason why each requires the specific 4k size should be
> documented there.

I'll move the comment to each test calling this check.

> This helper hides several things, like forcing 128M on
> the default test device, mkfs and mount. Those are probably inevitable
> to make sure the combination is supported but the helper should be
> independent and not possibly interfere with anything the test could use.

The only thing may affect test cases is the "prepare_test_dev 128M" call.
It can be easily avoided by calling this check before real
prepare_test_dev call.

I can update the test cases to co-operate this, and add extra comment
for that.

>
> The helper could be added as a separate patch, where the updated tests
> should document the reasons for using it.

No problem, and just in case I will also add comment in each test
calling this check to explain why.

>
>> +
>> check_global_prereq()
>> {
>> which "$1" &> /dev/null
>> diff --git a/tests/fsck-tests/012-leaf-corruption/test.sh b/tests/fsck-tests/012-leaf-corruption/test.sh
>> index 68d9f695d4de..d5da1d210f28 100755
>> --- a/tests/fsck-tests/012-leaf-corruption/test.sh
>> +++ b/tests/fsck-tests/012-leaf-corruption/test.sh
>> @@ -107,6 +107,7 @@ check_leaf_corrupt_no_data_ext()
>>
>> setup_root_helper
>>
>> +check_prereq_mount_with_sectorsize 4096
>> generate_leaf_corrupt_no_data_ext test.img
>
> So the tests with pre-generated images can't succeed so the test must be
> skipped, unless we have images for all valid page size values.

Or after we can mount 4K sector size fs on 64K page size system.

>
>> check_image test.img
>> check_leaf_corrupt_no_data_ext test.img
>> diff --git a/tests/fsck-tests/028-unaligned-super-dev-sizes/test.sh b/tests/fsck-tests/028-unaligned-super-dev-sizes/test.sh
>> index 4015df2d8570..49fa35241d04 100755
>> --- a/tests/fsck-tests/028-unaligned-super-dev-sizes/test.sh
>> +++ b/tests/fsck-tests/028-unaligned-super-dev-sizes/test.sh
>> @@ -6,6 +6,7 @@
>> source "$TEST_TOP/common"
>>
>> check_prereq btrfs
>> +check_prereq_mount_with_sectorsize 4096
>> setup_root_helper
>>
>> check_all_images
>> diff --git a/tests/fsck-tests/037-freespacetree-repair/test.sh b/tests/fsck-tests/037-freespacetree-repair/test.sh
>> index 7f547a33512d..32e6651ac705 100755
>> --- a/tests/fsck-tests/037-freespacetree-repair/test.sh
>> +++ b/tests/fsck-tests/037-freespacetree-repair/test.sh
>> @@ -10,6 +10,7 @@ prepare_test_dev 256M
>>
>> check_prereq btrfs
>> check_prereq mkfs.btrfs
>> +check_prereq_mount_with_sectorsize 4096
>> check_global_prereq grep
>> check_global_prereq tail
>> check_global_prereq head
>> @@ -55,7 +56,7 @@ if ! [ -f "/sys/fs/btrfs/features/free_space_tree" ]; then
>> exit
>> fi
>>
>> -run_check_mkfs_test_dev -n 4k
>> +run_check_mkfs_test_dev -s 4k -n 4k
>
> Can the test be updated so it always succeeds, ie. giving it a valid
> sectorsize/nodesize based on getconf? Besides the hardcoded size values
> (for the device and files), I don't see anything that would mandate 4k
> sectorsize.

Sure, I could try to test all valid node size combination based on
system page size.

Thanks,
Qu

>
>> run_check_mount_test_dev -oclear_cache,space_cache=v2
>>
>> # create files which will populate the FST
>> --
>> 2.22.0

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]