Re: [Qemu-block] [Qemu-devel] [RFC PATCH 06/12] qapi: add bitmap info

[John Snow]

On 05/14/2018 10:30 AM, Vladimir Sementsov-Ogievskiy wrote:
> 12.05.2018 04:25, John Snow wrote:
>> Add some of the necessary scaffolding for reporting bitmap information.
>>
>> Signed-off-by: John Snow <address@hidden>
>> ---
>>   qapi/block-core.json | 60
>> +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 59 insertions(+), 1 deletion(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index c50517bff3..8f33f41ce7 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -33,6 +33,61 @@
>>               'date-sec': 'int', 'date-nsec': 'int',
>>               'vm-clock-sec': 'int', 'vm-clock-nsec': 'int' } }
>>   +##
>> +# @BitmapTypeEnum:
>> +#
>> +# An enumeration of possible serialized bitmap types.
>> +#
>> +# @dirty-tracking: This bitmap records information on dirty
>> +#                  segments within the file.
>> +#
>> +# @unknown: This bitmap is of an unknown or reserved type.
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'enum': 'BitmapTypeEnum', 'data': [ 'dirty-tracking', 'unknown' ] }
>> +
>> +##
>> +# @BitmapFlagEnum:
>> +#
>> +# An enumeration of possible flags for serialized bitmaps.
>> +#
>> +# @in-use: This bitmap is considered to be in-use, and may now be
>> inconsistent.
>> +#
>> +# @auto: This bitmap must reflect any and all changes to the file it
>> describes.
>> +#        The type of this bitmap must be @DirtyTrackingBitmap.
> 
> logical, but I don't see this restriction in the spec. May be we need to
> update the spec
> 
1: auto

"The bitmap must reflect all changes of the virtual disk by any
application that would write to this qcow2 file (including writes,
snapshot switching, etc.). The type of this bitmap must be 'dirty
tracking bitmap'."

Actually, this looks correct now that I'm looking at the spec again.
I've used a terser phrasing but I think it's correct.

>> +#
>> +# @extra-data-compatible: This bitmap has extra information
>> associated with it.
> 
> no, this flag means, that extra data is compatible. So, if you don't
> know what is this extra data, you can read and modify the bitmap,
> leaving this data as is. If this flag is unset, and there are some extra
> data, bitmap must not be used.
> 
> Finally, this spec should be consistent (or, may be better, duplicate)
> spec from docs/interop/qcow2.txt..

I might suggest a rewrite of this portion of the spec as it's a little
unclear to me.

I've given this portion a rewrite.

>> +#
>> +# @unknown: This bitmap has unknown or reserved properties.
> 
> Better is only "reserved flags" (not unknown and not properties), they
> are reserved by spec.
> 
>> +#
>> +# Since: 2.13
>> +##
>> +{ 'enum': 'BitmapFlagEnum', 'data': [ 'in-use', 'auto',
>> +                                      'extra-data-compatible',
>> 'unknown' ] }
>> +
>> +##
>> +# @BitmapInfo:
>> +#
>> +# @name: The name of the bitmap.
>> +#
>> +# @type: The type of bitmap.
>> +#
>> +# @granularity: Bitmap granularity, in bytes.
>> +#
>> +# @count: Overall bitmap dirtiness, in bytes.
>> +#
>> +# @flags: Bitmap flags, if any.
>> +#
>> +# Since: 2.13
>> +#
>> +##
>> +{ 'struct': 'BitmapInfo',
>> +  'data': { 'name': 'str', 'type': 'BitmapTypeEnum', 'granularity':
>> 'int',
>> +            'count': 'int', '*flags': ['BitmapFlagEnum']
> 
> may be worth add 'has-extra-data'
> 
>> +  }
>> +}
>> +
>>   ##
>>   # @ImageInfoSpecificQCow2EncryptionBase:
>>   #
>> @@ -69,6 +124,8 @@
>>   # @encrypt: details about encryption parameters; only set if image
>>   #           is encrypted (since 2.10)
>>   #
>> +# @bitmaps: list of image bitmaps (since 2.13)
>> +#
>>   # Since: 1.7
>>   ##
>>   { 'struct': 'ImageInfoSpecificQCow2',
>> @@ -77,7 +134,8 @@
>>         '*lazy-refcounts': 'bool',
>>         '*corrupt': 'bool',
>>         'refcount-bits': 'int',
>> -      '*encrypt': 'ImageInfoSpecificQCow2Encryption'
>> +      '*encrypt': 'ImageInfoSpecificQCow2Encryption',
>> +      '*bitmaps': ['BitmapInfo']
>>     } }
>>     ##
> 
>