[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v5] qemu-img info lists bitmap directory entries

From: Eric Blake
Subject: Re: [Qemu-block] [PATCH v5] qemu-img info lists bitmap directory entries
Date: Mon, 10 Dec 2018 13:48:10 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1

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 
            imply the whole query failure).
            If the field exists in query results, there were no errors, and it 
            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 
            @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)

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]