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

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

[Qu Wenruo]

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
+
+# 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
+
+# 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
-- 
2.39.0

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

[Anand Jain]

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

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

[Anand Jain]

  With the following changes added to the local repo, I am currently 
testing it.

On 13/01/2023 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".

  Updated the final commit title.

> 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
> +

  Removed commented code.

> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs btrfs
> +_require_scratch_dev_pool 3
> +

_fixed_by_kernel_commit b7625f461da6 \
        "btrfs: sysfs: update fs features directory asynchronously"

> +# 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
> +

  call _fail() upon failure.

> +_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
> +

# Sync before checking for sysfs update during cleaner_kthread().
sync

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