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@xxxxxxxx> > > Signed-off-by: Boris Burkov <boris@xxxxxx> > > --- > > 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@xxxxxxxxxx> > > 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 > >