Re: [PULL 14/15] qcow2_format.py: dump bitmaps header extension

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 5425 bytes --]

On 09.06.20 22:52, Eric Blake wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> Add class for bitmap extension and dump its fields. Further work is to
> dump bitmap directory.
> 
> Test new functionality inside 291 iotest.

Unfortunately, it also breaks 291 with an external data file, which
worked before.  (The problems being that an external data file gives you
an additional header extension, and that the bitmap directory offset
changes.)

I think if we want to test testing tools, we have to do that in a
controlled environment where we exactly know what the image is.  It
looks to me now as if 291 is not such an environment.  Or phrased
differently, we probably shouldn’t test some testing tool in normal
tests that test qemu itself.

If we only test qcow2.py in normal tests, then I don’t think we have to
explicitly test it at all.  (Because if you test qcow2.py in a normal
test, and the test breaks, it’s unclear what’s broken.  So I think you
might as well forego the qcow2.py test altogether, because if it breaks,
that’ll probably show up in some other test case that uses it.)

In this case here, I can see three things we could do:

First, could just filter out the data file header extension and the
bitmap_directory_offset.  But I’m not sure whether that’s the best thing
to do, because it might break with some other obscure IMGOPTS that I
personally never use for the iotests.

I think that if we want a real qcow2.py test somewhere, it should be its
own test.  No custom IMGOPTS allowed.  So the second idea would be to
move it there, and drop the qcow2.py usage from here.

Or, third, maybe we actually don’t care that much about a real qcow2.py
test, and really want to just *use* (as opposed to “test”) qcow2.py
here.  Then we should filter what we really need from its output instead
of dropping what we don’t need.


(We could also disable 291 for external data files, but I don’t really
see why, if the only justification for this addition to it is to test
qcow2.py – which 291 isn’t for.)

Max

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> Message-Id: <20200606081806.23897-14-vsementsov@virtuozzo.com>
> [eblake: fix iotest output]
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/qemu-iotests/291             |  4 +++
>  tests/qemu-iotests/291.out         | 33 +++++++++++++++++++++++
>  tests/qemu-iotests/qcow2_format.py | 42 +++++++++++++++++++++++-------
>  3 files changed, 70 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/qemu-iotests/291 b/tests/qemu-iotests/291
> index 3ca83b9cd1f7..e0cffc7cb119 100755
> --- a/tests/qemu-iotests/291
> +++ b/tests/qemu-iotests/291
> @@ -62,6 +62,8 @@ $QEMU_IO -c 'w 1M 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io
>  $QEMU_IMG bitmap --disable -f $IMGFMT "$TEST_IMG" b1
>  $QEMU_IMG bitmap --enable -f $IMGFMT "$TEST_IMG" b2
>  $QEMU_IO -c 'w 2M 1M' -f $IMGFMT "$TEST_IMG" | _filter_qemu_io
> +echo "Check resulting qcow2 header extensions:"
> +$PYTHON qcow2.py "$TEST_IMG" dump-header-exts
> 
>  echo
>  echo "=== Bitmap preservation not possible to non-qcow2 ==="
> @@ -88,6 +90,8 @@ $QEMU_IMG bitmap --merge tmp -f $IMGFMT "$TEST_IMG" b0
>  $QEMU_IMG bitmap --remove --image-opts \
>      driver=$IMGFMT,file.driver=file,file.filename="$TEST_IMG" tmp
>  $QEMU_IMG info "$TEST_IMG" | _filter_img_info --format-specific
> +echo "Check resulting qcow2 header extensions:"
> +$PYTHON qcow2.py "$TEST_IMG" dump-header-exts
> 
>  echo
>  echo "=== Check bitmap contents ==="
> diff --git a/tests/qemu-iotests/291.out b/tests/qemu-iotests/291.out
> index 8c62017567e9..ccfcdc5e35ce 100644
> --- a/tests/qemu-iotests/291.out
> +++ b/tests/qemu-iotests/291.out
> @@ -14,6 +14,25 @@ wrote 1048576/1048576 bytes at offset 1048576
>  1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  wrote 1048576/1048576 bytes at offset 2097152
>  1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Check resulting qcow2 header extensions:
> +Header extension:
> +magic                     0xe2792aca (Backing format)
> +length                    5
> +data                      'qcow2'
> +
> +Header extension:
> +magic                     0x6803f857 (Feature table)
> +length                    336
> +data                      <binary>
> +
> +Header extension:
> +magic                     0x23852875 (Bitmaps)
> +length                    24
> +nb_bitmaps                2
> +reserved32                0
> +bitmap_directory_size     0x40
> +bitmap_directory_offset   0x510000
> +
> 
>  === Bitmap preservation not possible to non-qcow2 ===
> 
> @@ -65,6 +84,20 @@ Format specific information:
>              granularity: 65536
>      refcount bits: 16
>      corrupt: false
> +Check resulting qcow2 header extensions:
> +Header extension:
> +magic                     0x6803f857 (Feature table)
> +length                    336
> +data                      <binary>
> +
> +Header extension:
> +magic                     0x23852875 (Bitmaps)
> +length                    24
> +nb_bitmaps                3
> +reserved32                0
> +bitmap_directory_size     0x60
> +bitmap_directory_offset   0x520000
> +
> 
>  === Check bitmap contents ===
> 


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