On 2022/5/30 08:23, Anand Jain wrote:
On 5/27/22 13:49, Christoph Hellwig wrote:
Add a few helpers to consolidate code for btrfs read repair testing:
- _btrfs_get_first_logical() gets the btrfs logical address for the
first extent in a file
- _btrfs_get_device_path and _btrfs_get_physical use the
btrfs-map-logical tool to find the device path and physical address
for btrfs logical address for a specific mirror
- _btrfs_direct_read_on_mirror and _btrfs_buffered_read_on_mirror
read the data from a specific mirror
These will be used to consolidate the read repair tests and avoid
duplication for new tests.
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>
---
common/btrfs | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++
common/config | 1 +
2 files changed, 76 insertions(+)
diff --git a/common/btrfs b/common/btrfs
index ac597ca4..b69feeee 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -505,3 +505,78 @@ _btrfs_metadump()
$BTRFS_IMAGE_PROG "$device" "$dumpfile"
[ -n "$DUMP_COMPRESSOR" ] && $DUMP_COMPRESSOR -f "$dumpfile" &>
/dev/null
}
+
+# Return the btrfs logical address for the first block in a file
+_btrfs_get_first_logical()
+{
+ local file=$1
+ _require_command "$FILEFRAG_PROG" filefrag
+
+ ${FILEFRAG_PROG} -v $SCRATCH_MNT/foobar >> $seqres.full
+ ${FILEFRAG_PROG} -v $file | _filter_filefrag | cut -d '#' -f 1
+}
+
+# Find the device path for a btrfs logical offset
+_btrfs_get_device_path()
+{
+ local logical=$1
+ local stripe=$2
+
+ _require_command "$BTRFS_MAP_LOGICAL_PROG" btrfs-map-logical
+
+ $BTRFS_MAP_LOGICAL_PROG -l $logical $SCRATCH_DEV | \
+ $AWK_PROG "(\$1 ~ /mirror/ && \$2 ~ /$stripe/) { print \$8 }"
+}
+
+
+# Find the device physical sector for a btrfs logical offset
+_btrfs_get_physical()
+{
+ local logical=$1
+ local stripe=$2
+
+ _require_command "$BTRFS_MAP_LOGICAL_PROG" btrfs-map-logical
+
+ $BTRFS_MAP_LOGICAL_PROG -b -l $logical $SCRATCH_DEV >>
$seqres.full 2>&1
+ $BTRFS_MAP_LOGICAL_PROG -l $logical $SCRATCH_DEV | \
+ $AWK_PROG "(\$1 ~ /mirror/ && \$2 ~ /$stripe/) { print \$6 }"
+}
+
+# Read from a specific stripe to test read recovery that corrupted a
specific
+# stripe. Btrfs uses the PID to select the mirror, so keep reading
until the
+# xfs_io process that performed the read was executed with a PID that
ends up
+# on the intended mirror.
+_btrfs_direct_read_on_mirror()
+{
+ local mirror=$1
+ local nr_mirrors=$2
+ local file=$3
+ local offset=$4
+ local size=$5
+
+ while [[ -z $( (( BASHPID % nr_mirrors == mirror )) &&
+ exec $XFS_IO_PROG -d \
+ -c "pread -b $size $offset $size" $file) ]]; do
+ :
+ done
+}
+
+# Read from a specific stripe to test read recovery that corrupted a
specific
+# stripe. Btrfs uses the PID to select the mirror, so keep reading
until the
+# xfs_io process that performed the read was executed with a PID that
ends up
+# on the intended mirror.
+_btrfs_buffered_read_on_mirror()
+{
+ local mirror=$1
+ local nr_mirrors=$2
+ local file=$3
+ local offset=$4
+ local size=$5
+
+ echo 3 > /proc/sys/vm/drop_caches
+ while [[ -z $( (( BASHPID % nr_mirrors == mirror )) &&
+ exec $XFS_IO_PROG \
+ -c "pread -b $size $offset $size" $file) ]]; do
I am confused if it should be BASHPID or PID?
Next, it is ok if the xfs_io_prog fails and returns != 0.
(Part of the test).
But then we will continue in the while loop. No?
The loop goes like this:
If "BASHPID % nr_mirrors" isn't mirror, then the whole condition inside
the $() part is already 0, no need to exec the xfs_io command.
Then -z 0 return true, we continue the while loop (aka, doing nothing).
Only when (BASHPID % nr_mirrors == mirror) is true, we will try to exec
c XFS_IO_PROGS in the same PID, and normally we will exit.
Some of your concern are valid.
Like if some code bug makes the pread failed, then we will continue the
loop, possibly lead to an infinite loop.
The pid-check-then-exec idea is pretty good, it allows us to only
initiate the read, without doing some unnecessary read then check the pid.
But putting the whole exec into the pid check is indeed hacky and can
lead to problems.
Can we make it less hacky?
Thanks,
Qu
Thanks,
Qu
Sorry, I am sceptical about this. Could you please clarify
how this works?
Thanks, Anand
+ :
+ done
+}
diff --git a/common/config b/common/config
index c6428f90..df20afc1 100644
--- a/common/config
+++ b/common/config
@@ -228,6 +228,7 @@ export E2IMAGE_PROG="$(type -P e2image)"
export BLKZONE_PROG="$(type -P blkzone)"
export GZIP_PROG="$(type -P gzip)"
export BTRFS_IMAGE_PROG="$(type -P btrfs-image)"
+export BTRFS_MAP_LOGICAL_PROG=$(type -P btrfs-map-logical)
# use 'udevadm settle' or 'udevsettle' to wait for lv to be settled.
# newer systems have udevadm command but older systems like RHEL5
don't.
