Re: [Qemu-devel] [PATCH v12 08/10] qcow2: Optimize write zero of unaligned tail cluster

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2940 bytes --]

On 04.05.2017 05:07, Eric Blake wrote:
> We've already improved discards to operate efficiently on the tail
> of an unaligned qcow2 image; it's time to make a similar improvement
> to write zeroes.  The special case is only valid at the tail
> cluster of a file, where we must recognize that any sectors beyond
> the image end would implicitly read as zero, and therefore should
> not penalize our logic for widening a partial cluster into writing
> the whole cluster as zero.
> 
> However, note that for now, the special case of end-of-file is only
> recognized if there is no backing file, or if the backing file has
> the same length; that's because when the backing file is shorter
> than the active layer, we don't have code in place to recognize
> that reads of a sector unallocated at the top and beyond the backing
> end-of-file are implicitly zero.  It's not much of a real loss,
> because most people don't use images that aren't cluster-aligned,
> or where the active layer is a different size than the backing
> layer (especially where the difference falls within a single cluster).
> 
> Update test 154 to cover the new scenarios, using two images of
> intentionally differing length.
> 
> While at it, fix the test to gracefully skip when run as
> ./check -qcow2 -o compat=0.10 154
> since the older format lacks zero clusters already required earlier
> in the test.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> v12: fix testsuite problems, document shortcoming of differing
> v11: reserved for blkdebug half of v10
> size of backing file
> 
> v10: rebase to better reporting of preallocated zero clusters
> v9: new patch
> ---
>  block/qcow2.c              |   7 ++
>  tests/qemu-iotests/154     | 160 ++++++++++++++++++++++++++++++++++++++++++++-
>  tests/qemu-iotests/154.out | 129 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 294 insertions(+), 2 deletions(-)

[...]

> diff --git a/tests/qemu-iotests/154 b/tests/qemu-iotests/154
> index 7ca7219..687b8f3 100755
> --- a/tests/qemu-iotests/154
> +++ b/tests/qemu-iotests/154

[...]

> @@ -299,6 +302,159 @@ $QEMU_IO -c "read -P 0 75k 1k" "$TEST_IMG" | _filter_qemu_io
> 
>  $QEMU_IMG map --output=json "$TEST_IMG" | _filter_qemu_img_map
> 
> +echo
> +echo == unaligned image tail cluster, no allocation needed ==

[...]

> +# A preallocated cluster maintains its allocation, whether it stays as
> +# data due to a partial write:
> +# Convert 128m... | XX XX => ... | XX 00
> +_make_test_img $((size + 1024))
> +$QEMU_IO -c "write -P 1 $((size)) 1024" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "write -z $((size + 512)) 512" "$TEST_IMG.base" | _filter_qemu_io

s/\.base//, I suppose?

(You should read your reference output. "Pattern verification failed" is
never good. ;-))

With that (and the reference output) fixed:

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]