Re: [PATCH] fstests: btrfs/121: allow snapshot with invalid qgroup to return error

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]





在 2024/3/4 14:48, Zorro Lang 写道:
On Sun, Mar 03, 2024 at 05:22:51PM +1030, Qu Wenruo wrote:
[BUG]
After incoming kernel commit "btrfs: qgroup: verify btrfs_qgroup_inherit
parameter", test case btrfs/121 would fail like this:

btrfs/121 1s ... [failed, exit status 1]- output mismatch (see /xfstests/results//btrfs/121.out.bad)
     --- tests/btrfs/121.out	2022-05-11 09:55:30.739999997 +0800
     +++ /xfstests/results//btrfs/121.out.bad	2024-03-03 13:33:38.076666665 +0800
     @@ -1,2 +1,3 @@
      QA output created by 121
     -Silence is golden
     +failed: '/usr/bin/btrfs subvolume snapshot -i 1/10 /mnt/scratch /mnt/scratch/snap1'
     +(see /xfstests/results//btrfs/121.full for details)
     ...
     (Run 'diff -u /xfstests/tests/btrfs/121.out /xfstests/results//btrfs/121.out.bad'  to see the entire diff)

[CAUSE]
The incoming kernel commit would do early qgroups validation before
subvolume/snapshot creation, and reject invalid qgroups immediately.

Meanwhile that test case itself still assume the ioctl would go on
without any error, thus the new behavior would break the test case.

[FIX]
Instead of relying on the snapshot creation ioctl return value, we just
completely ignore the output of that snapshot creation.
Then manually check if the fs is still read-write.

For different kernels (3 cases), they would lead to the following
results:

- Older unpatched kernel
   The filesystem would trigger a transaction abort (would be caught by
   dmesg filter), and also fail the "touch" command.

- Older but patched kernel
   The filesystem continues to create the snapshot, while still keeps the
   fs read-write.

- Latest kernel with qgroup validation
   The filesystem refuses to create the snapshot, while still keeps the
   fs read-write.

Both "older but patched" and "latest" kernels would still pass the test
case, even with different behaviors.

Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
---
  tests/btrfs/121 | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/btrfs/121 b/tests/btrfs/121
index f4d54962..15a54274 100755
--- a/tests/btrfs/121
+++ b/tests/btrfs/121
@@ -24,8 +24,14 @@ _require_scratch
  _scratch_mkfs >/dev/null
  _scratch_mount
  _run_btrfs_util_prog quota enable $SCRATCH_MNT
-# The qgroup '1/10' does not exist and should be silently ignored
-_run_btrfs_util_prog subvolume snapshot -i 1/10 $SCRATCH_MNT $SCRATCH_MNT/snap1
+# The qgroup '1/10' does not exist. The kernel should either gives an error
+# (newer kernel with invalid qgroup detection) or ignore it (older kernel with
+# above fix).
+# Either way, we just ignore the output completely, and we will check if the fs
+# is still RW later.

The explanation makes sense to me, just ask if you might want to output to .full
file, to save some information for debug if the test fails? I can help to change
the "&> /dev/null" to "&> $seqres.full" if you only need to change.

Oh, that's very kind of you.

Although in that case "&>" would overwrite the .full file,
">> $seqres.full 2>&1" would be better IHMO.

Thanks,
Qu


Reviewed-by: Zorro Lang <zlang@xxxxxxxxxx>

Thanks,
Zorro

+$BTRFS_UTIL_PROG subvolume snapshot -i 1/10 $SCRATCH_MNT $SCRATCH_MNT/snap1 &> /dev/null
+
+touch $SCRATCH_MNT/foobar
echo "Silence is golden" --
2.42.0







[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux