On 2023/6/6 17:35, Filipe Manana wrote:
On Tue, Jun 6, 2023 at 4:20 AM Qu Wenruo <wqu@xxxxxxxx> wrote:
There is a regression on btrfs read-only scrub behavior.
The commit e02ee89baa66 ("btrfs: scrub: switch scrub_simple_mirror() to
scrub_stripe infrastructure") makes btrfs scrub to ignore the read-only
flag completely, causing scrub to always fix the corruption.
This test case would create an fs with repairable corruptions, then run
a read-only scrub, and finally to make sure the corruption is not
repaired.
Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
---
tests/btrfs/288 | 65 +++++++++++++++++++++++++++++++++++++++++++++
tests/btrfs/288.out | 39 +++++++++++++++++++++++++++
2 files changed, 104 insertions(+)
create mode 100755 tests/btrfs/288
create mode 100644 tests/btrfs/288.out
diff --git a/tests/btrfs/288 b/tests/btrfs/288
new file mode 100755
index 00000000..7795bdd9
--- /dev/null
+++ b/tests/btrfs/288
@@ -0,0 +1,65 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2023 SUSE Linux Products GmbH. All Rights Reserved.
+#
+# FS QA Test 288
+#
+# Make sure btrfs-scrub respects the read-only flag.
+#
+. ./common/preamble
+_begin_fstest auto repair quick volume scrub
+
+# For filedefrag and all the filters
This comment is a bit confusing. File defrag? The test doesn't exercise defrag.
Probably just leaving the comment out is better, it's obvious since we
are using filters.
+. ./common/filter
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs btrfs
+_require_scratch_dev_pool 2
+
+_require_odirect
+# Overwriting data is forbidden on a zoned block device
+_require_non_zoned_device "${SCRATCH_DEV}"
Can we please get a _fixed_by_kernel_commit call to identify the patch
that fixes the regression?
Unfortunately it's not yet merged, we only have the offending commit.
+
+_scratch_dev_pool_get 2
+
+# Step 1, create a raid btrfs with one 128K file
+echo "step 1......mkfs.btrfs"
+_scratch_pool_mkfs -d raid1 -b 1G >> $seqres.full 2>&1
+_scratch_mount
+
+$XFS_IO_PROG -f -d -c "pwrite -S 0xaa -b 128K 0 128K" "$SCRATCH_MNT/foobar" |\
+ _filter_xfs_io_offset
So why do we filter offsets?
Why not just a plain _filter_xfs_io as we most commonly do?
Oh yep, that's better.
Thanks for the review,
Qu
Thanks.
+
+# Step 2, corrupt one mirror so we can still repair the fs.
+echo "step 2......corrupt one mirror"
+# ensure btrfs-map-logical sees the tree updates
+sync
+
+logical=$(_btrfs_get_first_logical $SCRATCH_MNT/foobar)
+
+physical1=$(_btrfs_get_physical ${logical} 1)
+devpath1=$(_btrfs_get_device_path ${logical} 1)
+
+_scratch_unmount
+
+echo " corrupt stripe #1, devpath $devpath1 physical $physical1" \
+ >> $seqres.full
+$XFS_IO_PROG -d -c "pwrite -S 0xf1 -b 64K $physical1 64K" $devpath1 \
+ > /dev/null
+
+
+# Step 3, do a read-only scrub, which should not fix the corruption.
+_scratch_mount -o ro
+$BTRFS_UTIL_PROG scrub start -BRrd $SCRATCH_MNT >> $seqres.full 2>&1
+_scratch_unmount
+
+# Step 4, make sure the corruption is still there
+$XFS_IO_PROG -d -c "pread -v -b 512 $physical1 512" $devpath1 |\
+ _filter_xfs_io_offset
+
+_scratch_dev_pool_put
+# success, all done
+status=0
+exit
diff --git a/tests/btrfs/288.out b/tests/btrfs/288.out
new file mode 100644
index 00000000..c6c8e886
--- /dev/null
+++ b/tests/btrfs/288.out
@@ -0,0 +1,39 @@
+QA output created by 288
+step 1......mkfs.btrfs
+wrote 131072/131072 bytes
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+step 2......corrupt one mirror
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+XXXXXXXX: f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 f1 ................
+read 512/512 bytes
+XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
--
2.39.0
