Re: [PATCH 1/2] qcow2: introduce Qcow2Metadata structure

[Eric Blake <eblake@redhat.com>]

On 12/27/19 5:43 AM, Andrey Shinkevich wrote:
> The preliminary patch to provide an extendable structure for dumping
> QCOW2 metadata allocations in image.
> 
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   qapi/block-core.json | 208 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 207 insertions(+), 1 deletion(-)
> 

> +
> +##
> +# @Qcow2BitmapTbl:
> +#

Any reason we must abbreviate instead of spelling this out as Table?

> +# QCOW2 bitmap table information
> +#
> +# @table-entries: list of bitmap table entries
> +#
> +# @location: bitmap table offset and size in image
> +#
> +# Since: 5.0
> +##
> +{ 'struct': 'Qcow2BitmapTbl',
> +  'data': {'table-entries': ['Qcow2BitmapTblEntry'],
> +           'location': 'Qcow2Allocation' } }
> +
> +##
> +# @Qcow2BitmapTblEntry:

Similar question


> +{ 'struct': 'Qcow2BitmapTblEntry',
> +  'data': {'type': 'Qcow2BitmapTblEntryType',
> +           '*cluster': 'Qcow2Allocation' } }
> +
> +##
> +# @Qcow2BitmapTblEntryType:
> +#
> +# An enumeration of cluster types in bitmap table
> +#
> +# @all-zeros: cluster should be read as all zeros

While there are multiple 'zeros' in the code base, 'zeroes' appears to 
be the more common spelling.


> @@ -215,6 +418,8 @@
>   #                       field is present if the driver for the image format
>   #                       supports it
>   #
> +# @viscera: encapsulates QCOW2 tables allocation information

Missing a '(since 5.0)' tag.  Interesting choice of name; not one I 
would have picked out of the air.  Would 'metadata' be any more of a 
reasonable name?

> +#
>   # Since: 1.4
>   #
>   ##
> @@ -223,7 +428,8 @@
>              '*image-end-offset': 'int', '*corruptions': 'int', '*leaks': 'int',
>              '*corruptions-fixed': 'int', '*leaks-fixed': 'int',
>              '*total-clusters': 'int', '*allocated-clusters': 'int',
> -           '*fragmented-clusters': 'int', '*compressed-clusters': 'int' } }
> +           '*fragmented-clusters': 'int', '*compressed-clusters': 'int',
> +           '*viscera': 'Qcow2Metadata' } }

The field is listed as optional, but the docs don't describe what 
controls whether it is present or absent.  Is that worth adding?


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