All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fstests: btrfs: add a test case for deleting partially owned qgroup
@ 2026-10-02 11:59 Qu Wenruo
  2026-10-02 17:02 ` Filipe Manana
  0 siblings, 1 reply; 2+ messages in thread
From: Qu Wenruo @ 2026-10-02 11:59 UTC (permalink / raw)
  To: linux-btrfs, fstests

There is a bug report that, when deleting a partially owned qgroup,
btrfs will delete the on-disk items but leave the in-memory rb-tree and
sysfs entries untouched.

Introduce a regression test for it, verifying the behavior by:

- Create a btrfs with a specific qgroup layout

              2/1
               |
              1/1
               |
   0/subv1  0/snap1

   All involved qgroups will have "1M + nodesize" as reference number,
   and "nodesize" as exclusive number.

   For the remaining example I'll use 16K as nodesize.

- Remove the qgroup relationship between 0/snap1 and 1/1
  This will mark qgroup inconsistent, but leave the old numbers.

              2/1 (rfer: 1M+16K, excl: 16K)
               |
              1/1 (rfer: 1M+16K, excl: 16K)

- Remove qgroup 1/1
  Which will automatically remove the relationship between 1/1 and 2/1.

  And since qgroup 1/1 is not fully owned (rfer > excl), this will mark
  qgroup inconsistent and return 1.

  For unpatched kernels, that positive return value is treated as an
  error, causing on-disk metadata items removed but still leave
  the rb-tree and sysfs entries untouched.

- Check if /sys/fs/btrfs/<fsid>/qgroups/1_1/ still exists
  If exists, the kernel is not yet patched.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/358     | 79 +++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/358.out |  2 ++
 2 files changed, 81 insertions(+)
 create mode 100755 tests/btrfs/358
 create mode 100644 tests/btrfs/358.out

diff --git a/tests/btrfs/358 b/tests/btrfs/358
new file mode 100755
index 00000000..642b0808
--- /dev/null
+++ b/tests/btrfs/358
@@ -0,0 +1,79 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 SUSE S.A.  All Rights Reserved.
+#
+# FS QA Test 358
+#
+# Verify that when deleting a partially owned qgroup (excl != rfer),
+# the sysfs and in-memory rb-tree entries are also properly removed.
+#
+. ./common/preamble
+_begin_fstest auto quick qgroup
+
+_fixed_by_kernel_commit xxxxxxxxxxxx \
+	"btrfs: qgroup: do not treat \"ret > 0\" as error when deleting a qgroup"
+
+_require_btrfs_sysfs_fsid
+_require_btrfs_command qgroup remove --no-rescan
+_require_scratch
+_require_btrfs_qgroup_report
+
+# This implies making a btrfs and enables full qgroup mode.
+_require_scratch_qgroup
+_scratch_mount
+
+fsid=$(_btrfs_get_fsid "$SCRATCH_MNT")
+
+# We require /sys/fs/btrfs/<fsid>/qgroups/ directory
+if [ ! -d "/sys/fs/btrfs/$fsid/qgroups/0_5" ]; then
+	_scratch_unmount
+	_notrun "No sysfs qgroups directory"
+fi
+
+_btrfs subvolume create "$SCRATCH_MNT/subv1"
+_pwrite_byte 0x00 0 1M "$SCRATCH_MNT/subv1/data1" &> /dev/null
+_btrfs subvolume snapshot "$SCRATCH_MNT/subv1" "$SCRATCH_MNT/snap1"
+snap1id=$(_btrfs_get_subvolid $SCRATCH_MNT snap1)
+
+# Create the following qgroup layout and rescan.
+#                    2/1
+#                     |
+#                    1/1
+#                     |
+#  0/subv1           0/snap1
+#
+# Since snap1 is a snapshot of subv1, both will have 1M shared data
+# plus nodesize as exclusive metadata.
+# This means neither of them is fully exclusive (rfer > excl).
+_btrfs qgroup create 1/1 "$SCRATCH_MNT"
+_btrfs qgroup create 2/1 "$SCRATCH_MNT"
+_btrfs qgroup assign 1/1 2/1 "$SCRATCH_MNT"
+_btrfs qgroup assign 0/$snap1id 1/1 "$SCRATCH_MNT"
+
+# Do a rescan, as the above assign will mark the qgroup inconsistent.
+_btrfs quota rescan -w "$SCRATCH_MNT"
+
+# Now remove the qgroup relationship between 0/snap1 and 1/1
+# This will mark qgroup inconsistent but we do not want to trigger a rescan
+# automatically.
+# This will leave the old numbers (rfer > excl) for 1/1.
+_btrfs qgroup remove --no-rescan "0/$snap1id" 1/1 "$SCRATCH_MNT"
+
+# Now delete qgroup 1/1, which will automatically remove the relationship
+# between 1/1 and 2/1.
+# Since qgroup 1/1 still has stale values where rfer doesn't match excl,
+# the kernel marks the qgroups inconsistent and returns 1 to indicate a
+# rescan is needed.
+_btrfs qgroup destroy 1/1 "$SCRATCH_MNT"
+
+# For unpatched kernels, that positive return value is treated as an error,
+# causing btrfs to delete on-disk items for qgroup 1/1, but leaving the
+# in-memory rb-tree and sysfs entries untouched.
+# Check if the sysfs qgroup directory still has a directory for qgroup 1/1.
+if [ -d "/sys/fs/btrfs/$fsid/qgroups/1_1" ]; then
+	echo "Qgroup 1/1 still exists in sysfs qgroups directory"
+fi
+
+echo "Silence is golden"
+_scratch_unmount
+_exit 0
diff --git a/tests/btrfs/358.out b/tests/btrfs/358.out
new file mode 100644
index 00000000..8bcf2699
--- /dev/null
+++ b/tests/btrfs/358.out
@@ -0,0 +1,2 @@
+QA output created by 358
+Silence is golden
-- 
2.51.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] fstests: btrfs: add a test case for deleting partially owned qgroup
  2026-10-02 11:59 [PATCH] fstests: btrfs: add a test case for deleting partially owned qgroup Qu Wenruo
@ 2026-10-02 17:02 ` Filipe Manana
  0 siblings, 0 replies; 2+ messages in thread
From: Filipe Manana @ 2026-10-02 17:02 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs, fstests

On Fri, Oct 2, 2026 at 1:06 PM Qu Wenruo <wqu@suse.com> wrote:
>
> There is a bug report that, when deleting a partially owned qgroup,
> btrfs will delete the on-disk items but leave the in-memory rb-tree and
> sysfs entries untouched.
>
> Introduce a regression test for it, verifying the behavior by:
>
> - Create a btrfs with a specific qgroup layout
>
>               2/1
>                |
>               1/1
>                |
>    0/subv1  0/snap1
>
>    All involved qgroups will have "1M + nodesize" as reference number,
>    and "nodesize" as exclusive number.
>
>    For the remaining example I'll use 16K as nodesize.
>
> - Remove the qgroup relationship between 0/snap1 and 1/1
>   This will mark qgroup inconsistent, but leave the old numbers.
>
>               2/1 (rfer: 1M+16K, excl: 16K)
>                |
>               1/1 (rfer: 1M+16K, excl: 16K)
>
> - Remove qgroup 1/1
>   Which will automatically remove the relationship between 1/1 and 2/1.
>
>   And since qgroup 1/1 is not fully owned (rfer > excl), this will mark
>   qgroup inconsistent and return 1.
>
>   For unpatched kernels, that positive return value is treated as an
>   error, causing on-disk metadata items removed but still leave
>   the rb-tree and sysfs entries untouched.
>
> - Check if /sys/fs/btrfs/<fsid>/qgroups/1_1/ still exists
>   If exists, the kernel is not yet patched.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/btrfs/358     | 79 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/358.out |  2 ++
>  2 files changed, 81 insertions(+)
>  create mode 100755 tests/btrfs/358
>  create mode 100644 tests/btrfs/358.out
>
> diff --git a/tests/btrfs/358 b/tests/btrfs/358
> new file mode 100755
> index 00000000..642b0808
> --- /dev/null
> +++ b/tests/btrfs/358
> @@ -0,0 +1,79 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2026 SUSE S.A.  All Rights Reserved.
> +#
> +# FS QA Test 358
> +#
> +# Verify that when deleting a partially owned qgroup (excl != rfer),
> +# the sysfs and in-memory rb-tree entries are also properly removed.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick qgroup
> +
> +_fixed_by_kernel_commit xxxxxxxxxxxx \
> +       "btrfs: qgroup: do not treat \"ret > 0\" as error when deleting a qgroup"
> +
> +_require_btrfs_sysfs_fsid
> +_require_btrfs_command qgroup remove --no-rescan
> +_require_scratch
> +_require_btrfs_qgroup_report
> +
> +# This implies making a btrfs and enables full qgroup mode.
> +_require_scratch_qgroup
> +_scratch_mount
> +
> +fsid=$(_btrfs_get_fsid "$SCRATCH_MNT")
> +
> +# We require /sys/fs/btrfs/<fsid>/qgroups/ directory
> +if [ ! -d "/sys/fs/btrfs/$fsid/qgroups/0_5" ]; then
> +       _scratch_unmount

We shouldn't need to unmount explicitly, the fstests framework should do that.

> +       _notrun "No sysfs qgroups directory"
> +fi
> +
> +_btrfs subvolume create "$SCRATCH_MNT/subv1"
> +_pwrite_byte 0x00 0 1M "$SCRATCH_MNT/subv1/data1" &> /dev/null

We should redirect only stdout so that if an error occurs, we
immediately know the test failed because the write failed, not for
another reason.

Anyway:

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks.

> +_btrfs subvolume snapshot "$SCRATCH_MNT/subv1" "$SCRATCH_MNT/snap1"
> +snap1id=$(_btrfs_get_subvolid $SCRATCH_MNT snap1)
> +
> +# Create the following qgroup layout and rescan.
> +#                    2/1
> +#                     |
> +#                    1/1
> +#                     |
> +#  0/subv1           0/snap1
> +#
> +# Since snap1 is a snapshot of subv1, both will have 1M shared data
> +# plus nodesize as exclusive metadata.
> +# This means neither of them is fully exclusive (rfer > excl).
> +_btrfs qgroup create 1/1 "$SCRATCH_MNT"
> +_btrfs qgroup create 2/1 "$SCRATCH_MNT"
> +_btrfs qgroup assign 1/1 2/1 "$SCRATCH_MNT"
> +_btrfs qgroup assign 0/$snap1id 1/1 "$SCRATCH_MNT"
> +
> +# Do a rescan, as the above assign will mark the qgroup inconsistent.
> +_btrfs quota rescan -w "$SCRATCH_MNT"
> +
> +# Now remove the qgroup relationship between 0/snap1 and 1/1
> +# This will mark qgroup inconsistent but we do not want to trigger a rescan
> +# automatically.
> +# This will leave the old numbers (rfer > excl) for 1/1.
> +_btrfs qgroup remove --no-rescan "0/$snap1id" 1/1 "$SCRATCH_MNT"
> +
> +# Now delete qgroup 1/1, which will automatically remove the relationship
> +# between 1/1 and 2/1.
> +# Since qgroup 1/1 still has stale values where rfer doesn't match excl,
> +# the kernel marks the qgroups inconsistent and returns 1 to indicate a
> +# rescan is needed.
> +_btrfs qgroup destroy 1/1 "$SCRATCH_MNT"
> +
> +# For unpatched kernels, that positive return value is treated as an error,
> +# causing btrfs to delete on-disk items for qgroup 1/1, but leaving the
> +# in-memory rb-tree and sysfs entries untouched.
> +# Check if the sysfs qgroup directory still has a directory for qgroup 1/1.
> +if [ -d "/sys/fs/btrfs/$fsid/qgroups/1_1" ]; then
> +       echo "Qgroup 1/1 still exists in sysfs qgroups directory"
> +fi
> +
> +echo "Silence is golden"
> +_scratch_unmount
> +_exit 0
> diff --git a/tests/btrfs/358.out b/tests/btrfs/358.out
> new file mode 100644
> index 00000000..8bcf2699
> --- /dev/null
> +++ b/tests/btrfs/358.out
> @@ -0,0 +1,2 @@
> +QA output created by 358
> +Silence is golden
> --
> 2.51.2
>
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-02 17:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-02 11:59 [PATCH] fstests: btrfs: add a test case for deleting partially owned qgroup Qu Wenruo
2026-10-02 17:02 ` Filipe Manana

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.