On 10.12.2018 22:48, Eric Blake wrote: > On 12/10/18 12:50 PM, Vladimir Sementsov-Ogievskiy wrote: >> 10.12.2018 21:09, 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: >>> >> >> [...] >> >>> --- a/block/qcow2.c >>> +++ b/block/qcow2.c >>> @@ -4270,6 +4270,19 @@ static ImageInfoSpecific >>> *qcow2_get_specific_info(BlockDriverState *bs) >>> .refcount_bits = s->refcount_bits, >>> }; >>> } else if (s->qcow_version == 3) { >>> + bool has_bitmaps; >>> + Qcow2BitmapInfoList *bitmaps; >>> + Error *local_err = NULL; >>> + >>> + bitmaps = qcow2_get_bitmap_info_list(bs, &local_err); >>> + if (local_err) { >>> + /* TODO: Report the Error up to the caller when >>> implemented */ >>> + error_free(local_err); >>> + /* The 'bitmaps' empty list designates a failure to get >>> info */ >>> + has_bitmaps = true; > > I think you got it backwards. I would prefer has_bitmaps = false on > error,... > >>> + } else { >>> + has_bitmaps = !!bitmaps; > > ...and an empty list when you successfully determined that there are no > bitmaps. > > >>> +++ b/qapi/block-core.json >>> @@ -69,6 +69,11 @@ >>> # @encrypt: details about encryption parameters; only set if image >>> # is encrypted (since 2.10) >>> # >>> +# @bitmaps: list of qcow2 bitmaps details; the empty list designates >>> +# "fail to load bitmaps" if it is passed to the caller or >>> +# "no bitmaps" otherwise; >>> +# unsupported for the format QCOW2 v2 (since 4.0) >> >> >> For me, it looks simpler to declare alternative approach, assuming >> that absence >> of the field means error, like this: >> >> @bitmaps: optional field with uncommon semantics: >> Absence of the field means that bitmaps info query failed >> (which doesn't >> imply the whole query failure). >> If the field exists in query results, there were no >> errors, and it represents >> list of qcow2 bitmaps details. So, successful result will >> always use empty >> list to show that there are no bitmaps. >> Note: bitmaps are not supported before QCOW2 v3, so for >> elder versions >> @bitmaps will always be an empty list. > > I'd prefer: > > @bitmaps: A list of qcow2 bitmap details (possibly empty, such as for v2 > images which do not support bitmaps). Absent if bitmap > information could not be obtained. (since 4.0) >
Thank you all for your comments. The interpretation of an entity depends on an eye of the beholder. I am flexible on the discussed matter and will include your approach into v6 version. Thank you very much for your collaboration. -- With the best regards, Andrey Shinkevich