Re: [PATCH v2 3/3] btrfs-progs: mfks-tests: make sure mkfs.btrfs cleans up temporary chunks

Qu Wenruo
2021-10-11T22:54:43Z

On 2021/10/11 22:38, David Sterba wrote:
> On Mon, Oct 11, 2021 at 08:06:50PM +0800, Qu Wenruo wrote:
>> Since current "btrfs filesystem df" command will warn if there are
>> multiple profiles of the same type, it's a good way to detect left-over
>> temporary chunks.
>>
>> This patch will enhance the existing mkfs-tests/001-basic-profiles test
>> case to also check for the warning messages, to make sure mkfs.btrfs has
>> properly cleaned up all temporary chunks.
>>
>> There is a special workaround newly implemented in test_get_info(), as
>> recent kernel introduced single device RAID0 support, which is no
>> different than SINGLE.
>>
>> But for single device RAID0, kernel may choose to preallocate new chunks
>> with SINGLE profile, causing false alerts.
>>
>> Work around this kernel bug by mounting the btrfs read-only to prevent
>> preallocating new chunks.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> tests/mkfs-tests/001-basic-profiles/test.sh | 16 ++++++++++++++--
>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/mkfs-tests/001-basic-profiles/test.sh b/tests/mkfs-tests/001-basic-profiles/test.sh
>> index b3ba50d71ddc..0be199749864 100755
>> --- a/tests/mkfs-tests/001-basic-profiles/test.sh
>> +++ b/tests/mkfs-tests/001-basic-profiles/test.sh
>> @@ -11,10 +11,22 @@ setup_root_helper
>>
>> test_get_info()
>> {
>> + tmp_out=$(mktemp --tmpdir btrfs-progs-mkfs-tests-get-info.XXXXXX)
>
> Local variables should be declared
>
> local tmp_out
>
>> run_check $SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super "$dev1"
>> run_check $SUDO_HELPER "$TOP/btrfs" check "$dev1"
>> - run_check $SUDO_HELPER mount "$dev1" "$TEST_MNT"
>> - run_check "$TOP/btrfs" filesystem df "$TEST_MNT"
>> +
>> + btrfs ins dump-tree -t chunk "$dev1" >> "$RESULTS"
>
> Using $RESULTS is not recommended, the same can be achieved by
>
> run_check btrfs...
>
> Also you should run the "$TOP/btrfs" binary and use the full name of the
> subcommands, ie. 'inspect-internal'.

Oh, that's the debug output I forgot to remove.

Please remove the "ins dump-tree" call...

Thanks,
Qu

>
>> + # Work around a kernel bug that kernel will treat SINGLE and single
>> + # device RAID0 as the same.
>> + # Thus kernel may create new SINGLE chunks, causing extra warning
>> + # when testing single device RAID0.
>> + run_check $SUDO_HELPER mount -o ro "$dev1" "$TEST_MNT"
>> + if grep -q "Multiple block group profiles detected" "$tmp_out"; then
>> + rm -- "$tmp_out"
>> + _fail "temporary chunks are not properly cleaned up"
>> + fi
>> + rm -- "$tmp_out"
>
> So to be able to run the testsuite on unfixed kernels the workaround
> makes sense.
>
>> run_check $SUDO_HELPER "$TOP/btrfs" filesystem usage "$TEST_MNT"
>> run_check $SUDO_HELPER "$TOP/btrfs" device usage "$TEST_MNT"
>> run_check $SUDO_HELPER umount "$TEST_MNT"
>> --
>> 2.33.0
>