Re: [PATCH] btrfs: add a test case to verify that per-fs features directory gets updated

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

On 1/13/23 15:06, Qu Wenruo wrote:
> Although btrfs has a per-fs feature directory, it's not properly
> refreshed after new features are enabled.
> 
> We had some attempts to do that properly, like commit 14e46e04958d
> ("btrfs: synchronize incompat feature bits with sysfs files").
> But unfortunately that commit get later reverted as some call sites is
> not safe to update sysfs files.
> 
> Now we have a new patch to properly refresh that per-fs features
> directory, titled "btrfs: update fs features sysfs directory asynchronously".
> 
> So it's time to add a test case for it. The test case itself is pretty
> straightforward:
> 
> - Make a very basic 3 disks btrfs
>    Only using the very basic profiles (DUP/SINGLE) so that even older
>    mkfs.btrfs can support.
> 
> - Make sure per-fs features directory doesn't contain "raid1c34" file
> 
> - Balance the metadata to RAID1C3 profile
> 
> - Verify the per-fs features directory contains "raid1c34" feature file
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   tests/btrfs/283     | 73 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/btrfs/283.out |  2 ++
>   2 files changed, 75 insertions(+)
>   create mode 100755 tests/btrfs/283
>   create mode 100644 tests/btrfs/283.out
> 
> diff --git a/tests/btrfs/283 b/tests/btrfs/283
> new file mode 100755
> index 00000000..6c431273
> --- /dev/null
> +++ b/tests/btrfs/283
> @@ -0,0 +1,73 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2023 SUSE Linux Products GmbH. All Rights Reserved.
> +#
> +# FS QA Test 283
> +#
> +# Make sure that per-fs features sysfs interface get properly updated
> +# when a new feature is added.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick balance
> +


> +# Override the default cleanup function.
> +# _cleanup()
> +# {
> +# 	cd /
> +# 	rm -r -f $tmp.*
> +# }
> +
> +# Import common functions.
> +# . ./common/filter
> +

   Please remove the template code.


> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_require_scratch_dev_pool 3
> +
> +# We need the global features support
> +_require_btrfs_fs_sysfs
> +
> +global_features="/sys/fs/btrfs/features"
> +# Make sure we have support RAID1C34 first
> +if [ ! -f "${global_features}/raid1c34" ]; then
> +	_notrun "no RAID1C34 support"
> +fi
> +
> +_scratch_dev_pool_get 3
> +
> +# Go the very basic profile first, so that even older progs can support it.
> +_scratch_pool_mkfs -m dup -d single >>$seqres.full 2>&1
> +
> +_scratch_mount
> +uuid="$(findmnt -n -o UUID "$SCRATCH_MNT")"
> +per_fs_features="/sys/fs/btrfs/${uuid}/features"
> +
> +# First we need per-fs features directory
> +if [ ! -d "${per_fs_features}" ]; then
> +	_notrun "no per-fs features sysfs directory"
> +fi
> +
> +# Make sure the per-fs features doesn't include raid1c34
> +if [ -f "${per_fs_features}/raid1c34" ]; then
> +	_fail "raid1c34 feature found unexpectedly"
> +fi
> +
> +# Balance to RAID1C3
> +$BTRFS_UTIL_PROG balance start -mconvert=raid1c3 "$SCRATCH_MNT" >> $seqres.full
> +

IMO, it is a good idea to call sync to ensure that the transaction is 
committed.

The rest looks good.

Thanks, Anand

> +# Check if the per-fs features directory contains raid1c34 now
> +# Make sure the per-fs features doesn't include raid1c34
> +if [ ! -f "${per_fs_features}/raid1c34" ]; then
> +	_fail "raid1c34 feature not found"
> +fi
> +
> +echo "Silence is golden"
> +
> +_scratch_unmount
> +_scratch_dev_pool_put
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/btrfs/283.out b/tests/btrfs/283.out
> new file mode 100644
> index 00000000..efb2c583
> --- /dev/null
> +++ b/tests/btrfs/283.out
> @@ -0,0 +1,2 @@
> +QA output created by 283
> +Silence is golden