Re: [PATCH 3/3] iotests/138: Test leaks/corruptions fixed report

[Eric Blake]

On 2/27/20 11:02 AM, Max Reitz wrote:
> Test that qemu-img check reports the number of leaks and corruptions
> fixed in its JSON report (after a successful run).
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
>   tests/qemu-iotests/138     | 41 ++++++++++++++++++++++++++++++++++++--
>   tests/qemu-iotests/138.out | 14 +++++++++++++
>   2 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/tests/qemu-iotests/138 b/tests/qemu-iotests/138
> index 54b01046ad..25bfbd4cca 100755
> --- a/tests/qemu-iotests/138
> +++ b/tests/qemu-iotests/138
> @@ -41,8 +41,10 @@ _supported_fmt qcow2
>   _supported_proto file
>   _supported_os Linux
>   # With an external data file, data clusters are not refcounted
> -# (and so qemu-img check does not check their refcount)
> -_unsupported_imgopts data_file
> +# (and so qemu-img check does not check their refcount);

Not this patch's problem, but is that a bug in 'qemu-img check' for not 
validating refcounts on an external data file?  Or is it merely this 
comment wording is not quite perfect?

> +# we want to modify the refcounts, so we need them to have a specific
> +# format (namely u16)
> +_unsupported_imgopts data_file 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
>   
>   echo
>   echo '=== Check on an image with a multiple of 2^32 clusters ==='
> @@ -65,6 +67,41 @@ poke_file "$TEST_IMG" $((2048 + 8)) 
> "\x00\x80\x00\x00\x00\x00\x00\x00"
>   # allocate memory", we have an error showing that l2 entry is invalid.
>   _check_test_img
>   
> +echo
> +echo '=== Check leaks-fixed/corruptions-fixed report'
> +echo
> +
> +# After leaks and corruptions were fixed, those numbers should be
> +# reported by qemu-img check
> +_make_test_img 64k
> +
> +# Allocate data cluster
> +$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io
> +
> +reftable_ofs=$(peek_file_be "$TEST_IMG" 48 8)
> +refblock_ofs=$(peek_file_be "$TEST_IMG" $reftable_ofs 8)
> +
> +# Introduce a leak: Make the image header's refcount 2
> +poke_file "$TEST_IMG" "$refblock_ofs" "\x00\x02"

Why not use your brand-new poke_file_be "$TEST_IMG" "$refblock_ofs" 2 2

> +
> +l1_ofs=$(peek_file_be "$TEST_IMG" 40 8)
> +
> +# Introduce a corruption: Drop the COPIED flag from the (first) L1 entry
> +l1_entry=$(peek_file_be "$TEST_IMG" $l1_ofs 8)
> +l1_entry=$((l1_entry & ~(1 << 63)))
> +poke_file_be "$TEST_IMG" $l1_ofs 8 $l1_entry

Yep, the new function makes this task easier.  (You could also just peek 
1 byte at $((l1_ofs+7)) then write it back out with poke_file 
"$TEST_IMG" $((l1_ofs + 7)) $(printf '\\x%02x' $((val & 0xfe)))", but 
that just doesn't look as nice)

> +
> +echo
> +# Should print the number of corruptions and leaks fixed
> +# (Filter out all JSON fields (recognizable by their four-space
> +# indentation), but keep the "-fixed" fields (by removing two spaces
> +# from their indentation))
> +# (Also filter out the L1 entry, because why not)
> +_check_test_img -r all --output=json \
> +    | sed -e 's/^  \(.*\)-fixed"/\1-fixed"/' \
> +    | grep -v '^    ' \
> +    | sed -e "s/\\<$(printf %x $l1_entry)\\>/L1_ENTRY_VALUE/"

sed | grep | sed can often be done with a single sed:

... | sed -e 's/^  \(.*\)-fixed"/\1-fixed"/' \
       -e '/^    /d' \
       -e "s/\\..."

Using \\< and \\> in the sed regex is a GNUism; do we want this test to 
run on BSD?

Reviewed-by: Eric Blake <address@hidden>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org