Re: [PATCH v3] btrfs: add test for cleaner thread under seed-sprout

[Anand Jain <anand.jain@oracle.com>]

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