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

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

On 16/10/24 07:27, 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.
> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
> Changelog:
> v2:
> - update to real copyright info
> - add extra rw->ro transition checks
> - remove unnecessary _require_test
> ---
>   tests/btrfs/323     | 49 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/btrfs/323.out |  2 ++
>   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..ca57b1a1e
> --- /dev/null
> +++ b/tests/btrfs/323
> @@ -0,0 +1,49 @@
> +#! /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

This test case includes device addition; please also add the
'volume' group.

> +
> +. ./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 2
> +dev_seed=$(echo $SCRATCH_DEV_POOL | $AWK_PROG '{print $1}')
> +dev_sprout=$(echo $SCRATCH_DEV_POOL | $AWK_PROG '{print $2}')
> +

There are predefined variables $SCRATCH_DEV and $SPARE_DEV for these:

_scratch_dev_pool_get 1
_spare_dev_get


> +# create a read-only fs based off a read-only seed device
> +_mkfs_dev $dev_seed

_scratch_mkfs


> +$BTRFS_TUNE_PROG -S 1 $dev_seed

$BTRFS_TUNE_PROG -S 1 $SCRATCH_DEV


> +_mount $dev_seed $SCRATCH_MNT >>$seqres.full 2>&1


_scratch_mount


> +_btrfs device add -f $dev_sprout $SCRATCH_MNT >>$seqres.full
> +


_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 rw $SCRATCH_MNT
> +_mount -o remount,rw $SCRATCH_MNT
> +findmnt -n -O ro $SCRATCH_MNT
> +
> +# 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

> +_scratch_dev_pool_put
> +


Thanks,
Anand

> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/btrfs/323.out b/tests/btrfs/323.out
> new file mode 100644
> index 000000000..5dba9b5f0
> --- /dev/null
> +++ b/tests/btrfs/323.out
> @@ -0,0 +1,2 @@
> +QA output created by 323
> +Silence is golden