fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] fstests: btrfs/15[78] btrfs/14[23]: Use more accurate devid/phsyical for corruption
@ 2019-12-11 10:40 Qu Wenruo
  2019-12-11 10:40 ` [PATCH 1/3] fstests: common: Use more accurate kernel config for _require_fail_make_request Qu Wenruo
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Qu Wenruo @ 2019-12-11 10:40 UTC (permalink / raw)
  To: fstests, linux-btrfs

Btrfs-progs commit c501c9e3b816 ("btrfs-progs: mkfs: match devid order
to the stripe index") changed the chunk layout, making it more human
friendly for manually offset calculation.

However btrfs/14[23] btrfs/15[78] are using hard-coded devid sequence
for corruption, thus they can't stand such change and fail.

This patchset will handle such problems in patch 2 and 3, to use more
accurate helper to replace the hard-coded one.

And the first patch will address a bad _notrun messages, which is mostly
related to btrfs/14[23].

Qu Wenruo (3):
  fstests: common: Use more accurate kernel config for
    _require_fail_make_request
  fstests: btrfs/14[23]: Use proper help to get both devid and physical
    offset for corruption.
  fstests: btrfs/15[78]: Use proper helper to get both devid and
    physical offset for corruption

 common/rc           |  2 +-
 tests/btrfs/142     | 50 +++++++++++++++++++++++++++++-------------
 tests/btrfs/142.out |  2 --
 tests/btrfs/143     | 48 +++++++++++++++++++++++++++++-----------
 tests/btrfs/143.out |  2 --
 tests/btrfs/157     | 53 +++++++++++++++++++++++++++++----------------
 tests/btrfs/157.out |  4 ----
 tests/btrfs/158     | 48 +++++++++++++++++++++++++---------------
 tests/btrfs/158.out |  4 ----
 9 files changed, 136 insertions(+), 77 deletions(-)

-- 
2.23.0


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

* [PATCH 1/3] fstests: common: Use more accurate kernel config for _require_fail_make_request
  2019-12-11 10:40 [PATCH 0/3] fstests: btrfs/15[78] btrfs/14[23]: Use more accurate devid/phsyical for corruption Qu Wenruo
@ 2019-12-11 10:40 ` Qu Wenruo
  2019-12-17 17:24   ` Filipe Manana
  2019-12-11 10:40 ` [PATCH 2/3] fstests: btrfs/14[23]: Use proper help to get both devid and physical offset for corruption Qu Wenruo
  2019-12-11 10:40 ` [PATCH 3/3] fstests: btrfs/15[78]: Use proper helper " Qu Wenruo
  2 siblings, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2019-12-11 10:40 UTC (permalink / raw)
  To: fstests, linux-btrfs

Just enabling CONFIG_FAIL_MAKE_REQUEST will not fulfill
_require_fail_make_request.

It's CONFIG_FAULT_INJECTION_DEBUG_FS.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 common/rc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index 5cdd829b..2d72f158 100644
--- a/common/rc
+++ b/common/rc
@@ -2357,7 +2357,7 @@ _require_fail_make_request()
 {
     [ -f "$DEBUGFS_MNT/fail_make_request/probability" ] \
 	|| _notrun "$DEBUGFS_MNT/fail_make_request \
- not found. Seems that CONFIG_FAIL_MAKE_REQUEST kernel config option not enabled"
+ not found. Seems that CONFIG_FAULT_INJECTION_DEBUG_FS kernel config option not enabled"
 }
 
 # Disable extent zeroing for ext4 on the given device
-- 
2.23.0


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

* [PATCH 2/3] fstests: btrfs/14[23]: Use proper help to get both devid and physical offset for corruption.
  2019-12-11 10:40 [PATCH 0/3] fstests: btrfs/15[78] btrfs/14[23]: Use more accurate devid/phsyical for corruption Qu Wenruo
  2019-12-11 10:40 ` [PATCH 1/3] fstests: common: Use more accurate kernel config for _require_fail_make_request Qu Wenruo
@ 2019-12-11 10:40 ` Qu Wenruo
  2019-12-11 11:24   ` Nikolay Borisov
  2019-12-17 17:30   ` Filipe Manana
  2019-12-11 10:40 ` [PATCH 3/3] fstests: btrfs/15[78]: Use proper helper " Qu Wenruo
  2 siblings, 2 replies; 9+ messages in thread
From: Qu Wenruo @ 2019-12-11 10:40 UTC (permalink / raw)
  To: fstests, linux-btrfs; +Cc: Filipe Manana

[BUG]
When using btrfs-progs v5.4, btrfs/142 and btrfs/143 will fail:
btrfs/142 1s ... - output mismatch (see xfstests/results//btrfs/142.out.bad)
    --- tests/btrfs/142.out 2018-09-16 21:30:48.505104287 +0100
    +++ xfstests/results//btrfs/142.out.bad
2019-12-10 15:35:40.280392626 +0000
    @@ -3,37 +3,37 @@
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
     wrote 65536/65536 bytes
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
    -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
    -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
    -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
    ...
    (Run 'diff -u xfstests/tests/btrfs/142.out xfstests/results//btrfs/142.out.bad' to see the entire diff)

[CAUSE]
Btrfs/14[23] test whether a read on corrupted stripe will re-silver
itself.
Such test by its nature will need to modify on-disk data, thus need to
get the btrfs logical -> physical mapping, which is done by near
hard-coded lookup function, which rely on certain stripe:devid sequence.

Recent btrfs-progs commit c501c9e3b816 ("btrfs-progs: mkfs: match devid
order to the stripe index") changes how we use devices in mkfs.btrfs,
this caused a change in chunk layout, and break the hard-coded
stripe:devid sequence.

[FIX]
This patch will do full devid and physical offset lookup, instead of old
physical offset only lookup.

The only assumption made is, mkfs.btrfs assigns devid sequentially for
its devices.
Which means, for "mkfs.btrfs $dev1 $dev2 $dev3", we get devid 1 for $dev1,
devid 2 for $dev2, and so on.

This change will allow btrfs/14[23] to handle even future chunk layout
change. (Although I hope this will never happen again).

This also addes extra debug output (although less than 10 lines) into
$seqres.full, just in case when layout changes and current lookup can't
handle it, developer can still pindown the problem easily.

Reported-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/142     | 50 +++++++++++++++++++++++++++++++--------------
 tests/btrfs/142.out |  2 --
 tests/btrfs/143     | 48 +++++++++++++++++++++++++++++++------------
 tests/btrfs/143.out |  2 --
 4 files changed, 70 insertions(+), 32 deletions(-)

diff --git a/tests/btrfs/142 b/tests/btrfs/142
index a23fe1bf..9c037ff6 100755
--- a/tests/btrfs/142
+++ b/tests/btrfs/142
@@ -47,30 +47,45 @@ _require_command "$FILEFRAG_PROG" filefrag
 
 get_physical()
 {
-        # $1 is logical address
-        # print chunk tree and find devid 2 which is $SCRATCH_DEV
+	local logical=$1
+	local stripe=$2
         $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
-	grep $1 -A 6 | awk '($1 ~ /stripe/ && $3 ~ /devid/ && $4 ~ /1/) { print $6 }'
+		grep $logical -A 6 | \
+		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"
 }
 
+get_devid()
+{
+	local logical=$1
+	local stripe=$2
+        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
+		grep $logical -A 6 | \
+		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"
+}
 
-SYSFS_BDEV=`_sysfs_dev $SCRATCH_DEV`
+get_device_path()
+{
+	local devid=$1
+	echo "$SCRATCH_DEV_POOL" | awk "{print \$$devid}"
+}
 
 start_fail()
 {
+	local sysfs_bdev="$1"
 	echo 100 > $DEBUGFS_MNT/fail_make_request/probability
 	echo 2 > $DEBUGFS_MNT/fail_make_request/times
 	echo 1 > $DEBUGFS_MNT/fail_make_request/task-filter
 	echo 0 > $DEBUGFS_MNT/fail_make_request/verbose
-	echo 1 > $SYSFS_BDEV/make-it-fail
+	echo 1 > $sysfs_bdev/make-it-fail
 }
 
 stop_fail()
 {
+	local sysfs_bdev="$1"
 	echo 0 > $DEBUGFS_MNT/fail_make_request/probability
 	echo 0 > $DEBUGFS_MNT/fail_make_request/times
 	echo 0 > $DEBUGFS_MNT/fail_make_request/task-filter
-	echo 0 > $SYSFS_BDEV/make-it-fail
+	echo 0 > $sysfs_bdev/make-it-fail
 }
 
 _scratch_dev_pool_get 2
@@ -87,17 +102,23 @@ _scratch_mount -o nospace_cache,nodatasum
 $XFS_IO_PROG -f -d -c "pwrite -S 0xaa -b 128K 0 128K" "$SCRATCH_MNT/foobar" |\
 	_filter_xfs_io_offset
 
-# step 2, corrupt the first 64k of one copy (on SCRATCH_DEV which is the first
-# one in $SCRATCH_DEV_POOL
+# step 2, corrupt the first 64k of stripe #1
 echo "step 2......corrupt file extent" >>$seqres.full
 
 ${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar >> $seqres.full
 logical_in_btrfs=`${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar | _filter_filefrag | cut -d '#' -f 1`
-physical_on_scratch=`get_physical ${logical_in_btrfs}`
+physical=`get_physical ${logical_in_btrfs} 1`
+devid=$(get_devid ${logical_in_btrfs} 1)
+target_dev=$(get_device_path $devid)
+
+SYSFS_BDEV=`_sysfs_dev $target_dev`
 
 _scratch_unmount
-$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical_on_scratch 64K" $SCRATCH_DEV |\
-	_filter_xfs_io_offset
+$BTRFS_UTIL_PROG ins dump-tree -t 3 $SCRATCH_DEV | \
+	grep $logical_in_btrfs -A 6 >> $seqres.full
+echo "Corrupt stripe 1 devid $devid devpath $target_dev physical $physical" \
+	>> $seqres.full
+$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical 64K" $target_dev > /dev/null
 
 _scratch_mount -o nospace_cache
 
@@ -106,8 +127,7 @@ echo "step 3......repair the bad copy" >>$seqres.full
 
 # since raid1 consists of two copies, and the bad copy was put on stripe #1
 # while the good copy lies on stripe #0, the bad copy only gets access when the
-# reader's pid % 2 == 1 is true
-start_fail
+start_fail $SYSFS_BDEV
 while [[ -z ${result} ]]; do
 	# enable task-filter only fails the following dio read so the repair is
 	# supposed to work.
@@ -117,12 +137,12 @@ while [[ -z ${result} ]]; do
 		exec $XFS_IO_PROG -d -c \"pread -b 128K 0 128K\" \"$SCRATCH_MNT/foobar\"
 	fi");
 done
-stop_fail
+stop_fail $SYSFS_BDEV
 
 _scratch_unmount
 
 # check if the repair works
-$XFS_IO_PROG -c "pread -v -b 512 $physical_on_scratch 512" $SCRATCH_DEV |\
+$XFS_IO_PROG -c "pread -v -b 512 $physical 512" $target_dev | \
 	_filter_xfs_io_offset
 
 _scratch_dev_pool_put
diff --git a/tests/btrfs/142.out b/tests/btrfs/142.out
index 0f32ffbe..2e22f292 100644
--- a/tests/btrfs/142.out
+++ b/tests/btrfs/142.out
@@ -1,8 +1,6 @@
 QA output created by 142
 wrote 131072/131072 bytes
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes
-XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
 XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
 XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
diff --git a/tests/btrfs/143 b/tests/btrfs/143
index 91af52f9..b2ffeb63 100755
--- a/tests/btrfs/143
+++ b/tests/btrfs/143
@@ -54,30 +54,48 @@ _require_command "$FILEFRAG_PROG" filefrag
 
 get_physical()
 {
-        # $1 is logical address
-        # print chunk tree and find devid 2 which is $SCRATCH_DEV
+	local logical=$1
+	local stripe=$2
         $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
-	grep $1 -A 6 | awk '($1 ~ /stripe/ && $3 ~ /devid/ && $4 ~ /1/) { print $6 }'
+		grep $logical -A 6 | \
+		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"
+}
+
+get_devid()
+{
+	local logical=$1
+	local stripe=$2
+        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
+		grep $logical -A 6 | \
+		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"
+}
+
+get_device_path()
+{
+	local devid=$1
+	echo "$SCRATCH_DEV_POOL" | awk "{print \$$devid}"
 }
 
 SYSFS_BDEV=`_sysfs_dev $SCRATCH_DEV`
 
 start_fail()
 {
+	local sysfs_bdev="$1"
 	echo 100 > $DEBUGFS_MNT/fail_make_request/probability
 	# the 1st one fails the first bio which is reading 4k (or more due to
 	# readahead), and the 2nd one fails the retry of validation so that it
 	# triggers read-repair
 	echo 2 > $DEBUGFS_MNT/fail_make_request/times
 	echo 0 > $DEBUGFS_MNT/fail_make_request/verbose
-	echo 1 > $SYSFS_BDEV/make-it-fail
+	echo 1 > $sysfs_bdev/make-it-fail
 }
 
 stop_fail()
 {
+	local sysfs_bdev="$1"
 	echo 0 > $DEBUGFS_MNT/fail_make_request/probability
 	echo 0 > $DEBUGFS_MNT/fail_make_request/times
-	echo 0 > $SYSFS_BDEV/make-it-fail
+	echo 0 > $sysfs_bdev/make-it-fail
 }
 
 _scratch_dev_pool_get 2
@@ -94,17 +112,21 @@ _scratch_mount -o nospace_cache,nodatasum
 $XFS_IO_PROG -f -d -c "pwrite -S 0xaa -b 128K 0 128K" "$SCRATCH_MNT/foobar" |\
 	_filter_xfs_io_offset
 
-# step 2, corrupt the first 64k of one copy (on SCRATCH_DEV which is the first
-# one in $SCRATCH_DEV_POOL
+# step 2, corrupt the first 64k of stripe #1
 echo "step 2......corrupt file extent" >>$seqres.full
 
 ${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar >> $seqres.full
 logical_in_btrfs=`${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar | _filter_filefrag | cut -d '#' -f 1`
-physical_on_scratch=`get_physical ${logical_in_btrfs}`
+physical=`get_physical ${logical_in_btrfs} 1`
+devid=$(get_devid ${logical_in_btrfs} 1)
+target_dev=$(get_device_path $devid)
 
+SYSFS_BDEV=`_sysfs_dev $target_dev`
 _scratch_unmount
-$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical_on_scratch 64K" $SCRATCH_DEV |\
-	_filter_xfs_io_offset
+
+echo "corrupt stripe 1 devid $devid devpath $target_dev physical $physical" \
+	>> $seqres.full
+$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical 64K" $target_dev > /dev/null
 
 _scratch_mount -o nospace_cache
 
@@ -118,18 +140,18 @@ while [[ -z ${result} ]]; do
     # invalidate the page cache.
     _scratch_cycle_mount
 
-    start_fail
+    start_fail $SYSFS_BDEV
     result=$(bash -c "
         if [[ \$((\$\$ % 2)) -eq 1 ]]; then
                 exec $XFS_IO_PROG -c \"pread 0 4K\" \"$SCRATCH_MNT/foobar\"
         fi");
-    stop_fail
+    stop_fail $SYSFS_BDEV
 done
 
 _scratch_unmount
 
 # check if the repair works
-$XFS_IO_PROG -c "pread -v -b 512 $physical_on_scratch 512" $SCRATCH_DEV |\
+$XFS_IO_PROG -c "pread -v -b 512 $physical 512" $target_dev |\
 	_filter_xfs_io_offset
 
 _scratch_dev_pool_put
diff --git a/tests/btrfs/143.out b/tests/btrfs/143.out
index 66afea4b..a9e82ceb 100644
--- a/tests/btrfs/143.out
+++ b/tests/btrfs/143.out
@@ -1,8 +1,6 @@
 QA output created by 143
 wrote 131072/131072 bytes
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes
-XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
 XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
 XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
-- 
2.23.0


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

* [PATCH 3/3] fstests: btrfs/15[78]: Use proper helper to get both devid and physical offset for corruption
  2019-12-11 10:40 [PATCH 0/3] fstests: btrfs/15[78] btrfs/14[23]: Use more accurate devid/phsyical for corruption Qu Wenruo
  2019-12-11 10:40 ` [PATCH 1/3] fstests: common: Use more accurate kernel config for _require_fail_make_request Qu Wenruo
  2019-12-11 10:40 ` [PATCH 2/3] fstests: btrfs/14[23]: Use proper help to get both devid and physical offset for corruption Qu Wenruo
@ 2019-12-11 10:40 ` Qu Wenruo
  2019-12-11 11:24   ` Nikolay Borisov
  2019-12-17 17:33   ` Filipe Manana
  2 siblings, 2 replies; 9+ messages in thread
From: Qu Wenruo @ 2019-12-11 10:40 UTC (permalink / raw)
  To: fstests, linux-btrfs; +Cc: Filipe Manana

[BUG]
When using btrfs-progs v5.4, btrfs/157 and btrfs/158 will fail:

btrfs/157 1s ... - output mismatch (see xfstests/results//btrfs/157.out.bad)
    --- tests/btrfs/157.out 2018-09-16 21:30:48.505104287 +0100
    +++ xfstests/results//btrfs/157.out.bad
2019-12-10 15:35:43.112390076 +0000
    @@ -1,9 +1,9 @@
     QA output created by 157
     wrote 131072/131072 bytes at offset 0
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -wrote 65536/65536 bytes at offset 9437184
    +wrote 65536/65536 bytes at offset 22020096
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -wrote 65536/65536 bytes at offset 9437184
    ...
    (Run 'diff -u xfstests/tests/btrfs/157.out xfstests/results//btrfs/157.out.bad'  to see the entire diff)
btrfs/158 2s ... - output mismatch (see xfstests/results//btrfs/158.out.bad)
    --- tests/btrfs/158.out 2018-09-16 21:30:48.505104287 +0100
    +++ xfstests/results//btrfs/158.out.bad
2019-12-10 15:35:44.844388521 +0000
    @@ -1,9 +1,9 @@
     QA output created by 158
     wrote 131072/131072 bytes at offset 0
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -wrote 65536/65536 bytes at offset 9437184
    +wrote 65536/65536 bytes at offset 22020096
     XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
    -wrote 65536/65536 bytes at offset 9437184
    ...
    (Run 'diff -u xfstests/tests/btrfs/158.out xfstests/results//btrfs/158.out.bad'  to see the entire diff)

[CAUSE]
This two tests use physical offset as golden output, while mkfs.btrfs
can do whatever it likes to arrange its chunk layout, thus physical
offset is never reliable.

And btrfs-progs commit c501c9e3b816 ("btrfs-progs: mkfs: match devid
order to the stripe index") just changed the layout.

So the output mismatch and failed.

[FIX]
In fact, that btrfs-progs commit not only changed offset, but also the
device sequence.

So we can't just simply remove the physical offset, but also need to use
proper helper to get both devid (as its device path) and physical offset
for corruption.

As long as mkfs.btrfs still uses sequential devid, these tests should
handle future chunk layout change without problem.

Reported-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/157     | 53 +++++++++++++++++++++++++++++----------------
 tests/btrfs/157.out |  4 ----
 tests/btrfs/158     | 48 +++++++++++++++++++++++++---------------
 tests/btrfs/158.out |  4 ----
 4 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/tests/btrfs/157 b/tests/btrfs/157
index 7f75c407..80b01b8d 100755
--- a/tests/btrfs/157
+++ b/tests/btrfs/157
@@ -51,22 +51,30 @@ _require_scratch_dev_pool 4
 _require_btrfs_command inspect-internal dump-tree
 _require_btrfs_fs_feature raid56
 
-get_physical_stripe0()
+get_physical()
 {
-	$BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
-	grep " DATA\|RAID6" -A 10 | \
-	$AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /0/) { print $6 }'
+	local stripe=$1
+        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
+		grep " DATA\|RAID6" -A 10 | \
+		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"
 }
 
-get_physical_stripe1()
+get_devid()
 {
-	$BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
-	grep " DATA\|RAID6" -A 10 | \
-	$AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /1/) { print $6 }'
+	local stripe=$1
+        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
+		grep " DATA\|RAID6" -A 10 | \
+		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"
+}
+
+get_device_path()
+{
+	local devid=$1
+	echo "$SCRATCH_DEV_POOL" | awk "{print \$$devid}"
 }
 
 _scratch_dev_pool_get 4
-# step 1: create a raid6 btrfs and create a 4K file
+# step 1: create a raid6 btrfs and create a 128K file
 echo "step 1......mkfs.btrfs" >>$seqres.full
 
 mkfs_opts="-d raid6 -b 1G"
@@ -80,18 +88,25 @@ _scratch_mount -o nospace_cache
 $XFS_IO_PROG -f -d -c "pwrite -S 0xaa 0 128K" -c "fsync" \
 	"$SCRATCH_MNT/foobar" | _filter_xfs_io
 
+logical=`${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar | _filter_filefrag | cut -d '#' -f 1`
 _scratch_unmount
 
-stripe_0=`get_physical_stripe0`
-stripe_1=`get_physical_stripe1`
-dev4=`echo $SCRATCH_DEV_POOL | awk '{print $4}'`
-dev3=`echo $SCRATCH_DEV_POOL | awk '{print $3}'`
-
-# step 2: corrupt the 1st and 2nd stripe (stripe 0 and 1)
-echo "step 2......simulate bitrot at offset $stripe_0 of device_4($dev4) and offset $stripe_1 of device_3($dev3)" >>$seqres.full
-
-$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_0 64K" $dev4 | _filter_xfs_io
-$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_1 64K" $dev3 | _filter_xfs_io
+phy0=$(get_physical 0)
+devid0=$(get_devid 0)
+devpath0=$(get_device_path $devid0)
+phy1=$(get_physical 1)
+devid1=$(get_devid 1)
+devpath1=$(get_device_path $devid1)
+
+# step 2: corrupt stripe #0 and #1
+echo "step 2......simulate bitrot at:" >>$seqres.full
+echo "      ......stripe #0: devid $devid0 devpath $devpath0 phy $phy0" \
+	>>$seqres.full
+echo "      ......stripe #1: devid $devid1 devpath $devpath1 phy $phy1" \
+	>>$seqres.full
+
+$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy0 64K" $devpath0 > /dev/null
+$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy1 64K" $devpath1 > /dev/null
 
 # step 3: read foobar to repair the bitrot
 echo "step 3......repair the bitrot" >> $seqres.full
diff --git a/tests/btrfs/157.out b/tests/btrfs/157.out
index 08d592c4..d69c0f1d 100644
--- a/tests/btrfs/157.out
+++ b/tests/btrfs/157.out
@@ -1,10 +1,6 @@
 QA output created by 157
 wrote 131072/131072 bytes at offset 0
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 9437184
-XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 9437184
-XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 0200000 aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa
 *
 0400000
diff --git a/tests/btrfs/158 b/tests/btrfs/158
index 603e8bea..c8d5af82 100755
--- a/tests/btrfs/158
+++ b/tests/btrfs/158
@@ -43,22 +43,30 @@ _require_scratch_dev_pool 4
 _require_btrfs_command inspect-internal dump-tree
 _require_btrfs_fs_feature raid56
 
-get_physical_stripe0()
+get_physical()
 {
-	$BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
-	grep " DATA\|RAID6" -A 10 | \
-	$AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /0/) { print $6 }'
+	local stripe=$1
+        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
+		grep " DATA\|RAID6" -A 10 | \
+		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"
 }
 
-get_physical_stripe1()
+get_devid()
 {
-	$BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
-	grep " DATA\|RAID6" -A 10 | \
-	$AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /1/) { print $6 }'
+	local stripe=$1
+        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
+		grep " DATA\|RAID6" -A 10 | \
+		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"
+}
+
+get_device_path()
+{
+	local devid=$1
+	echo "$SCRATCH_DEV_POOL" | awk "{print \$$devid}"
 }
 
 _scratch_dev_pool_get 4
-# step 1: create a raid6 btrfs and create a 4K file
+# step 1: create a raid6 btrfs and create a 128K file
 echo "step 1......mkfs.btrfs" >>$seqres.full
 
 mkfs_opts="-d raid6 -b 1G"
@@ -74,16 +82,22 @@ $XFS_IO_PROG -f -d -c "pwrite -S 0xaa 0 128K" -c "fsync" \
 
 _scratch_unmount
 
-stripe_0=`get_physical_stripe0`
-stripe_1=`get_physical_stripe1`
-dev4=`echo $SCRATCH_DEV_POOL | awk '{print $4}'`
-dev3=`echo $SCRATCH_DEV_POOL | awk '{print $3}'`
+phy0=$(get_physical 0)
+devid0=$(get_devid 0)
+devpath0=$(get_device_path $devid0)
+phy1=$(get_physical 1)
+devid1=$(get_devid 1)
+devpath1=$(get_device_path $devid1)
 
 # step 2: corrupt the 1st and 2nd stripe (stripe 0 and 1)
-echo "step 2......simulate bitrot at offset $stripe_0 of device_4($dev4) and offset $stripe_1 of device_3($dev3)" >>$seqres.full
-
-$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_0 64K" $dev4 | _filter_xfs_io
-$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_1 64K" $dev3 | _filter_xfs_io
+echo "step 2......simulate bitrot at:" >>$seqres.full
+echo "      ......stripe #0: devid $devid0 devpath $devpath0 phy $phy0" \
+	>>$seqres.full
+echo "      ......stripe #1: devid $devid1 devpath $devpath1 phy $phy1" \
+	>>$seqres.full
+
+$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy0 64K" $devpath0 > /dev/null
+$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy1 64K" $devpath1 > /dev/null
 
 # step 3: scrub filesystem to repair the bitrot
 echo "step 3......repair the bitrot" >> $seqres.full
diff --git a/tests/btrfs/158.out b/tests/btrfs/158.out
index 1f5ad3f7..95562f49 100644
--- a/tests/btrfs/158.out
+++ b/tests/btrfs/158.out
@@ -1,10 +1,6 @@
 QA output created by 158
 wrote 131072/131072 bytes at offset 0
 XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 9437184
-XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 9437184
-XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 0000000 aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa
 *
 0400000
-- 
2.23.0


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

* Re: [PATCH 2/3] fstests: btrfs/14[23]: Use proper help to get both devid and physical offset for corruption.
  2019-12-11 10:40 ` [PATCH 2/3] fstests: btrfs/14[23]: Use proper help to get both devid and physical offset for corruption Qu Wenruo
@ 2019-12-11 11:24   ` Nikolay Borisov
  2019-12-17 17:30   ` Filipe Manana
  1 sibling, 0 replies; 9+ messages in thread
From: Nikolay Borisov @ 2019-12-11 11:24 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs; +Cc: Filipe Manana



On 11.12.19 г. 12:40 ч., Qu Wenruo wrote:
> [BUG]
> When using btrfs-progs v5.4, btrfs/142 and btrfs/143 will fail:
> btrfs/142 1s ... - output mismatch (see xfstests/results//btrfs/142.out.bad)
>     --- tests/btrfs/142.out 2018-09-16 21:30:48.505104287 +0100
>     +++ xfstests/results//btrfs/142.out.bad
> 2019-12-10 15:35:40.280392626 +0000
>     @@ -3,37 +3,37 @@
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>      wrote 65536/65536 bytes
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
>     -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
>     -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
>     -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
>     ...
>     (Run 'diff -u xfstests/tests/btrfs/142.out xfstests/results//btrfs/142.out.bad' to see the entire diff)
> 
> [CAUSE]
> Btrfs/14[23] test whether a read on corrupted stripe will re-silver
> itself.
> Such test by its nature will need to modify on-disk data, thus need to
> get the btrfs logical -> physical mapping, which is done by near
> hard-coded lookup function, which rely on certain stripe:devid sequence.
> 
> Recent btrfs-progs commit c501c9e3b816 ("btrfs-progs: mkfs: match devid
> order to the stripe index") changes how we use devices in mkfs.btrfs,
> this caused a change in chunk layout, and break the hard-coded
> stripe:devid sequence.
> 
> [FIX]
> This patch will do full devid and physical offset lookup, instead of old
> physical offset only lookup.
> 
> The only assumption made is, mkfs.btrfs assigns devid sequentially for
> its devices.
> Which means, for "mkfs.btrfs $dev1 $dev2 $dev3", we get devid 1 for $dev1,
> devid 2 for $dev2, and so on.
> 
> This change will allow btrfs/14[23] to handle even future chunk layout
> change. (Although I hope this will never happen again).
> 
> This also addes extra debug output (although less than 10 lines) into
> $seqres.full, just in case when layout changes and current lookup can't
> handle it, developer can still pindown the problem easily.
> 
> Reported-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Tested-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  tests/btrfs/142     | 50 +++++++++++++++++++++++++++++++--------------
>  tests/btrfs/142.out |  2 --
>  tests/btrfs/143     | 48 +++++++++++++++++++++++++++++++------------
>  tests/btrfs/143.out |  2 --
>  4 files changed, 70 insertions(+), 32 deletions(-)
> 
> diff --git a/tests/btrfs/142 b/tests/btrfs/142
> index a23fe1bf..9c037ff6 100755
> --- a/tests/btrfs/142
> +++ b/tests/btrfs/142
> @@ -47,30 +47,45 @@ _require_command "$FILEFRAG_PROG" filefrag
>  
>  get_physical()
>  {
> -        # $1 is logical address
> -        # print chunk tree and find devid 2 which is $SCRATCH_DEV
> +	local logical=$1
> +	local stripe=$2
>          $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> -	grep $1 -A 6 | awk '($1 ~ /stripe/ && $3 ~ /devid/ && $4 ~ /1/) { print $6 }'
> +		grep $logical -A 6 | \
> +		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"
>  }
>  
> +get_devid()
> +{
> +	local logical=$1
> +	local stripe=$2
> +        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> +		grep $logical -A 6 | \
> +		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"
> +}
>  
> -SYSFS_BDEV=`_sysfs_dev $SCRATCH_DEV`
> +get_device_path()
> +{
> +	local devid=$1
> +	echo "$SCRATCH_DEV_POOL" | awk "{print \$$devid}"
> +}
>  
>  start_fail()
>  {
> +	local sysfs_bdev="$1"
>  	echo 100 > $DEBUGFS_MNT/fail_make_request/probability
>  	echo 2 > $DEBUGFS_MNT/fail_make_request/times
>  	echo 1 > $DEBUGFS_MNT/fail_make_request/task-filter
>  	echo 0 > $DEBUGFS_MNT/fail_make_request/verbose
> -	echo 1 > $SYSFS_BDEV/make-it-fail
> +	echo 1 > $sysfs_bdev/make-it-fail
>  }
>  
>  stop_fail()
>  {
> +	local sysfs_bdev="$1"
>  	echo 0 > $DEBUGFS_MNT/fail_make_request/probability
>  	echo 0 > $DEBUGFS_MNT/fail_make_request/times
>  	echo 0 > $DEBUGFS_MNT/fail_make_request/task-filter
> -	echo 0 > $SYSFS_BDEV/make-it-fail
> +	echo 0 > $sysfs_bdev/make-it-fail
>  }
>  
>  _scratch_dev_pool_get 2
> @@ -87,17 +102,23 @@ _scratch_mount -o nospace_cache,nodatasum
>  $XFS_IO_PROG -f -d -c "pwrite -S 0xaa -b 128K 0 128K" "$SCRATCH_MNT/foobar" |\
>  	_filter_xfs_io_offset
>  
> -# step 2, corrupt the first 64k of one copy (on SCRATCH_DEV which is the first
> -# one in $SCRATCH_DEV_POOL
> +# step 2, corrupt the first 64k of stripe #1
>  echo "step 2......corrupt file extent" >>$seqres.full
>  
>  ${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar >> $seqres.full
>  logical_in_btrfs=`${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar | _filter_filefrag | cut -d '#' -f 1`
> -physical_on_scratch=`get_physical ${logical_in_btrfs}`
> +physical=`get_physical ${logical_in_btrfs} 1`
> +devid=$(get_devid ${logical_in_btrfs} 1)
> +target_dev=$(get_device_path $devid)
> +
> +SYSFS_BDEV=`_sysfs_dev $target_dev`
>  
>  _scratch_unmount
> -$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical_on_scratch 64K" $SCRATCH_DEV |\
> -	_filter_xfs_io_offset
> +$BTRFS_UTIL_PROG ins dump-tree -t 3 $SCRATCH_DEV | \
> +	grep $logical_in_btrfs -A 6 >> $seqres.full
> +echo "Corrupt stripe 1 devid $devid devpath $target_dev physical $physical" \
> +	>> $seqres.full
> +$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical 64K" $target_dev > /dev/null
>  
>  _scratch_mount -o nospace_cache
>  
> @@ -106,8 +127,7 @@ echo "step 3......repair the bad copy" >>$seqres.full
>  
>  # since raid1 consists of two copies, and the bad copy was put on stripe #1
>  # while the good copy lies on stripe #0, the bad copy only gets access when the
> -# reader's pid % 2 == 1 is true
> -start_fail
> +start_fail $SYSFS_BDEV
>  while [[ -z ${result} ]]; do
>  	# enable task-filter only fails the following dio read so the repair is
>  	# supposed to work.
> @@ -117,12 +137,12 @@ while [[ -z ${result} ]]; do
>  		exec $XFS_IO_PROG -d -c \"pread -b 128K 0 128K\" \"$SCRATCH_MNT/foobar\"
>  	fi");
>  done
> -stop_fail
> +stop_fail $SYSFS_BDEV
>  
>  _scratch_unmount
>  
>  # check if the repair works
> -$XFS_IO_PROG -c "pread -v -b 512 $physical_on_scratch 512" $SCRATCH_DEV |\
> +$XFS_IO_PROG -c "pread -v -b 512 $physical 512" $target_dev | \
>  	_filter_xfs_io_offset
>  
>  _scratch_dev_pool_put
> diff --git a/tests/btrfs/142.out b/tests/btrfs/142.out
> index 0f32ffbe..2e22f292 100644
> --- a/tests/btrfs/142.out
> +++ b/tests/btrfs/142.out
> @@ -1,8 +1,6 @@
>  QA output created by 142
>  wrote 131072/131072 bytes
>  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
> diff --git a/tests/btrfs/143 b/tests/btrfs/143
> index 91af52f9..b2ffeb63 100755
> --- a/tests/btrfs/143
> +++ b/tests/btrfs/143
> @@ -54,30 +54,48 @@ _require_command "$FILEFRAG_PROG" filefrag
>  
>  get_physical()
>  {
> -        # $1 is logical address
> -        # print chunk tree and find devid 2 which is $SCRATCH_DEV
> +	local logical=$1
> +	local stripe=$2
>          $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> -	grep $1 -A 6 | awk '($1 ~ /stripe/ && $3 ~ /devid/ && $4 ~ /1/) { print $6 }'
> +		grep $logical -A 6 | \
> +		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"
> +}
> +
> +get_devid()
> +{
> +	local logical=$1
> +	local stripe=$2
> +        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> +		grep $logical -A 6 | \
> +		awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"
> +}
> +
> +get_device_path()
> +{
> +	local devid=$1
> +	echo "$SCRATCH_DEV_POOL" | awk "{print \$$devid}"
>  }
>  
>  SYSFS_BDEV=`_sysfs_dev $SCRATCH_DEV`
>  
>  start_fail()
>  {
> +	local sysfs_bdev="$1"
>  	echo 100 > $DEBUGFS_MNT/fail_make_request/probability
>  	# the 1st one fails the first bio which is reading 4k (or more due to
>  	# readahead), and the 2nd one fails the retry of validation so that it
>  	# triggers read-repair
>  	echo 2 > $DEBUGFS_MNT/fail_make_request/times
>  	echo 0 > $DEBUGFS_MNT/fail_make_request/verbose
> -	echo 1 > $SYSFS_BDEV/make-it-fail
> +	echo 1 > $sysfs_bdev/make-it-fail
>  }
>  
>  stop_fail()
>  {
> +	local sysfs_bdev="$1"
>  	echo 0 > $DEBUGFS_MNT/fail_make_request/probability
>  	echo 0 > $DEBUGFS_MNT/fail_make_request/times
> -	echo 0 > $SYSFS_BDEV/make-it-fail
> +	echo 0 > $sysfs_bdev/make-it-fail
>  }
>  
>  _scratch_dev_pool_get 2
> @@ -94,17 +112,21 @@ _scratch_mount -o nospace_cache,nodatasum
>  $XFS_IO_PROG -f -d -c "pwrite -S 0xaa -b 128K 0 128K" "$SCRATCH_MNT/foobar" |\
>  	_filter_xfs_io_offset
>  
> -# step 2, corrupt the first 64k of one copy (on SCRATCH_DEV which is the first
> -# one in $SCRATCH_DEV_POOL
> +# step 2, corrupt the first 64k of stripe #1
>  echo "step 2......corrupt file extent" >>$seqres.full
>  
>  ${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar >> $seqres.full
>  logical_in_btrfs=`${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar | _filter_filefrag | cut -d '#' -f 1`
> -physical_on_scratch=`get_physical ${logical_in_btrfs}`
> +physical=`get_physical ${logical_in_btrfs} 1`
> +devid=$(get_devid ${logical_in_btrfs} 1)
> +target_dev=$(get_device_path $devid)
>  
> +SYSFS_BDEV=`_sysfs_dev $target_dev`
>  _scratch_unmount
> -$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical_on_scratch 64K" $SCRATCH_DEV |\
> -	_filter_xfs_io_offset
> +
> +echo "corrupt stripe 1 devid $devid devpath $target_dev physical $physical" \
> +	>> $seqres.full
> +$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical 64K" $target_dev > /dev/null
>  
>  _scratch_mount -o nospace_cache
>  
> @@ -118,18 +140,18 @@ while [[ -z ${result} ]]; do
>      # invalidate the page cache.
>      _scratch_cycle_mount
>  
> -    start_fail
> +    start_fail $SYSFS_BDEV
>      result=$(bash -c "
>          if [[ \$((\$\$ % 2)) -eq 1 ]]; then
>                  exec $XFS_IO_PROG -c \"pread 0 4K\" \"$SCRATCH_MNT/foobar\"
>          fi");
> -    stop_fail
> +    stop_fail $SYSFS_BDEV
>  done
>  
>  _scratch_unmount
>  
>  # check if the repair works
> -$XFS_IO_PROG -c "pread -v -b 512 $physical_on_scratch 512" $SCRATCH_DEV |\
> +$XFS_IO_PROG -c "pread -v -b 512 $physical 512" $target_dev |\
>  	_filter_xfs_io_offset
>  
>  _scratch_dev_pool_put
> diff --git a/tests/btrfs/143.out b/tests/btrfs/143.out
> index 66afea4b..a9e82ceb 100644
> --- a/tests/btrfs/143.out
> +++ b/tests/btrfs/143.out
> @@ -1,8 +1,6 @@
>  QA output created by 143
>  wrote 131072/131072 bytes
>  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
> 

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

* Re: [PATCH 3/3] fstests: btrfs/15[78]: Use proper helper to get both devid and physical offset for corruption
  2019-12-11 10:40 ` [PATCH 3/3] fstests: btrfs/15[78]: Use proper helper " Qu Wenruo
@ 2019-12-11 11:24   ` Nikolay Borisov
  2019-12-17 17:33   ` Filipe Manana
  1 sibling, 0 replies; 9+ messages in thread
From: Nikolay Borisov @ 2019-12-11 11:24 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs; +Cc: Filipe Manana



On 11.12.19 г. 12:40 ч., Qu Wenruo wrote:
> [BUG]
> When using btrfs-progs v5.4, btrfs/157 and btrfs/158 will fail:
> 
> btrfs/157 1s ... - output mismatch (see xfstests/results//btrfs/157.out.bad)
>     --- tests/btrfs/157.out 2018-09-16 21:30:48.505104287 +0100
>     +++ xfstests/results//btrfs/157.out.bad
> 2019-12-10 15:35:43.112390076 +0000
>     @@ -1,9 +1,9 @@
>      QA output created by 157
>      wrote 131072/131072 bytes at offset 0
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 9437184
>     +wrote 65536/65536 bytes at offset 22020096
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 9437184
>     ...
>     (Run 'diff -u xfstests/tests/btrfs/157.out xfstests/results//btrfs/157.out.bad'  to see the entire diff)
> btrfs/158 2s ... - output mismatch (see xfstests/results//btrfs/158.out.bad)
>     --- tests/btrfs/158.out 2018-09-16 21:30:48.505104287 +0100
>     +++ xfstests/results//btrfs/158.out.bad
> 2019-12-10 15:35:44.844388521 +0000
>     @@ -1,9 +1,9 @@
>      QA output created by 158
>      wrote 131072/131072 bytes at offset 0
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 9437184
>     +wrote 65536/65536 bytes at offset 22020096
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 9437184
>     ...
>     (Run 'diff -u xfstests/tests/btrfs/158.out xfstests/results//btrfs/158.out.bad'  to see the entire diff)
> 
> [CAUSE]
> This two tests use physical offset as golden output, while mkfs.btrfs
> can do whatever it likes to arrange its chunk layout, thus physical
> offset is never reliable.
> 
> And btrfs-progs commit c501c9e3b816 ("btrfs-progs: mkfs: match devid
> order to the stripe index") just changed the layout.
> 
> So the output mismatch and failed.
> 
> [FIX]
> In fact, that btrfs-progs commit not only changed offset, but also the
> device sequence.
> 
> So we can't just simply remove the physical offset, but also need to use
> proper helper to get both devid (as its device path) and physical offset
> for corruption.
> 
> As long as mkfs.btrfs still uses sequential devid, these tests should
> handle future chunk layout change without problem.
> 
> Reported-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Tested-by: Nikolay Borisov <nborisov@suse.com>

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

* Re: [PATCH 1/3] fstests: common: Use more accurate kernel config for _require_fail_make_request
  2019-12-11 10:40 ` [PATCH 1/3] fstests: common: Use more accurate kernel config for _require_fail_make_request Qu Wenruo
@ 2019-12-17 17:24   ` Filipe Manana
  0 siblings, 0 replies; 9+ messages in thread
From: Filipe Manana @ 2019-12-17 17:24 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: fstests, linux-btrfs

On Wed, Dec 11, 2019 at 10:40 AM Qu Wenruo <wqu@suse.com> wrote:
>
> Just enabling CONFIG_FAIL_MAKE_REQUEST will not fulfill
> _require_fail_make_request.
>
> It's CONFIG_FAULT_INJECTION_DEBUG_FS.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

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

Thanks.

> ---
>  common/rc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/rc b/common/rc
> index 5cdd829b..2d72f158 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2357,7 +2357,7 @@ _require_fail_make_request()
>  {
>      [ -f "$DEBUGFS_MNT/fail_make_request/probability" ] \
>         || _notrun "$DEBUGFS_MNT/fail_make_request \
> - not found. Seems that CONFIG_FAIL_MAKE_REQUEST kernel config option not enabled"
> + not found. Seems that CONFIG_FAULT_INJECTION_DEBUG_FS kernel config option not enabled"
>  }
>
>  # Disable extent zeroing for ext4 on the given device
> --
> 2.23.0
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

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

* Re: [PATCH 2/3] fstests: btrfs/14[23]: Use proper help to get both devid and physical offset for corruption.
  2019-12-11 10:40 ` [PATCH 2/3] fstests: btrfs/14[23]: Use proper help to get both devid and physical offset for corruption Qu Wenruo
  2019-12-11 11:24   ` Nikolay Borisov
@ 2019-12-17 17:30   ` Filipe Manana
  1 sibling, 0 replies; 9+ messages in thread
From: Filipe Manana @ 2019-12-17 17:30 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: fstests, linux-btrfs, Filipe Manana

On Wed, Dec 11, 2019 at 10:41 AM Qu Wenruo <wqu@suse.com> wrote:
>
> [BUG]
> When using btrfs-progs v5.4, btrfs/142 and btrfs/143 will fail:
> btrfs/142 1s ... - output mismatch (see xfstests/results//btrfs/142.out.bad)
>     --- tests/btrfs/142.out 2018-09-16 21:30:48.505104287 +0100
>     +++ xfstests/results//btrfs/142.out.bad
> 2019-12-10 15:35:40.280392626 +0000
>     @@ -3,37 +3,37 @@
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>      wrote 65536/65536 bytes
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
>     -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
>     -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
>     -XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa ................
>     ...
>     (Run 'diff -u xfstests/tests/btrfs/142.out xfstests/results//btrfs/142.out.bad' to see the entire diff)
>
> [CAUSE]
> Btrfs/14[23] test whether a read on corrupted stripe will re-silver
> itself.
> Such test by its nature will need to modify on-disk data, thus need to
> get the btrfs logical -> physical mapping, which is done by near
> hard-coded lookup function, which rely on certain stripe:devid sequence.
>
> Recent btrfs-progs commit c501c9e3b816 ("btrfs-progs: mkfs: match devid
> order to the stripe index") changes how we use devices in mkfs.btrfs,
> this caused a change in chunk layout, and break the hard-coded
> stripe:devid sequence.
>
> [FIX]
> This patch will do full devid and physical offset lookup, instead of old
> physical offset only lookup.
>
> The only assumption made is, mkfs.btrfs assigns devid sequentially for
> its devices.
> Which means, for "mkfs.btrfs $dev1 $dev2 $dev3", we get devid 1 for $dev1,
> devid 2 for $dev2, and so on.
>
> This change will allow btrfs/14[23] to handle even future chunk layout
> change. (Although I hope this will never happen again).
>
> This also addes extra debug output (although less than 10 lines) into
> $seqres.full, just in case when layout changes and current lookup can't
> handle it, developer can still pindown the problem easily.
>
> Reported-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/btrfs/142     | 50 +++++++++++++++++++++++++++++++--------------
>  tests/btrfs/142.out |  2 --
>  tests/btrfs/143     | 48 +++++++++++++++++++++++++++++++------------
>  tests/btrfs/143.out |  2 --
>  4 files changed, 70 insertions(+), 32 deletions(-)
>
> diff --git a/tests/btrfs/142 b/tests/btrfs/142
> index a23fe1bf..9c037ff6 100755
> --- a/tests/btrfs/142
> +++ b/tests/btrfs/142
> @@ -47,30 +47,45 @@ _require_command "$FILEFRAG_PROG" filefrag
>
>  get_physical()
>  {
> -        # $1 is logical address
> -        # print chunk tree and find devid 2 which is $SCRATCH_DEV
> +       local logical=$1
> +       local stripe=$2
>          $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> -       grep $1 -A 6 | awk '($1 ~ /stripe/ && $3 ~ /devid/ && $4 ~ /1/) { print $6 }'
> +               grep $logical -A 6 | \
> +               awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"

Should use $AWK_PROG instead of direct call to 'awk'.

>  }
>
> +get_devid()
> +{
> +       local logical=$1
> +       local stripe=$2
> +        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> +               grep $logical -A 6 | \
> +               awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"

Same here.

> +}
>
> -SYSFS_BDEV=`_sysfs_dev $SCRATCH_DEV`
> +get_device_path()
> +{
> +       local devid=$1
> +       echo "$SCRATCH_DEV_POOL" | awk "{print \$$devid}"
> +}
>
>  start_fail()
>  {
> +       local sysfs_bdev="$1"
>         echo 100 > $DEBUGFS_MNT/fail_make_request/probability
>         echo 2 > $DEBUGFS_MNT/fail_make_request/times
>         echo 1 > $DEBUGFS_MNT/fail_make_request/task-filter
>         echo 0 > $DEBUGFS_MNT/fail_make_request/verbose
> -       echo 1 > $SYSFS_BDEV/make-it-fail
> +       echo 1 > $sysfs_bdev/make-it-fail
>  }
>
>  stop_fail()
>  {
> +       local sysfs_bdev="$1"
>         echo 0 > $DEBUGFS_MNT/fail_make_request/probability
>         echo 0 > $DEBUGFS_MNT/fail_make_request/times
>         echo 0 > $DEBUGFS_MNT/fail_make_request/task-filter
> -       echo 0 > $SYSFS_BDEV/make-it-fail
> +       echo 0 > $sysfs_bdev/make-it-fail
>  }
>
>  _scratch_dev_pool_get 2
> @@ -87,17 +102,23 @@ _scratch_mount -o nospace_cache,nodatasum
>  $XFS_IO_PROG -f -d -c "pwrite -S 0xaa -b 128K 0 128K" "$SCRATCH_MNT/foobar" |\
>         _filter_xfs_io_offset
>
> -# step 2, corrupt the first 64k of one copy (on SCRATCH_DEV which is the first
> -# one in $SCRATCH_DEV_POOL
> +# step 2, corrupt the first 64k of stripe #1
>  echo "step 2......corrupt file extent" >>$seqres.full
>
>  ${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar >> $seqres.full
>  logical_in_btrfs=`${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar | _filter_filefrag | cut -d '#' -f 1`
> -physical_on_scratch=`get_physical ${logical_in_btrfs}`
> +physical=`get_physical ${logical_in_btrfs} 1`
> +devid=$(get_devid ${logical_in_btrfs} 1)
> +target_dev=$(get_device_path $devid)
> +
> +SYSFS_BDEV=`_sysfs_dev $target_dev`
>
>  _scratch_unmount
> -$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical_on_scratch 64K" $SCRATCH_DEV |\
> -       _filter_xfs_io_offset
> +$BTRFS_UTIL_PROG ins dump-tree -t 3 $SCRATCH_DEV | \
> +       grep $logical_in_btrfs -A 6 >> $seqres.full
> +echo "Corrupt stripe 1 devid $devid devpath $target_dev physical $physical" \
> +       >> $seqres.full
> +$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical 64K" $target_dev > /dev/null
>
>  _scratch_mount -o nospace_cache
>
> @@ -106,8 +127,7 @@ echo "step 3......repair the bad copy" >>$seqres.full
>
>  # since raid1 consists of two copies, and the bad copy was put on stripe #1
>  # while the good copy lies on stripe #0, the bad copy only gets access when the
> -# reader's pid % 2 == 1 is true
> -start_fail
> +start_fail $SYSFS_BDEV
>  while [[ -z ${result} ]]; do
>         # enable task-filter only fails the following dio read so the repair is
>         # supposed to work.
> @@ -117,12 +137,12 @@ while [[ -z ${result} ]]; do
>                 exec $XFS_IO_PROG -d -c \"pread -b 128K 0 128K\" \"$SCRATCH_MNT/foobar\"
>         fi");
>  done
> -stop_fail
> +stop_fail $SYSFS_BDEV
>
>  _scratch_unmount
>
>  # check if the repair works
> -$XFS_IO_PROG -c "pread -v -b 512 $physical_on_scratch 512" $SCRATCH_DEV |\
> +$XFS_IO_PROG -c "pread -v -b 512 $physical 512" $target_dev | \
>         _filter_xfs_io_offset
>
>  _scratch_dev_pool_put
> diff --git a/tests/btrfs/142.out b/tests/btrfs/142.out
> index 0f32ffbe..2e22f292 100644
> --- a/tests/btrfs/142.out
> +++ b/tests/btrfs/142.out
> @@ -1,8 +1,6 @@
>  QA output created by 142
>  wrote 131072/131072 bytes
>  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
> diff --git a/tests/btrfs/143 b/tests/btrfs/143
> index 91af52f9..b2ffeb63 100755
> --- a/tests/btrfs/143
> +++ b/tests/btrfs/143
> @@ -54,30 +54,48 @@ _require_command "$FILEFRAG_PROG" filefrag
>
>  get_physical()
>  {
> -        # $1 is logical address
> -        # print chunk tree and find devid 2 which is $SCRATCH_DEV
> +       local logical=$1
> +       local stripe=$2
>          $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> -       grep $1 -A 6 | awk '($1 ~ /stripe/ && $3 ~ /devid/ && $4 ~ /1/) { print $6 }'
> +               grep $logical -A 6 | \
> +               awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"

Same here.

> +}
> +
> +get_devid()
> +{
> +       local logical=$1
> +       local stripe=$2
> +        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> +               grep $logical -A 6 | \
> +               awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"

Same here.

> +}
> +
> +get_device_path()
> +{
> +       local devid=$1
> +       echo "$SCRATCH_DEV_POOL" | awk "{print \$$devid}"
>  }
>
>  SYSFS_BDEV=`_sysfs_dev $SCRATCH_DEV`
>
>  start_fail()
>  {
> +       local sysfs_bdev="$1"
>         echo 100 > $DEBUGFS_MNT/fail_make_request/probability
>         # the 1st one fails the first bio which is reading 4k (or more due to
>         # readahead), and the 2nd one fails the retry of validation so that it
>         # triggers read-repair
>         echo 2 > $DEBUGFS_MNT/fail_make_request/times
>         echo 0 > $DEBUGFS_MNT/fail_make_request/verbose
> -       echo 1 > $SYSFS_BDEV/make-it-fail
> +       echo 1 > $sysfs_bdev/make-it-fail
>  }
>
>  stop_fail()
>  {
> +       local sysfs_bdev="$1"
>         echo 0 > $DEBUGFS_MNT/fail_make_request/probability
>         echo 0 > $DEBUGFS_MNT/fail_make_request/times
> -       echo 0 > $SYSFS_BDEV/make-it-fail
> +       echo 0 > $sysfs_bdev/make-it-fail
>  }
>
>  _scratch_dev_pool_get 2
> @@ -94,17 +112,21 @@ _scratch_mount -o nospace_cache,nodatasum
>  $XFS_IO_PROG -f -d -c "pwrite -S 0xaa -b 128K 0 128K" "$SCRATCH_MNT/foobar" |\
>         _filter_xfs_io_offset
>
> -# step 2, corrupt the first 64k of one copy (on SCRATCH_DEV which is the first
> -# one in $SCRATCH_DEV_POOL
> +# step 2, corrupt the first 64k of stripe #1
>  echo "step 2......corrupt file extent" >>$seqres.full
>
>  ${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar >> $seqres.full
>  logical_in_btrfs=`${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar | _filter_filefrag | cut -d '#' -f 1`
> -physical_on_scratch=`get_physical ${logical_in_btrfs}`
> +physical=`get_physical ${logical_in_btrfs} 1`
> +devid=$(get_devid ${logical_in_btrfs} 1)
> +target_dev=$(get_device_path $devid)
>
> +SYSFS_BDEV=`_sysfs_dev $target_dev`
>  _scratch_unmount
> -$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical_on_scratch 64K" $SCRATCH_DEV |\
> -       _filter_xfs_io_offset
> +
> +echo "corrupt stripe 1 devid $devid devpath $target_dev physical $physical" \
> +       >> $seqres.full
> +$XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical 64K" $target_dev > /dev/null
>
>  _scratch_mount -o nospace_cache
>
> @@ -118,18 +140,18 @@ while [[ -z ${result} ]]; do
>      # invalidate the page cache.
>      _scratch_cycle_mount
>
> -    start_fail
> +    start_fail $SYSFS_BDEV
>      result=$(bash -c "
>          if [[ \$((\$\$ % 2)) -eq 1 ]]; then
>                  exec $XFS_IO_PROG -c \"pread 0 4K\" \"$SCRATCH_MNT/foobar\"
>          fi");
> -    stop_fail
> +    stop_fail $SYSFS_BDEV
>  done
>
>  _scratch_unmount
>
>  # check if the repair works
> -$XFS_IO_PROG -c "pread -v -b 512 $physical_on_scratch 512" $SCRATCH_DEV |\
> +$XFS_IO_PROG -c "pread -v -b 512 $physical 512" $target_dev |\
>         _filter_xfs_io_offset
>
>  _scratch_dev_pool_put
> diff --git a/tests/btrfs/143.out b/tests/btrfs/143.out
> index 66afea4b..a9e82ceb 100644
> --- a/tests/btrfs/143.out
> +++ b/tests/btrfs/143.out
> @@ -1,8 +1,6 @@
>  QA output created by 143
>  wrote 131072/131072 bytes
>  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
>  XXXXXXXX:  aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa aa  ................
> --
> 2.23.0
>

Other than that (which can probably be fixed at commit time by Eryu)
it looks good and it solves the problem.

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

Thanks.


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

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

* Re: [PATCH 3/3] fstests: btrfs/15[78]: Use proper helper to get both devid and physical offset for corruption
  2019-12-11 10:40 ` [PATCH 3/3] fstests: btrfs/15[78]: Use proper helper " Qu Wenruo
  2019-12-11 11:24   ` Nikolay Borisov
@ 2019-12-17 17:33   ` Filipe Manana
  1 sibling, 0 replies; 9+ messages in thread
From: Filipe Manana @ 2019-12-17 17:33 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: fstests, linux-btrfs, Filipe Manana

On Wed, Dec 11, 2019 at 10:40 AM Qu Wenruo <wqu@suse.com> wrote:
>
> [BUG]
> When using btrfs-progs v5.4, btrfs/157 and btrfs/158 will fail:
>
> btrfs/157 1s ... - output mismatch (see xfstests/results//btrfs/157.out.bad)
>     --- tests/btrfs/157.out 2018-09-16 21:30:48.505104287 +0100
>     +++ xfstests/results//btrfs/157.out.bad
> 2019-12-10 15:35:43.112390076 +0000
>     @@ -1,9 +1,9 @@
>      QA output created by 157
>      wrote 131072/131072 bytes at offset 0
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 9437184
>     +wrote 65536/65536 bytes at offset 22020096
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 9437184
>     ...
>     (Run 'diff -u xfstests/tests/btrfs/157.out xfstests/results//btrfs/157.out.bad'  to see the entire diff)
> btrfs/158 2s ... - output mismatch (see xfstests/results//btrfs/158.out.bad)
>     --- tests/btrfs/158.out 2018-09-16 21:30:48.505104287 +0100
>     +++ xfstests/results//btrfs/158.out.bad
> 2019-12-10 15:35:44.844388521 +0000
>     @@ -1,9 +1,9 @@
>      QA output created by 158
>      wrote 131072/131072 bytes at offset 0
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 9437184
>     +wrote 65536/65536 bytes at offset 22020096
>      XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>     -wrote 65536/65536 bytes at offset 9437184
>     ...
>     (Run 'diff -u xfstests/tests/btrfs/158.out xfstests/results//btrfs/158.out.bad'  to see the entire diff)
>
> [CAUSE]
> This two tests use physical offset as golden output, while mkfs.btrfs
> can do whatever it likes to arrange its chunk layout, thus physical
> offset is never reliable.
>
> And btrfs-progs commit c501c9e3b816 ("btrfs-progs: mkfs: match devid
> order to the stripe index") just changed the layout.
>
> So the output mismatch and failed.
>
> [FIX]
> In fact, that btrfs-progs commit not only changed offset, but also the
> device sequence.
>
> So we can't just simply remove the physical offset, but also need to use
> proper helper to get both devid (as its device path) and physical offset
> for corruption.
>
> As long as mkfs.btrfs still uses sequential devid, these tests should
> handle future chunk layout change without problem.
>
> Reported-by: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/btrfs/157     | 53 +++++++++++++++++++++++++++++----------------
>  tests/btrfs/157.out |  4 ----
>  tests/btrfs/158     | 48 +++++++++++++++++++++++++---------------
>  tests/btrfs/158.out |  4 ----
>  4 files changed, 65 insertions(+), 44 deletions(-)
>
> diff --git a/tests/btrfs/157 b/tests/btrfs/157
> index 7f75c407..80b01b8d 100755
> --- a/tests/btrfs/157
> +++ b/tests/btrfs/157
> @@ -51,22 +51,30 @@ _require_scratch_dev_pool 4
>  _require_btrfs_command inspect-internal dump-tree
>  _require_btrfs_fs_feature raid56
>
> -get_physical_stripe0()
> +get_physical()
>  {
> -       $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> -       grep " DATA\|RAID6" -A 10 | \
> -       $AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /0/) { print $6 }'
> +       local stripe=$1
> +        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> +               grep " DATA\|RAID6" -A 10 | \
> +               awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"

Same as in the other patch, use $AWK_PROG instead.

>  }
>
> -get_physical_stripe1()
> +get_devid()
>  {
> -       $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> -       grep " DATA\|RAID6" -A 10 | \
> -       $AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /1/) { print $6 }'
> +       local stripe=$1
> +        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> +               grep " DATA\|RAID6" -A 10 | \
> +               awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"

Same here.

> +}
> +
> +get_device_path()
> +{
> +       local devid=$1
> +       echo "$SCRATCH_DEV_POOL" | awk "{print \$$devid}"
>  }
>
>  _scratch_dev_pool_get 4
> -# step 1: create a raid6 btrfs and create a 4K file
> +# step 1: create a raid6 btrfs and create a 128K file
>  echo "step 1......mkfs.btrfs" >>$seqres.full
>
>  mkfs_opts="-d raid6 -b 1G"
> @@ -80,18 +88,25 @@ _scratch_mount -o nospace_cache
>  $XFS_IO_PROG -f -d -c "pwrite -S 0xaa 0 128K" -c "fsync" \
>         "$SCRATCH_MNT/foobar" | _filter_xfs_io
>
> +logical=`${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar | _filter_filefrag | cut -d '#' -f 1`
>  _scratch_unmount
>
> -stripe_0=`get_physical_stripe0`
> -stripe_1=`get_physical_stripe1`
> -dev4=`echo $SCRATCH_DEV_POOL | awk '{print $4}'`
> -dev3=`echo $SCRATCH_DEV_POOL | awk '{print $3}'`
> -
> -# step 2: corrupt the 1st and 2nd stripe (stripe 0 and 1)
> -echo "step 2......simulate bitrot at offset $stripe_0 of device_4($dev4) and offset $stripe_1 of device_3($dev3)" >>$seqres.full
> -
> -$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_0 64K" $dev4 | _filter_xfs_io
> -$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_1 64K" $dev3 | _filter_xfs_io
> +phy0=$(get_physical 0)
> +devid0=$(get_devid 0)
> +devpath0=$(get_device_path $devid0)
> +phy1=$(get_physical 1)
> +devid1=$(get_devid 1)
> +devpath1=$(get_device_path $devid1)
> +
> +# step 2: corrupt stripe #0 and #1
> +echo "step 2......simulate bitrot at:" >>$seqres.full
> +echo "      ......stripe #0: devid $devid0 devpath $devpath0 phy $phy0" \
> +       >>$seqres.full
> +echo "      ......stripe #1: devid $devid1 devpath $devpath1 phy $phy1" \
> +       >>$seqres.full
> +
> +$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy0 64K" $devpath0 > /dev/null
> +$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy1 64K" $devpath1 > /dev/null
>
>  # step 3: read foobar to repair the bitrot
>  echo "step 3......repair the bitrot" >> $seqres.full
> diff --git a/tests/btrfs/157.out b/tests/btrfs/157.out
> index 08d592c4..d69c0f1d 100644
> --- a/tests/btrfs/157.out
> +++ b/tests/btrfs/157.out
> @@ -1,10 +1,6 @@
>  QA output created by 157
>  wrote 131072/131072 bytes at offset 0
>  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes at offset 9437184
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes at offset 9437184
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  0200000 aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa
>  *
>  0400000
> diff --git a/tests/btrfs/158 b/tests/btrfs/158
> index 603e8bea..c8d5af82 100755
> --- a/tests/btrfs/158
> +++ b/tests/btrfs/158
> @@ -43,22 +43,30 @@ _require_scratch_dev_pool 4
>  _require_btrfs_command inspect-internal dump-tree
>  _require_btrfs_fs_feature raid56
>
> -get_physical_stripe0()
> +get_physical()
>  {
> -       $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> -       grep " DATA\|RAID6" -A 10 | \
> -       $AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /0/) { print $6 }'
> +       local stripe=$1
> +        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> +               grep " DATA\|RAID6" -A 10 | \
> +               awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$6 }"

Same here.

>  }
>
> -get_physical_stripe1()
> +get_devid()
>  {
> -       $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> -       grep " DATA\|RAID6" -A 10 | \
> -       $AWK_PROG '($1 ~ /stripe/ && $3 ~ /devid/ && $2 ~ /1/) { print $6 }'
> +       local stripe=$1
> +        $BTRFS_UTIL_PROG inspect-internal dump-tree -t 3 $SCRATCH_DEV | \
> +               grep " DATA\|RAID6" -A 10 | \
> +               awk "(\$1 ~ /stripe/ && \$3 ~ /devid/ && \$2 ~ /$stripe/) { print \$4 }"

Same here.

> +}
> +
> +get_device_path()
> +{
> +       local devid=$1
> +       echo "$SCRATCH_DEV_POOL" | awk "{print \$$devid}"
>  }
>
>  _scratch_dev_pool_get 4
> -# step 1: create a raid6 btrfs and create a 4K file
> +# step 1: create a raid6 btrfs and create a 128K file
>  echo "step 1......mkfs.btrfs" >>$seqres.full
>
>  mkfs_opts="-d raid6 -b 1G"
> @@ -74,16 +82,22 @@ $XFS_IO_PROG -f -d -c "pwrite -S 0xaa 0 128K" -c "fsync" \
>
>  _scratch_unmount
>
> -stripe_0=`get_physical_stripe0`
> -stripe_1=`get_physical_stripe1`
> -dev4=`echo $SCRATCH_DEV_POOL | awk '{print $4}'`
> -dev3=`echo $SCRATCH_DEV_POOL | awk '{print $3}'`
> +phy0=$(get_physical 0)
> +devid0=$(get_devid 0)
> +devpath0=$(get_device_path $devid0)
> +phy1=$(get_physical 1)
> +devid1=$(get_devid 1)
> +devpath1=$(get_device_path $devid1)
>
>  # step 2: corrupt the 1st and 2nd stripe (stripe 0 and 1)
> -echo "step 2......simulate bitrot at offset $stripe_0 of device_4($dev4) and offset $stripe_1 of device_3($dev3)" >>$seqres.full
> -
> -$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_0 64K" $dev4 | _filter_xfs_io
> -$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $stripe_1 64K" $dev3 | _filter_xfs_io
> +echo "step 2......simulate bitrot at:" >>$seqres.full
> +echo "      ......stripe #0: devid $devid0 devpath $devpath0 phy $phy0" \
> +       >>$seqres.full
> +echo "      ......stripe #1: devid $devid1 devpath $devpath1 phy $phy1" \
> +       >>$seqres.full
> +
> +$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy0 64K" $devpath0 > /dev/null
> +$XFS_IO_PROG -f -d -c "pwrite -S 0xbb $phy1 64K" $devpath1 > /dev/null
>
>  # step 3: scrub filesystem to repair the bitrot
>  echo "step 3......repair the bitrot" >> $seqres.full
> diff --git a/tests/btrfs/158.out b/tests/btrfs/158.out
> index 1f5ad3f7..95562f49 100644
> --- a/tests/btrfs/158.out
> +++ b/tests/btrfs/158.out
> @@ -1,10 +1,6 @@
>  QA output created by 158
>  wrote 131072/131072 bytes at offset 0
>  XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes at offset 9437184
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes at offset 9437184
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  0000000 aaaa aaaa aaaa aaaa aaaa aaaa aaaa aaaa
>  *
>  0400000
> --
> 2.23.0
>

Other than that (which can probably be fixed at commit time by Eryu)
it looks good and it solves the problem.

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

Thanks.


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

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

end of thread, other threads:[~2019-12-17 17:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-11 10:40 [PATCH 0/3] fstests: btrfs/15[78] btrfs/14[23]: Use more accurate devid/phsyical for corruption Qu Wenruo
2019-12-11 10:40 ` [PATCH 1/3] fstests: common: Use more accurate kernel config for _require_fail_make_request Qu Wenruo
2019-12-17 17:24   ` Filipe Manana
2019-12-11 10:40 ` [PATCH 2/3] fstests: btrfs/14[23]: Use proper help to get both devid and physical offset for corruption Qu Wenruo
2019-12-11 11:24   ` Nikolay Borisov
2019-12-17 17:30   ` Filipe Manana
2019-12-11 10:40 ` [PATCH 3/3] fstests: btrfs/15[78]: Use proper helper " Qu Wenruo
2019-12-11 11:24   ` Nikolay Borisov
2019-12-17 17:33   ` Filipe Manana

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).