| Message ID | 20ebca40c55ed9207b6ea77aa50e93f3baf698ad.1729101127.git.boris@bur.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] btrfs: add test for cleaner thread under seed-sprout | expand |
On 17/10/24 01:54, Boris Burkov wrote: > We have a longstanding bug that creating a seed sprout fs with the > ro->rw transition done with > > mount -o remount,rw $mnt > > instead of > > umount $mnt > mount $sprout_dev $mnt > > results in an fs without BTRFS_FS_OPEN set, which fails to ever run the > critical btrfs cleaner thread. > > This test reproduces that bug and detects it by creating and deleting a > subvolume, then triggering the cleaner thread. The expected behavior is > for the cleaner thread to delete the stale subvolume and for the list to > show no entries. Without the fix, we see a DELETED entry for the subvol. > > Reviewed-by: Qu Wenruo <wqu@suse.com> > Signed-off-by: Boris Burkov <boris@bur.io> > --- > Changelog: > v3: > - add volume group > - switch to SCRATCH_DEV/SPARE_DEV > - filter scratch devs from mount/findmnt output instead of suppressing. > This adds the expected read only message that comes with mounting seed > devices to the golden output, and makes the ro/rw check more natural. > v2: > - update to real copyright info > - add extra rw->ro transition checks > - remove unnecessary _require_test > --- > tests/btrfs/323 | 47 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/323.out | 4 ++++ > 2 files changed, 51 insertions(+) > create mode 100755 tests/btrfs/323 > create mode 100644 tests/btrfs/323.out > > diff --git a/tests/btrfs/323 b/tests/btrfs/323 > new file mode 100755 > index 000000000..4e389d66a > --- /dev/null > +++ b/tests/btrfs/323 > @@ -0,0 +1,47 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2024 Meta Platforms, Inc. All Rights Reserved. > +# > +# FS QA Test 323 > +# > +# Test that remounted seed/sprout device FS is fully functional. For example, that it can purge stale subvolumes. > +# > +. ./common/preamble > +_begin_fstest auto quick seed remount volume Zorro, can you pls add it while merging. Reviewed-by: Anand Jain <anand.jain@oracle.com> Thanks Anand > + > +. ./common/filter > +_require_command "$BTRFS_TUNE_PROG" btrfstune > +_require_scratch_dev_pool 2 > + > +_fixed_by_kernel_commit XXXXXXXX \ > + "btrfs: do not clear read-only when adding sprout device" > + > +_scratch_dev_pool_get 1 > +_spare_dev_get > + > +# create a read-only fs based off a read-only seed device > +_scratch_mkfs >>$seqres.full > +$BTRFS_TUNE_PROG -S 1 $SCRATCH_DEV > +_scratch_mount 2>&1 | _filter_scratch > +_btrfs device add -f $SPARE_DEV $SCRATCH_MNT >>$seqres.full > + > +# switch ro to rw, checking that it was ro before and rw after > +findmnt -n -O ro -o TARGET $SCRATCH_MNT | _filter_scratch > +_mount -o remount,rw $SCRATCH_MNT > +findmnt -n -O rw -o TARGET $SCRATCH_MNT | _filter_scratch > + > +# do stuff in the seed/sprout fs > +_btrfs subvolume create $SCRATCH_MNT/subv > +_btrfs subvolume delete $SCRATCH_MNT/subv > + > +# trigger cleaner thread without remounting > +_btrfs filesystem sync $SCRATCH_MNT > + > +# expect no deleted subvolumes remaining > +$BTRFS_UTIL_PROG subvolume list -d $SCRATCH_MNT > + > +_spare_dev_put > + > +# success, all done > +status=0 > +exit > diff --git a/tests/btrfs/323.out b/tests/btrfs/323.out > new file mode 100644 > index 000000000..1ca2e4b13 > --- /dev/null > +++ b/tests/btrfs/323.out > @@ -0,0 +1,4 @@ > +QA output created by 323 > +mount: SCRATCH_MNT: WARNING: source write-protected, mounted read-only. > +SCRATCH_MNT > +SCRATCH_MNT
On Sat, Oct 19, 2024 at 06:19:20AM +0800, Anand Jain wrote: > On 17/10/24 01:54, Boris Burkov wrote: > > We have a longstanding bug that creating a seed sprout fs with the > > ro->rw transition done with > > > > mount -o remount,rw $mnt > > > > instead of > > > > umount $mnt > > mount $sprout_dev $mnt > > > > results in an fs without BTRFS_FS_OPEN set, which fails to ever run the > > critical btrfs cleaner thread. > > > > This test reproduces that bug and detects it by creating and deleting a > > subvolume, then triggering the cleaner thread. The expected behavior is > > for the cleaner thread to delete the stale subvolume and for the list to > > show no entries. Without the fix, we see a DELETED entry for the subvol. > > > > Reviewed-by: Qu Wenruo <wqu@suse.com> > > Signed-off-by: Boris Burkov <boris@bur.io> > > --- > > Changelog: > > v3: > > - add volume group > > - switch to SCRATCH_DEV/SPARE_DEV > > - filter scratch devs from mount/findmnt output instead of suppressing. > > This adds the expected read only message that comes with mounting seed > > devices to the golden output, and makes the ro/rw check more natural. > > v2: > > - update to real copyright info > > - add extra rw->ro transition checks > > - remove unnecessary _require_test > > --- > > tests/btrfs/323 | 47 +++++++++++++++++++++++++++++++++++++++++++++ > > tests/btrfs/323.out | 4 ++++ > > 2 files changed, 51 insertions(+) > > create mode 100755 tests/btrfs/323 > > create mode 100644 tests/btrfs/323.out > > > > diff --git a/tests/btrfs/323 b/tests/btrfs/323 > > new file mode 100755 > > index 000000000..4e389d66a > > --- /dev/null > > +++ b/tests/btrfs/323 > > @@ -0,0 +1,47 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2024 Meta Platforms, Inc. All Rights Reserved. > > +# > > +# FS QA Test 323 > > +# > > +# Test that remounted seed/sprout device FS is fully functional. For example, that it can purge stale subvolumes. > > +# > > +. ./common/preamble > > +_begin_fstest auto quick seed remount > > volume > > Zorro, can you pls add it while merging. Sure, thanks Anand! > > Reviewed-by: Anand Jain <anand.jain@oracle.com> > > Thanks > Anand > > > > + > > +. ./common/filter > > +_require_command "$BTRFS_TUNE_PROG" btrfstune > > +_require_scratch_dev_pool 2 > > + > > +_fixed_by_kernel_commit XXXXXXXX \ > > + "btrfs: do not clear read-only when adding sprout device" > > + > > +_scratch_dev_pool_get 1 > > +_spare_dev_get > > + > > +# create a read-only fs based off a read-only seed device > > +_scratch_mkfs >>$seqres.full > > +$BTRFS_TUNE_PROG -S 1 $SCRATCH_DEV > > +_scratch_mount 2>&1 | _filter_scratch > > +_btrfs device add -f $SPARE_DEV $SCRATCH_MNT >>$seqres.full > > + > > +# switch ro to rw, checking that it was ro before and rw after > > +findmnt -n -O ro -o TARGET $SCRATCH_MNT | _filter_scratch > > +_mount -o remount,rw $SCRATCH_MNT > > +findmnt -n -O rw -o TARGET $SCRATCH_MNT | _filter_scratch > > + > > +# do stuff in the seed/sprout fs > > +_btrfs subvolume create $SCRATCH_MNT/subv > > +_btrfs subvolume delete $SCRATCH_MNT/subv > > + > > +# trigger cleaner thread without remounting > > +_btrfs filesystem sync $SCRATCH_MNT > > + > > +# expect no deleted subvolumes remaining > > +$BTRFS_UTIL_PROG subvolume list -d $SCRATCH_MNT > > + > > +_spare_dev_put > > + > > +# success, all done > > +status=0 > > +exit > > diff --git a/tests/btrfs/323.out b/tests/btrfs/323.out > > new file mode 100644 > > index 000000000..1ca2e4b13 > > --- /dev/null > > +++ b/tests/btrfs/323.out > > @@ -0,0 +1,4 @@ > > +QA output created by 323 > > +mount: SCRATCH_MNT: WARNING: source write-protected, mounted read-only. > > +SCRATCH_MNT > > +SCRATCH_MNT > >
diff --git a/tests/btrfs/323 b/tests/btrfs/323 new file mode 100755 index 000000000..4e389d66a --- /dev/null +++ b/tests/btrfs/323 @@ -0,0 +1,47 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2024 Meta Platforms, Inc. All Rights Reserved. +# +# FS QA Test 323 +# +# Test that remounted seed/sprout device FS is fully functional. For example, that it can purge stale subvolumes. +# +. ./common/preamble +_begin_fstest auto quick seed remount + +. ./common/filter +_require_command "$BTRFS_TUNE_PROG" btrfstune +_require_scratch_dev_pool 2 + +_fixed_by_kernel_commit XXXXXXXX \ + "btrfs: do not clear read-only when adding sprout device" + +_scratch_dev_pool_get 1 +_spare_dev_get + +# create a read-only fs based off a read-only seed device +_scratch_mkfs >>$seqres.full +$BTRFS_TUNE_PROG -S 1 $SCRATCH_DEV +_scratch_mount 2>&1 | _filter_scratch +_btrfs device add -f $SPARE_DEV $SCRATCH_MNT >>$seqres.full + +# switch ro to rw, checking that it was ro before and rw after +findmnt -n -O ro -o TARGET $SCRATCH_MNT | _filter_scratch +_mount -o remount,rw $SCRATCH_MNT +findmnt -n -O rw -o TARGET $SCRATCH_MNT | _filter_scratch + +# do stuff in the seed/sprout fs +_btrfs subvolume create $SCRATCH_MNT/subv +_btrfs subvolume delete $SCRATCH_MNT/subv + +# trigger cleaner thread without remounting +_btrfs filesystem sync $SCRATCH_MNT + +# expect no deleted subvolumes remaining +$BTRFS_UTIL_PROG subvolume list -d $SCRATCH_MNT + +_spare_dev_put + +# success, all done +status=0 +exit diff --git a/tests/btrfs/323.out b/tests/btrfs/323.out new file mode 100644 index 000000000..1ca2e4b13 --- /dev/null +++ b/tests/btrfs/323.out @@ -0,0 +1,4 @@ +QA output created by 323 +mount: SCRATCH_MNT: WARNING: source write-protected, mounted read-only. +SCRATCH_MNT +SCRATCH_MNT