On Wed, Jan 24, 2024 at 11:05:10AM -0500, Josef Bacik wrote:
> For validating that read repair works properly we corrupt one mirror and
> then read back the physical location after we do a direct or buffered
> read on the mounted file system and then unmount the file system. The
> golden output expects all a's, however with encryption this will
> obviously not be the case.
>
> However I still broke read repair, so these tests are quite valuable.
> Fix them to dump the on disk values to a temporary file and then md5sum
> the files, and then validate the md5sum to make sure the read repair
> worked properly.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> tests/btrfs/140 | 15 ++++++++++++++-
> tests/btrfs/140.out | 34 ----------------------------------
> tests/btrfs/141 | 16 +++++++++++++++-
> tests/btrfs/141.out | 34 ----------------------------------
> 4 files changed, 29 insertions(+), 70 deletions(-)
>
> diff --git a/tests/btrfs/140 b/tests/btrfs/140
> index 247a7356..8e1aafa3 100755
> --- a/tests/btrfs/140
> +++ b/tests/btrfs/140
> @@ -77,6 +77,13 @@ devpath=$(get_device_path ${devid})
>
> _scratch_unmount
>
> +# Grab the contents of the the area so we can compare to the final part
> +orig=$(mktemp)
We don't use mktemp for tmpfile. Please use $tmp (it's defined at the
beginning of each test case), then it'll be sure to be removed in _cleanup().
> +$XFS_IO_PROG -d -c "pread -v -b 512 $physical 512" $devpath |\
> + _filter_xfs_io_offset > $orig
> +origcsum=$(_md5_checksum $orig)
> +rm -f $orig
> +
> echo " corrupt stripe #1, devid $devid devpath $devpath physical $physical" \
> >> $seqres.full
> $XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical 64K" $devpath > /dev/null
> @@ -91,10 +98,16 @@ _btrfs_direct_read_on_mirror 1 2 "$SCRATCH_MNT/foobar" 0 128K
> _scratch_unmount
>
> # check if the repair works
> +final=$(mktemp)
> $XFS_IO_PROG -d -c "pread -v -b 512 $physical 512" $devpath |\
> - _filter_xfs_io_offset
> + _filter_xfs_io_offset > $final
> +finalcsum=$(_md5_checksum $final)
> +rm -f $final
>
> _scratch_dev_pool_put
> +
> +[ "$origcsum" == "$finalcsum" ] || _fail "repair failed, csums don't match"
Might be worth outputting the $orig and $final into .full file, at least
when the test fails, to help debug.
Same review points below.
If there's not objection from the the original author of these two cases
and btrfs list, I'm good with this change if it helps for more testing.
Thanks,
Zorro
> +
> # success, all done
> status=0
> exit
> diff --git a/tests/btrfs/140.out b/tests/btrfs/140.out
> index fb5aa108..58dfb24e 100644
> --- a/tests/btrfs/140.out
> +++ b/tests/btrfs/140.out
> @@ -1,37 +1,3 @@
> QA output created by 140
> wrote 131072/131072 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 ................
> -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 ................
> -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 ................
> -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 ................
> -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 ................
> -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 ................
> -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 ................
> -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 ................
> -read 512/512 bytes
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> diff --git a/tests/btrfs/141 b/tests/btrfs/141
> index 90a90d00..41407f90 100755
> --- a/tests/btrfs/141
> +++ b/tests/btrfs/141
> @@ -74,6 +74,14 @@ devid=$(get_devid ${logical_in_btrfs} 1)
> devpath=$(get_device_path ${devid})
>
> _scratch_unmount
> +
> +# Grab the contents of the area so we can compare to the final part
> +orig=$(mktemp)
> +$XFS_IO_PROG -c "pread -v -b 512 $physical 512" $devpath |\
> + _filter_xfs_io_offset > $orig
> +origcsum=$(_md5_checksum $orig)
> +rm -f $orig
> +
> echo " corrupt stripe #1, devid $devid devpath $devpath physical $physical" \
> >> $seqres.full
> $XFS_IO_PROG -d -c "pwrite -S 0xbb -b 64K $physical 64K" $devpath > /dev/null
> @@ -88,10 +96,16 @@ _btrfs_buffered_read_on_mirror 1 2 "$SCRATCH_MNT/foobar" 0 128K
> _scratch_unmount
>
> # check if the repair works
> +final=$(mktemp)
> $XFS_IO_PROG -c "pread -v -b 512 $physical 512" $devpath |\
> - _filter_xfs_io_offset
> + _filter_xfs_io_offset > $final
> +finalcsum=$(_md5_checksum $final)
> +rm -f $final
>
> _scratch_dev_pool_put
> +
> +[ "$origcsum" == "$finalcsum" ] || _fail "repair failed, csums don't match"
> +
> # success, all done
> status=0
> exit
> diff --git a/tests/btrfs/141.out b/tests/btrfs/141.out
> index 4b8be189..d8c6940f 100644
> --- a/tests/btrfs/141.out
> +++ b/tests/btrfs/141.out
> @@ -1,37 +1,3 @@
> QA output created by 141
> wrote 131072/131072 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 ................
> -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 ................
> -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 ................
> -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 ................
> -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 ................
> -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 ................
> -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 ................
> -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 ................
> -read 512/512 bytes
> -XXX Bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> --
> 2.43.0
>
>
