Re: [Qemu-devel] [PATCH 02/21] qcow2: Add refcount_width to format-specific info

[Eric Blake <eblake@redhat.com>]

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

On 11/10/2014 06:45 AM, Max Reitz wrote:
> Add the bit width of every refcount entry to the format-specific
> information.
> 
> This breaks some test outputs, fix them.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c              |  4 +++-
>  qapi/block-core.json       |  5 ++++-
>  tests/qemu-iotests/060.out |  1 +
>  tests/qemu-iotests/065     | 23 +++++++++++++++--------
>  tests/qemu-iotests/067.out | 10 +++++-----
>  tests/qemu-iotests/082.out |  7 +++++++
>  tests/qemu-iotests/089.out |  2 ++
>  7 files changed, 37 insertions(+), 15 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index f57aff9..d70e927 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2475,7 +2475,8 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
>      };
>      if (s->qcow_version == 2) {
>          *spec_info->qcow2 = (ImageInfoSpecificQCow2){
> -            .compat = g_strdup("0.10"),
> +            .compat             = g_strdup("0.10"),
> +            .refcount_width     = s->refcount_bits,

Hmm - is it really worth displaying a constant?  Since the 0.10 format
cannot change the width from 16, I'm not sure if it adds anything to the
output to display it.  After all, there's other things we omit for the
old format when they cannot be altered (such as the state of a lazy
flag).  On the other hand, if it makes your changes to later iotests
easier for tests that operate on both image formats, I'm not opposed to it.


> +++ b/tests/qemu-iotests/065
> @@ -88,34 +88,41 @@ class TestQMP(TestImageInfoSpecific):
>  class TestQCow2(TestQemuImgInfo):
>      '''Testing a qcow2 version 2 image'''
>      img_options = 'compat=0.10'
> -    json_compare = { 'compat': '0.10' }
> -    human_compare = [ 'compat: 0.10' ]
> +    json_compare = { 'compat': '0.10', 'refcount-width': 16 }
> +    human_compare = [ 'compat: 0.10', 'refcount width: 16' ]

This would be a test that does not change if you decide to not output
the constant.

> -{"return": [{"io-status": "ok", "device": "disk", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}
> +{"return": [{"io-status": "ok", "device": "disk", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "refcount-width": 16, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]}

It would be nice to figure out how to avoid long lines in the testsuite.
 But that's a project for another day.

If you can make a strong argument for always outputting the constant
width of 16 for 0.10 formats, then I can live with it, so:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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