Re: [PATCH] btrfs/254: test cleaning up of the stale device

Anand Jain
2021-12-09T06:41:35Z

On 08/12/2021 22:50, Josef Bacik wrote:
> On Wed, Dec 08, 2021 at 10:07:46PM +0800, Anand Jain wrote:
>> Recreating a new filesystem or adding a device to a mounted the filesystem
>> should remove the device entries under its previous fsid even when
>> confused with different device paths to the same device.
>>
>> Fixed by the kernel patch (in the ml):
>> btrfs: harden identification of the stale device
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> tests/btrfs/254 | 110 ++++++++++++++++++++++++++++++++++++++++++++
>> tests/btrfs/254.out | 6 +++
>> 2 files changed, 116 insertions(+)
>> create mode 100755 tests/btrfs/254
>> create mode 100644 tests/btrfs/254.out
>>
>> diff --git a/tests/btrfs/254 b/tests/btrfs/254
>> new file mode 100755
>> index 000000000000..6c3414f73d15
>> --- /dev/null
>> +++ b/tests/btrfs/254
>> @@ -0,0 +1,110 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2021 Anand Jain. All Rights Reserved.
>> +# Copyright (c) 2021 Oracle. All Rights Reserved.
>> +#
>> +# FS QA Test No. 254
>> +#
>> +# Test if the kernel can free the stale device entries.
>> +#
>
> Can you include the patch name here as well, it makes it easier when I'm
> rebasing our staging branch to figure out if I need to disable a new test for
> our overnight runs.
>

Ok. I will include.

>> +. ./common/preamble
>> +_begin_fstest auto quick
>> +
>> +# Override the default cleanup function.
>> +node=$seq-test
>> +cleanup_dmdev()
>> +{
>> + _dmsetup_remove $node
>> +}
>> +
>> +_cleanup()
>> +{
>> + cd /
>> + rm -f $tmp.*
>> + rm -rf $seq_mnt > /dev/null 2>&1
>> + cleanup_dmdev
>> +}
>> +
>> +# Import common functions.
>> +. ./common/filter
>> +. ./common/filter.btrfs
>> +
>> +# real QA test starts here
>> +_supported_fs btrfs
>> +_require_scratch_dev_pool 3
>> +_require_block_device $SCRATCH_DEV
>> +_require_dm_target linear
>> +_require_btrfs_forget_or_module_loadable
>> +_require_scratch_nocheck
>> +_require_command "$WIPEFS_PROG" wipefs
>> +
>> +_scratch_dev_pool_get 3
>> +
>> +setup_dmdev()
>> +{
>> + # Some small size.
>> + size=$((1024 * 1024 * 1024))
>> + size_in_sector=$((size / 512))
>> +
>> + table="0 $size_in_sector linear $SCRATCH_DEV 0"
>> + _dmsetup_create $node --table "$table" || \
>> + _fail "setup dm device failed"
>> +}
>> +
>> +# Use a known it is much easier to debug.
>> +uuid="--uuid 12345678-1234-1234-1234-123456789abc"
>> +lvdev=/dev/mapper/$node
>> +
>> +seq_mnt=$TEST_DIR/$seq.mnt
>> +mkdir -p $seq_mnt
>> +
>> +test_forget()
>> +{
>> + setup_dmdev
>> + dmdev=$(realpath $lvdev)
>> +
>> + _mkfs_dev $uuid $dmdev
>> +
>> + # Check if we can un-scan using the mapper device path.
>> + $BTRFS_UTIL_PROG device scan --forget $lvdev
>> +
>> + # Cleanup
>> + $WIPEFS_PROG -a $lvdev > /dev/null 2>&1
>> + $BTRFS_UTIL_PROG device scan --forget
>> +
>> + cleanup_dmdev
>> +}
>> +
>> +test_add_device()
>> +{
>> + setup_dmdev
>> + dmdev=$(realpath $lvdev)
>> + scratch_dev2=$(echo $SCRATCH_DEV_POOL | awk '{print $2}')
>> + scratch_dev3=$(echo $SCRATCH_DEV_POOL | awk '{print $3}')
>> +
>> + _mkfs_dev $scratch_dev3
>> + _mount $scratch_dev3 $seq_mnt
>> +
>> + _mkfs_dev $uuid -draid1 -mraid1 $dmdev $scratch_dev2
>> +
>> + # Add device should free the device under $uuid in the kernel.
>> + $BTRFS_UTIL_PROG device add -f $lvdev $seq_mnt
>> +
>
> You need to redirect this to /dev/null, otherwise we get the TRIM message with
> newer btrfs-progs.
>

Ok.

>> + _mount -o degraded $scratch_dev2 $SCRATCH_MNT
>> +
>> + # Check if the missing device is shown.
>> + $BTRFS_UTIL_PROG filesystem show -m $SCRATCH_MNT | \
>> + _filter_btrfs_filesystem_show
>> +
>> + $UMOUNT_PROG $seq_mnt
>> + _scratch_unmount
>> + cleanup_dmdev
>> +}
>> +
>> +test_forget
>> +test_add_device
>> +
>> +_scratch_dev_pool_put
>> +
>> +status=0
>> +exit
>> diff --git a/tests/btrfs/254.out b/tests/btrfs/254.out
>> new file mode 100644
>> index 000000000000..20819cf5140c
>> --- /dev/null
>> +++ b/tests/btrfs/254.out
>> @@ -0,0 +1,6 @@
>> +QA output created by 254
>> +Label: none uuid: <UUID>
>> + Total devices <NUM> FS bytes used <SIZE>
>> + devid <DEVID> size <SIZE> used <SIZE> path SCRATCH_DEV
>> + *** Some devices missing
>
> I ran this on a box without your fix and I got this failure
>
> [root@xfstests2 xfstests-dev]# cat /xfstests-dev/results//kdave/btrfs/254.out.bad
> QA output created by 254

> ERROR: cannot unregister device '/dev/mapper/254-test': No such file or directory

Without the fix the error is expected.

> Label: none uuid: <UUID>
> Total devices <NUM> FS bytes used <SIZE>
> devid <DEVID> size <SIZE> used <SIZE> path SCRATCH_DEV
> *** Some devices missing
>
> Is this what you're expecting?

Hmm, no. Without the fix, we shouldn't see the missing here.

> I was expecting to not see the "*** Some devices
> missing" part as well, but I guess that's the racier part?

Right. I am guessing race with udev auto scan?

> It does fail properly without the patch and pass with your patch, so as long as
> this is what you expect to see then I'm good with this part. Thanks,

Yeah, we shouldn't see missing device _without_ the fix.
Could you please share your xfstests config?

Thanks, Anand

>
> Josef