On 08/01/2020 01:07, Eric Blake wrote:
> 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 <vsement...@virtuozzo.com>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkev...@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?
>
Thank you Eric for your comments. The reason for the abbreviation is
that the name 'Qcow2BitmapTable' is in conflict with the existing
structure name in block/qcow2-bitmap.c. I can rename it to
'Qcow2BitmapTableInfo'
>> +# 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
>
and to 'Qcow2BitmapTableInfoEntry' here.
>
>> +{ '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?
>
>
--
With the best regards,
Andrey Shinkevich