On Tue, Jan 04, 2022 at 07:25:20PM +0800, Anand Jain wrote:
>
> Gentle ping? More below.
It's already been merged in last update.
>
> On 20/12/2021 19:06, Anand Jain wrote:
> >
> >
> > On 19/12/2021 22:02, Eryu Guan wrote:
> > > On Sat, Dec 11, 2021 at 02:14:41AM +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@xxxxxxxxxx>
> > >
> > > I was testing with v5.16-rc2 kernel, which should not contain the kernel
> > > fix, but test still passed for me, I was testing with three loop devices
> > > as SCRATCH_DEV_POOL, and all default mkfs & mount options
> > >
> > > SECTION -- btrfs
> > > RECREATING -- btrfs on /dev/mapper/testvg-lv1
> > > FSTYP -- btrfs
> > > PLATFORM -- Linux/x86_64 fedoravm 5.16.0-rc2 #22 SMP PREEMPT
> > > Mon Nov 29 00:54:26 CST 2021
> > > MKFS_OPTIONS -- /dev/loop0
> > > MOUNT_OPTIONS -- /dev/loop0 /mnt/scratch
> > > btrfs/254 5s ... 5s
> > > Ran: btrfs/254
> > > Passed all 1 tests
> > >
> > > Anything wrong with my setup?
> > >
> > > And if tested with lv devices as SCRATCH_DEV_POOL
> > >
> > > SCRATCH_DEV_POOL="/dev/mapper/testvg-lv2 /dev/mapper/testvg-lv3
> > > /dev/mapper/testvg-lv4 /dev/mapper/testvg-lv5
> > > /dev/mapper/testvg-lv6"
> > >
> > > I got the following test failure
> > >
> > > QA output created by 254
> > > +ERROR: cannot unregister device '/dev/mapper/254-test': No such
> > > file or directory
> > > Label: none uuid: <UUID>
> > > Total devices <NUM> FS bytes used <SIZE>
> > > devid <DEVID> size <SIZE> used <SIZE> path SCRATCH_DEV
> > >
> > > Maybe we should use _require_scratch_nolvm as well?
> > >
> >
>
> > The test case is inconsistent because the systemd-udev block device scan
> > interferes with the test script. There is no way to disable the
> > systemd-udev scan (that I could find).
> >
> > The same inconsistency (due to race with systemd-udev scan) would
> > persist with/without lvm.
> >
> > So when the race fails, the test case is successful to reproduce the
> > issue. As you saw in the 2nd iteration.
>
>
> There isn't any approach to stop the test case from racing with the device
> scan, so reproducing the issue is inconsistent. As we have had some success
> so IMO this test case can still integrate, it will help to verify the kernel
Agreed.
> fix on some systems.
>
> More below.
>
> >
> > Any suggestions?
> >
> > Thanks, Anand
> >
> >
> > > > ---
> > > > v2: Add kernel patch title in the test case
> > > > Redirect device add output to /dev/null (avoids tirm message)
> > > > Use the lv path for mkfs and the dm path for the device add
> > > > so that now path used in udev scan should match with what
> > > > we already have in the kernel memory.
> > > >
> > > > - _mkfs_dev $uuid -draid1 -mraid1 $dmdev $scratch_dev2
> > > > + _mkfs_dev $uuid -draid1 -mraid1 $lvdev $scratch_dev2
> > > > # Add device should free the device under $uuid in the kernel.
> > > > - $BTRFS_UTIL_PROG device add -f $lvdev $seq_mnt > /dev/null 2>&1
> > > > + $BTRFS_UTIL_PROG device add -f $dmdev $seq_mnt > /dev/null 2>&1
> > > >
> > > >
> > > > tests/btrfs/254 | 113 ++++++++++++++++++++++++++++++++++++++++++++
> > > > tests/btrfs/254.out | 6 +++
> > > > 2 files changed, 119 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..b70b9d165897
> > > > --- /dev/null
> > > > +++ b/tests/btrfs/254
> > > > @@ -0,0 +1,113 @@
> > > > +#! /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.
> > > > +#
> > > > +# Tests bug fixed by the kernel patch:
> > > > +# btrfs: harden identification of the stale device
> > > > +#
> > > > +. ./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
> > >
> > > Should wipefs in cleanup as well, otherwise test fails with non-unique
> > > UUID
> > >
> > > -Label: none uuid: <UUID>
> > > - Total devices <NUM> FS bytes used <SIZE>
> > > - devid <DEVID> size <SIZE> used <SIZE> path SCRATCH_DEV
> > > - *** Some devices missing
>
> > > +ERROR: non-unique UUID: 12345678-1234-1234-1234-123456789abc
>
>
> We don't need non-unique UUID. I have fixed this in v3.
Then would you please help patch the existing test? And we should add
'volume' group as well?
Thanks,
Eryu
>
> Thanks,
> Anand
>
>
> > > +btrfs-progs v5.4
> > > +See http://btrfs.wiki.kernel.org for more information.
> > >
> > > Thanks,
> > > Eryu
> > >
> > > > +}
> > > > +
> > > > +# 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 $lvdev $scratch_dev2
> > > > +
> > > > + # Add device should free the device under $uuid in the kernel.
> > > > + $BTRFS_UTIL_PROG device add -f $dmdev $seq_mnt > /dev/null 2>&1
> > > > +
> > > > + _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
> > > > +
> > > > --
> > > > 2.27.0
