On 07.12.2018 19:20, Eric Blake wrote:
> On 12/7/18 4:00 AM, Andrey Shinkevich wrote:
>> In the 'Format specific information' section of the 'qemu-img info'
>> command output, the supplemental information about existing QCOW2
>> bitmaps will be shown, such as a bitmap name, flags and granularity:
>>
>> image: /vz/vmprivate/VM1/harddisk.hdd
>> file format: qcow2
>> virtual size: 64G (68719476736 bytes)
>> disk size: 3.0M
>> cluster_size: 1048576
>> Format specific information:
>> compat: 1.1
>> lazy refcounts: true
>> bitmaps:
>> [0]:
>> flags:
>> [0]: in-use
>> [1]: auto
>> name: back-up1
>> unknown flags: 4
>
> I'm guessing you doctored an image in a hex-editor to get this
> particular output?
>
>> granularity: 65536
>> [1]:
>> flags:
>> [0]: in-use
>> [1]: auto
>> name: back-up2
>> unknown flags: 8
>> granularity: 65536
>> refcount bits: 16
>> corrupt: false
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkev...@virtuozzo.com>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
>> ---
>> v4:
>> Unknown flags are checked with the mask BME_RESERVED_FLAGS.
>> The code minor refactoring was made.
>>
>
>> block/qcow2-bitmap.c | 56
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> block/qcow2.c | 8 ++++++++
>> block/qcow2.h | 2 ++
>> qapi/block-core.json | 40 ++++++++++++++++++++++++++++++++++++-
>> 4 files changed, 105 insertions(+), 1 deletion(-)
>
> I'm assuming John will merge this as a bitmap-related patch; make sure
> he is in cc if you send a v5 (adding now).
>
>> +++ b/block/qcow2.c
>> @@ -4270,6 +4270,12 @@ static ImageInfoSpecific
>> *qcow2_get_specific_info(BlockDriverState *bs)
>> .refcount_bits = s->refcount_bits,
>> };
>> } else if (s->qcow_version == 3) {
>> + Qcow2BitmapInfoList *bitmaps;
>> + Error *local_err = NULL;
>> + bitmaps = qcow2_get_bitmap_info_list(bs, &local_err);
>> + if (local_err != NULL) {
>> + error_report_err(local_err);
>> + }
>
> Ouch. Calling error_report_err() doesn't always work in QMP context;
> better would be to plumb Error **errp back up to the caller, if
> getting this specific information can fail and we want the overall
> query-block to fail. Or, we could decide that failure to get bitmap
> info is non-fatal, and that it was just a best-effort attempt to get
> more info, where we then ignore the failure, rather than trying to
> report it incorrectly.
>
>
>> +++ b/qapi/block-core.json
>> @@ -69,6 +69,8 @@
>> # @encrypt: details about encryption parameters; only set if image
>> # is encrypted (since 2.10)
>> #
>> +# @bitmaps: list of qcow2 bitmaps details (since 4.0)
>> +#
>> # Since: 1.7
>> ##
>> { 'struct': 'ImageInfoSpecificQCow2',
>> @@ -77,7 +79,8 @@
>> '*lazy-refcounts': 'bool',
>> '*corrupt': 'bool',
>> 'refcount-bits': 'int',
>> - '*encrypt': 'ImageInfoSpecificQCow2Encryption'
>> + '*encrypt': 'ImageInfoSpecificQCow2Encryption',
>> + '*bitmaps': ['Qcow2BitmapInfo']
>
> Hmm. You're omitting this field both if there are 0 bitmaps, and when
> it was a version 2 image. Is it worth including this field as a
> 0-length array when there are no bitmaps but when the image format is
> new enough to support them, or are we happy with the idea of only
> including the field when it has at least one bitmap? The difference is
> whether the calling app can explicitly learn that there are no bitmaps
> (0-length reply) vs. the ambiguity of omitting it from the reply
> (missing might mean no bitmaps, or an error in trying to report the
> bitmaps, or an older qemu that didn't know how to report bitmaps).
>
>
>> +##
>> +# @Qcow2BitmapInfo:
>> +#
>> +# Qcow2 bitmap information.
>> +#
>> +# @name: the name of the bitmap
>> +#
>> +# @granularity: granularity of the bitmap in bytes
>> +#
>> +# @flags: flags of the bitmap
>> +#
>> +# @unknown-flags: unspecified flags if detected
>> +#
>> +# Since: 4.0
>> +##
>> +{ 'struct': 'Qcow2BitmapInfo',
>> + 'data': {'name': 'str', 'granularity': 'uint32',
>> + 'flags': ['Qcow2BitmapInfoFlags'],
>> + '*unknown-flags': 'uint32' } }
>
> Here, you said flags will always be present, even if it is a 0-length
> array. Did you test the case where an on-disk bitmap has neither
> 'in-use' nor 'auto' set (where get_bitmap_info_flags() returns NULL)
> to ensure that it indeed results in a 0-length reply and not a crash?
In case of no flag, the output for a bitmap looks like this:
image: /vz/vmprivate/VM1/harddisk.hdd
file format: qcow2
virtual size: 64G (68719476736 bytes)
disk size: 3.0M
cluster_size: 1048576
Format specific information:
compat: 1.1
lazy refcounts: true
bitmaps:
[0]:
flags:
name: x
granularity: 65536
refcount bits: 16
corrupt: false
The output format is generated automatically based on the json syntax.
No crash happened.
>
> Otherwise, it's looking fairly good.
>
Kindly,
Andrey Shinkevich
