Re: [PATCH] btrfs/319: make the test work when compression is used

[Qu Wenruo <quwenruo.btrfs@xxxxxxx>]

在 2024/9/6 01:08, fdmanana@xxxxxxxxxx 写道:
> From: Filipe Manana <fdmanana@xxxxxxxx>
>
> Currently btrfs/319 assumes there is no compression and that the files
> get a single extent (1 fiemap line) with a size of 1048581 bytes. However
> when testing with compression, for example by passing "-o compress" to
> MOUNT_OPTIONS environment variable, we get several extents and two lines
> of fiemap output, which makes the test fail since it hardcodes the fiemap
> output:
>
>    $ MOUNT_OPTIONS="-o compress" ./check btrfs/319
>    FSTYP         -- btrfs
>    PLATFORM      -- Linux/x86_64 debian0 6.11.0-rc6-btrfs-next-173+ #1 SMP PREEMPT_DYNAMIC Tue Sep  3 17:40:24 WEST 2024
>    MKFS_OPTIONS  -- /dev/sdc
>    MOUNT_OPTIONS -- -o compress /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
>
>    btrfs/319 1s ... - output mismatch (see /home/fdmanana/git/hub/xfstests/results//btrfs/319.out.bad)
>        --- tests/btrfs/319.out	2024-08-12 14:16:55.653383284 +0100
>        +++ /home/fdmanana/git/hub/xfstests/results//btrfs/319.out.bad	2024-09-05 15:24:53.323076548 +0100
>        @@ -6,11 +6,13 @@
>         e61178ee0288ebe3fa36a3c975b02c94  SCRATCH_MNT/snap/foo
>         e61178ee0288ebe3fa36a3c975b02c94  SCRATCH_MNT/snap/bar
>         File bar fiemap in the original filesystem:
>        -0: [0..2055]: shared|last
>        +0: [0..2047]: shared
>        +1: [2048..2055]: shared|last
>         Creating a new filesystem to receive the send stream...
>        ...
>        (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/319.out /home/fdmanana/git/hub/xfstests/results//btrfs/319.out.bad'  to see the entire diff)
>
>    HINT: You _MAY_ be missing kernel fix:
>          46a6e10a1ab1 btrfs: send: allow cloning non-aligned extent if it ends at i_size
>
>    Ran: btrfs/319
>    Failures: btrfs/319
>    Failed 1 of 1 tests
>
> So change the test to not rely on the fiemap output in its golden output
> and instead just check if all the extents reported by fiemap have the
> shared flag set (failing if there are any without the shared flag).

Looks good to me.

Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>

Just one minor improvement to make debug a little easier.

> Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
> ---
>   tests/btrfs/319     | 19 +++++++++++++++----
>   tests/btrfs/319.out |  4 ----
>   2 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/tests/btrfs/319 b/tests/btrfs/319
> index 975c1497..7cfd3d00 100755
> --- a/tests/btrfs/319
> +++ b/tests/btrfs/319
> @@ -32,6 +32,19 @@ _require_odirect
>   _fixed_by_kernel_commit 46a6e10a1ab1 \
>   	"btrfs: send: allow cloning non-aligned extent if it ends at i_size"
>
> +check_all_extents_shared()
> +{
> +	local file=$1
> +	local fiemap_output
> +
> +	fiemap_output=$($XFS_IO_PROG -r -c "fiemap -v" $file | _filter_fiemap_flags)

Maybe also save the full unfiltered output to seqres.full?

Thanks,
Qu

> +	echo "$fiemap_output" | grep -qv 'shared'
> +	if [ $? -eq 0 ]; then
> +		echo -e "Found non-shared extents for file $file:\n"
> +		echo "$fiemap_output"
> +	fi
> +}
> +
>   send_files_dir=$TEST_DIR/btrfs-test-$seq
>   send_stream=$send_files_dir/snap.stream
>
> @@ -58,8 +71,7 @@ echo "File digests in the original filesystem:"
>   md5sum $SCRATCH_MNT/snap/foo | _filter_scratch
>   md5sum $SCRATCH_MNT/snap/bar | _filter_scratch
>
> -echo "File bar fiemap in the original filesystem:"
> -$XFS_IO_PROG -r -c "fiemap -v" $SCRATCH_MNT/snap/bar | _filter_fiemap_flags
> +check_all_extents_shared "$SCRATCH_MNT/snap/bar"
>
>   echo "Creating a new filesystem to receive the send stream..."
>   _scratch_unmount
> @@ -72,8 +84,7 @@ echo "File digests in the new filesystem:"
>   md5sum $SCRATCH_MNT/snap/foo | _filter_scratch
>   md5sum $SCRATCH_MNT/snap/bar | _filter_scratch
>
> -echo "File bar fiemap in the new filesystem:"
> -$XFS_IO_PROG -r -c "fiemap -v" $SCRATCH_MNT/snap/bar | _filter_fiemap_flags
> +check_all_extents_shared "$SCRATCH_MNT/snap/bar"
>
>   # success, all done
>   status=0
> diff --git a/tests/btrfs/319.out b/tests/btrfs/319.out
> index 14079dbe..18a50ff8 100644
> --- a/tests/btrfs/319.out
> +++ b/tests/btrfs/319.out
> @@ -5,12 +5,8 @@ Creating snapshot and a send stream for it...
>   File digests in the original filesystem:
>   e61178ee0288ebe3fa36a3c975b02c94  SCRATCH_MNT/snap/foo
>   e61178ee0288ebe3fa36a3c975b02c94  SCRATCH_MNT/snap/bar
> -File bar fiemap in the original filesystem:
> -0: [0..2055]: shared|last
>   Creating a new filesystem to receive the send stream...
>   At subvol snap
>   File digests in the new filesystem:
>   e61178ee0288ebe3fa36a3c975b02c94  SCRATCH_MNT/snap/foo
>   e61178ee0288ebe3fa36a3c975b02c94  SCRATCH_MNT/snap/bar
> -File bar fiemap in the new filesystem:
> -0: [0..2055]: shared|last