Re: [PATCH v2 10/10] btrfs: test tempfsid with device add, seed, and balance

Anand Jain
2024-02-24T11:29:44Z

On 2/20/24 22:35, Filipe Manana wrote:
> On Mon, Feb 19, 2024 at 7:50 PM Anand Jain <anand.jain@oracle.com> wrote:
>>
>> Make sure that basic functions such as seeding and device add fail,
>> while balance runs successfully with tempfsid.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> v2:
>> Remove unnecessary function.
>> Add clone group
>> use $UMOUNT_PROG
>> Let _cp_reflink fail on the stdout.
>>
>> tests/btrfs/315 | 79 +++++++++++++++++++++++++++++++++++++++++++++
>> tests/btrfs/315.out | 9 ++++++
>> 2 files changed, 88 insertions(+)
>> create mode 100755 tests/btrfs/315
>> create mode 100644 tests/btrfs/315.out
>>
>> diff --git a/tests/btrfs/315 b/tests/btrfs/315
>> new file mode 100755
>> index 000000000000..4376c7f1849c
>> --- /dev/null
>> +++ b/tests/btrfs/315
>> @@ -0,0 +1,79 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2024 YOUR NAME HERE. All Rights Reserved.
>
> Your name here...
>
>> +#
>> +# FS QA Test 315
>> +#
>> +# Verify if the seed and device add to a tempfsid filesystem fails.
>
> And balance too...
>
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick volume seed tempfsid
>
> Missing 'balance' group.
>

Now added.

>> +
>> +_cleanup()
>> +{
>> + cd /
>> + $UMOUNT_PROG $tempfsid_mnt 2>/dev/null
>> + rm -r -f $tmp.*
>> + rm -r -f $tempfsid_mnt
>> +}
>> +
>> +. ./common/filter.btrfs
>> +
>> +_supported_fs btrfs
>> +_require_btrfs_sysfs_fsid
>> +_require_scratch_dev_pool 3
>> +_require_btrfs_fs_feature temp_fsid

>> +_require_btrfs_command inspect-internal dump-super
>> +_require_btrfs_mkfs_uuid_option
>
> So same as before, these last 2 _require_* are because of the
> mkfs_clone() function,
> defined at common/btrfs, so they should be in the function and not
> spread over every test case that calls it.

The last two _require have now been removed because mkfs_clone()
includes those checks.

>> +
>> +_scratch_dev_pool_get 3
>> +
>> +# mount point for the tempfsid device
>> +tempfsid_mnt=$TEST_DIR/$seq/tempfsid_mnt
>> +
>> +seed_device_must_fail()
>> +{
>> + echo ---- $FUNCNAME ----
>> +
>> + mkfs_clone ${SCRATCH_DEV} ${SCRATCH_DEV_NAME[1]}
>> +
>> + $BTRFS_TUNE_PROG -S 1 ${SCRATCH_DEV}
>> + $BTRFS_TUNE_PROG -S 1 ${SCRATCH_DEV_NAME[1]}
>> +
>> + _scratch_mount 2>&1 | _filter_scratch
>> + _mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt} 2>&1 | _filter_test_dir
>> +}
>> +
>> +device_add_must_fail()
>> +{
>> + echo ---- $FUNCNAME ----
>> +
>> + mkfs_clone ${SCRATCH_DEV} ${SCRATCH_DEV_NAME[1]}
>> + _scratch_mount
>> + _mount ${SCRATCH_DEV_NAME[1]} ${tempfsid_mnt}
>> +
>> + $XFS_IO_PROG -fc 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/foo | \
>> + _filter_xfs_io
>> +

>> +$BTRFS_UTIL_PROG device add -f ${SCRATCH_DEV_NAME[2]} ${tempfsid_mnt} >> \
>> + $seqres.full 2>&1 && _fail "Failed to file device add"
>
> Can't we do without the && _fail?
> Just call device add and put the expected error message in the golden output.
> It's the preferred pattern in fstests in general, and makes everything
> shorter and easier to read.

Upon rethinking, it can be done as outlined below, also updated
the golden output accordingly.

$BTRFS_UTIL_PROG device add -f ${SCRATCH_DEV_NAME[2]} ${tempfsid_mnt}
2>&1 | \
grep -v "Performing full device TRIM" |
_filter_scratch_pool

Thanks.