[Qemu-devel] [PATCH v8 0/2] qemu-img info lists bitmap directory entries

[Qemu-devel] [PATCH v8 0/2] qemu-img info lists bitmap directory entries

[Andrey Shinkevich]

Here is the update for the bitmap extension of the qcow2 specific
information as the output of the 'qemu-img info' command.
The version #7 was in email with the message ID:
<1544698788-52893-1-git-send-email-andrey.shinkevich@virtuozzo.com>

v8:
The output benchmark files for the qemu-iotests, namely 060, 065 082, 198
and 206, were modified to show the bitmap extension for the qemu specific
information. A new test file 239 was added to the test set that checks the
output for the fields of the bitmap section.
The backward compatibility of the output for images of the version 2
of qcow2 was added.

v7:
A description was added to the function qcow2_get_bitmap_info_list().
In the function qcow2_get_specific_info(), the comment was modified
so that we ignore any error in obtaining the list of bitmaps to
pass the rest of QCOW2 specific information to a caller.

v6:
'[PATCH v6] qemu-img info lists bitmap directory entries'.
The error handling logic for the bitmaps empty list was reversed.

v5:
'[PATCH v5] qemu-img info lists bitmap directory entries'.
The error handling logic for the bitmaps empty list was fixed and documented.

v4:
'[PATCH v4] qemu-img info lists bitmap directory entries'.
Unknown flags are checked with the mask BME_RESERVED_FLAGS.
The code minor refactoring was made.

v3:
'[PATCH v3] qemu-img info lists bitmap directory entries'.
Now, qcow2_get_bitmap_info_list() is invoked under the condition of QCOW
version #3 to avoid memory leaks in case of QCOW version #2.
Furthermore, qcow2_get_bitmap_info_list() checks the number of existing bitmaps.
So, if no bitmap exists, no bitmap error message is printed in the output.
The data type of the bitmap 'granularity' parameter was left as 'uint32'
because bitmap_list_load() returns error if granularity_bits is grater than 31.

v2:
'[PATCH v2] qemu-img info lists bitmap directory entries'.
The targeted version of the release at 'Since' word of the comments to the new
structures changed to 4.0 in the file qapi/block-core.json.
A comment to the 'bitmaps' new member was supplied.
The 'unknown flags' parameter was introduced to indicate presence of QCOW2
bitmap unknown flags, if any.
The word 'dirty' was removed from the code and from the comments as we list all
the bitmaps.
The 'bitmaps' printed parameter was removed for the release versions earlier
than 3.x.
The example of the output was moved above the 'Signed-off-by' line.

The first version was '[PATCH] qemu-img info lists bitmap directory entries'.

Andrey Shinkevich (2):
  qemu-img info lists bitmap directory entries
  qemu-img info: bitmaps extension new test 239

 block/qapi.c               |  6 +++++
 block/qcow2-bitmap.c       | 64 ++++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c              | 13 ++++++++++
 block/qcow2.h              |  2 ++
 qapi/block-core.json       | 42 +++++++++++++++++++++++++++++-
 tests/qemu-iotests/060.out |  1 +
 tests/qemu-iotests/065     | 16 ++++++------
 tests/qemu-iotests/082.out |  7 +++++
 tests/qemu-iotests/198.out |  2 ++
 tests/qemu-iotests/206.out |  5 ++++
 tests/qemu-iotests/239     | 58 +++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/239.out | 33 ++++++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 13 files changed, 241 insertions(+), 9 deletions(-)
 create mode 100755 tests/qemu-iotests/239
 create mode 100644 tests/qemu-iotests/239.out

-- 
1.8.3.1

[Qemu-devel] [PATCH v8 1/2] qemu-img info lists bitmap directory entries

[Andrey Shinkevich]

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
            granularity: 65536
        [1]:
            flags:
                [0]: in-use
                [1]: auto
            name: back-up2
            unknown flags: 8
            granularity: 65536
    refcount bits: 16
    corrupt: false

As the print of the qcow2 specific information expanded by adding
the bitmap parameters to the 'qemu-img info' command output,
it requires amendment of the output benchmark in the following
tests: 060, 065, 082, 198, and 206.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block/qapi.c               |  6 +++++
 block/qcow2-bitmap.c       | 64 ++++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c              | 13 ++++++++++
 block/qcow2.h              |  2 ++
 qapi/block-core.json       | 42 +++++++++++++++++++++++++++++-
 tests/qemu-iotests/060.out |  1 +
 tests/qemu-iotests/065     | 16 ++++++------
 tests/qemu-iotests/082.out |  7 +++++
 tests/qemu-iotests/198.out |  2 ++
 tests/qemu-iotests/206.out |  5 ++++
 10 files changed, 149 insertions(+), 9 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index c66f949..0fde98c 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -38,6 +38,7 @@
 #include "qapi/qmp/qstring.h"
 #include "sysemu/block-backend.h"
 #include "qemu/cutils.h"
+#include "qemu/error-report.h"
 
 BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
                                         BlockDriverState *bs, Error **errp)
@@ -868,6 +869,11 @@ void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
 
     if (info->has_format_specific) {
         func_fprintf(f, "Format specific information:\n");
+        if (info->format_specific &&
+            info->format_specific->type == IMAGE_INFO_SPECIFIC_KIND_QCOW2 &&
+            info->format_specific->u.qcow2.data->has_bitmaps == false) {
+            warn_report("Failed to load bitmap list");
+        }
         bdrv_image_info_specific_dump(func_fprintf, f, info->format_specific);
     }
 }
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index b946301..ae842eb 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1006,6 +1006,70 @@ fail:
     return false;
 }
 
+static Qcow2BitmapInfoFlagsList *get_bitmap_info_flags(uint32_t flags)
+{
+    Qcow2BitmapInfoFlagsList *list = NULL;
+    Qcow2BitmapInfoFlagsList **plist = &list;
+
+    if (flags & BME_FLAG_IN_USE) {
+        Qcow2BitmapInfoFlagsList *entry = g_new0(Qcow2BitmapInfoFlagsList, 1);
+        entry->value = QCOW2_BITMAP_INFO_FLAGS_IN_USE;
+        *plist = entry;
+        plist = &entry->next;
+    }
+    if (flags & BME_FLAG_AUTO) {
+        Qcow2BitmapInfoFlagsList *entry = g_new0(Qcow2BitmapInfoFlagsList, 1);
+        entry->value = QCOW2_BITMAP_INFO_FLAGS_AUTO;
+        *plist = entry;
+    }
+    return list;
+}
+
+/*
+ * qcow2_get_bitmap_info_list()
+ * Returns a list of QCOW2 bitmap details.
+ * In case of no bitmaps, the function returns NULL and
+ * the @errp parameter is not set (for a 0-length list in the QMP).
+ * When bitmap information can not be obtained, the function returns
+ * NULL and the @errp parameter is set (for omitting the list in QMP).
+ */
+Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs,
+                                                Error **errp)
+{
+    BDRVQcow2State *s = bs->opaque;
+    Qcow2BitmapList *bm_list;
+    Qcow2Bitmap *bm;
+    Qcow2BitmapInfoList *list = NULL;
+    Qcow2BitmapInfoList **plist = &list;
+
+    if (s->nb_bitmaps == 0) {
+        return NULL;
+    }
+
+    bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
+                               s->bitmap_directory_size, errp);
+    if (bm_list == NULL) {
+        return NULL;
+    }
+
+    QSIMPLEQ_FOREACH(bm, bm_list, entry) {
+        Qcow2BitmapInfo *info = g_new0(Qcow2BitmapInfo, 1);
+        Qcow2BitmapInfoList *obj = g_new0(Qcow2BitmapInfoList, 1);
+        info->granularity = 1U << bm->granularity_bits;
+        info->name = g_strdup(bm->name);
+        info->flags = get_bitmap_info_flags(bm->flags);
+        info->unknown_flags = bm->flags & BME_RESERVED_FLAGS;
+        info->has_unknown_flags = !!info->unknown_flags;
+        obj->value = info;
+        *plist = obj;
+        plist = &obj->next;
+    }
+
+    bitmap_list_free(bm_list);
+
+    return list;
+}
+
 int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
                                  Error **errp)
 {
diff --git a/block/qcow2.c b/block/qcow2.c
index 4897aba..07b99ee 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4386,8 +4386,14 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
         *spec_info->u.qcow2.data = (ImageInfoSpecificQCow2){
             .compat             = g_strdup("0.10"),
             .refcount_bits      = s->refcount_bits,
+            .has_bitmaps        = true, /* To handle error check properly */
+            .bitmaps            = NULL, /* Unsupported for version 2 */
         };
     } else if (s->qcow_version == 3) {
+        Qcow2BitmapInfoList *bitmaps;
+        Error *local_err = NULL;
+
+        bitmaps = qcow2_get_bitmap_info_list(bs, &local_err);
         *spec_info->u.qcow2.data = (ImageInfoSpecificQCow2){
             .compat             = g_strdup("1.1"),
             .lazy_refcounts     = s->compatible_features &
@@ -4397,7 +4403,14 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
                                   QCOW2_INCOMPAT_CORRUPT,
             .has_corrupt        = true,
             .refcount_bits      = s->refcount_bits,
+            .has_bitmaps        = !local_err,
+            .bitmaps            = bitmaps,
         };
+        /*
+         * If an error occurs in obtaining bitmaps, ignore
+         * it to show other QCOW2 specific information.
+         */
+        error_free(local_err);
     } else {
         /* if this assertion fails, this probably means a new version was
          * added without having it covered here */
diff --git a/block/qcow2.h b/block/qcow2.h
index 438a1de..13e8964 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -684,6 +684,8 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
                                   void **refcount_table,
                                   int64_t *refcount_table_size);
 bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp);
+Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs,
+                                                Error **errp);
 int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
                                  Error **errp);
 int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 91685be..3352562 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -69,6 +69,10 @@
 # @encrypt: details about encryption parameters; only set if image
 #           is encrypted (since 2.10)
 #
+# @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)
+#
 # Since: 1.7
 ##
 { 'struct': 'ImageInfoSpecificQCow2',
@@ -77,7 +81,8 @@
       '*lazy-refcounts': 'bool',
       '*corrupt': 'bool',
       'refcount-bits': 'int',
-      '*encrypt': 'ImageInfoSpecificQCow2Encryption'
+      '*encrypt': 'ImageInfoSpecificQCow2Encryption',
+      '*bitmaps': ['Qcow2BitmapInfo']
   } }
 
 ##
@@ -454,6 +459,41 @@
            'status': 'DirtyBitmapStatus'} }
 
 ##
+# @Qcow2BitmapInfoFlags:
+#
+# An enumeration of flags that a bitmap can report to the user.
+#
+# @in-use: The bitmap was not saved correctly and may be inconsistent.
+#
+# @auto: The bitmap must reflect all changes of the virtual disk by any
+#        application that would write to this qcow2 file.
+#
+# Since: 4.0
+##
+{ 'enum': 'Qcow2BitmapInfoFlags',
+  'data': ['in-use', 'auto'] }
+
+##
+# @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' } }
+
+##
 # @BlockLatencyHistogramInfo:
 #
 # Block latency histogram.
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index af623cf..d0438b2 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -18,6 +18,7 @@ cluster_size: 65536
 Format specific information:
     compat: 1.1
     lazy refcounts: false
+    bitmaps:
     refcount bits: 16
     corrupt: true
 can't open device TEST_DIR/t.IMGFMT: IMGFMT: Image is corrupt; cannot be opened read/write
diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
index 8bac383..86406cb 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -88,23 +88,23 @@ class TestQMP(TestImageInfoSpecific):
 class TestQCow2(TestQemuImgInfo):
     '''Testing a qcow2 version 2 image'''
     img_options = 'compat=0.10'
-    json_compare = { 'compat': '0.10', 'refcount-bits': 16 }
-    human_compare = [ 'compat: 0.10', 'refcount bits: 16' ]
+    json_compare = { 'compat': '0.10', 'bitmaps': [], 'refcount-bits': 16 }
+    human_compare = [ 'compat: 0.10', 'bitmaps:', 'refcount bits: 16' ]
 
 class TestQCow3NotLazy(TestQemuImgInfo):
     '''Testing a qcow2 version 3 image with lazy refcounts disabled'''
     img_options = 'compat=1.1,lazy_refcounts=off'
-    json_compare = { 'compat': '1.1', 'lazy-refcounts': False,
+    json_compare = { 'compat': '1.1', 'lazy-refcounts': False, 'bitmaps': [],
                      'refcount-bits': 16, 'corrupt': False }
-    human_compare = [ 'compat: 1.1', 'lazy refcounts: false',
+    human_compare = [ 'compat: 1.1', 'lazy refcounts: false', 'bitmaps:',
                       'refcount bits: 16', 'corrupt: false' ]
 
 class TestQCow3Lazy(TestQemuImgInfo):
     '''Testing a qcow2 version 3 image with lazy refcounts enabled'''
     img_options = 'compat=1.1,lazy_refcounts=on'
-    json_compare = { 'compat': '1.1', 'lazy-refcounts': True,
+    json_compare = { 'compat': '1.1', 'lazy-refcounts': True, 'bitmaps': [],
                      'refcount-bits': 16, 'corrupt': False }
-    human_compare = [ 'compat: 1.1', 'lazy refcounts: true',
+    human_compare = [ 'compat: 1.1', 'lazy refcounts: true', 'bitmaps:',
                       'refcount bits: 16', 'corrupt: false' ]
 
 class TestQCow3NotLazyQMP(TestQMP):
@@ -112,7 +112,7 @@ class TestQCow3NotLazyQMP(TestQMP):
        with lazy refcounts enabled'''
     img_options = 'compat=1.1,lazy_refcounts=off'
     qemu_options = 'lazy-refcounts=on'
-    compare = { 'compat': '1.1', 'lazy-refcounts': False,
+    compare = { 'compat': '1.1', 'lazy-refcounts': False, 'bitmaps': [],
                 'refcount-bits': 16, 'corrupt': False }
 
 
@@ -121,7 +121,7 @@ class TestQCow3LazyQMP(TestQMP):
        with lazy refcounts disabled'''
     img_options = 'compat=1.1,lazy_refcounts=on'
     qemu_options = 'lazy-refcounts=off'
-    compare = { 'compat': '1.1', 'lazy-refcounts': True,
+    compare = { 'compat': '1.1', 'lazy-refcounts': True, 'bitmaps': [],
                 'refcount-bits': 16, 'corrupt': False }
 
 TestImageInfoSpecific = None
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 0ce18c0..cfae9c4 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -18,6 +18,7 @@ cluster_size: 4096
 Format specific information:
     compat: 1.1
     lazy refcounts: true
+    bitmaps:
     refcount bits: 16
     corrupt: false
 
@@ -30,6 +31,7 @@ cluster_size: 8192
 Format specific information:
     compat: 1.1
     lazy refcounts: true
+    bitmaps:
     refcount bits: 16
     corrupt: false
 
@@ -276,6 +278,7 @@ cluster_size: 4096
 Format specific information:
     compat: 1.1
     lazy refcounts: true
+    bitmaps:
     refcount bits: 16
     corrupt: false
 
@@ -287,6 +290,7 @@ cluster_size: 8192
 Format specific information:
     compat: 1.1
     lazy refcounts: true
+    bitmaps:
     refcount bits: 16
     corrupt: false
 
@@ -529,6 +533,7 @@ cluster_size: 65536
 Format specific information:
     compat: 1.1
     lazy refcounts: true
+    bitmaps:
     refcount bits: 16
     corrupt: false
 
@@ -540,6 +545,7 @@ cluster_size: 65536
 Format specific information:
     compat: 1.1
     lazy refcounts: false
+    bitmaps:
     refcount bits: 16
     corrupt: false
 
@@ -551,6 +557,7 @@ cluster_size: 65536
 Format specific information:
     compat: 1.1
     lazy refcounts: true
+    bitmaps:
     refcount bits: 16
     corrupt: false
 
diff --git a/tests/qemu-iotests/198.out b/tests/qemu-iotests/198.out
index adb805c..aeee3c6 100644
--- a/tests/qemu-iotests/198.out
+++ b/tests/qemu-iotests/198.out
@@ -36,6 +36,7 @@ image: json:{"encrypt.key-secret": "sec0", "driver": "IMGFMT", "file": {"driver"
 file format: IMGFMT
 virtual size: 16M (16777216 bytes)
 Format specific information:
+    bitmaps:
     encrypt:
         ivgen alg: plain64
         hash alg: sha256
@@ -79,6 +80,7 @@ file format: IMGFMT
 virtual size: 16M (16777216 bytes)
 backing file: TEST_DIR/t.IMGFMT.base
 Format specific information:
+    bitmaps:
     encrypt:
         ivgen alg: plain64
         hash alg: sha256
diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out
index 91f4db5..9758569 100644
--- a/tests/qemu-iotests/206.out
+++ b/tests/qemu-iotests/206.out
@@ -19,6 +19,7 @@ cluster_size: 65536
 Format specific information:
     compat: 1.1
     lazy refcounts: false
+    bitmaps:
     refcount bits: 16
     corrupt: false
 
@@ -41,6 +42,7 @@ cluster_size: 65536
 Format specific information:
     compat: 1.1
     lazy refcounts: false
+    bitmaps:
     refcount bits: 16
     corrupt: false
 
@@ -63,6 +65,7 @@ cluster_size: 2097152
 Format specific information:
     compat: 1.1
     lazy refcounts: true
+    bitmaps:
     refcount bits: 1
     corrupt: false
 
@@ -86,6 +89,7 @@ backing file: TEST_IMG.base
 backing file format: IMGFMT
 Format specific information:
     compat: 0.10
+    bitmaps:
     refcount bits: 16
 
 === Successful image creation (encrypted) ===
@@ -103,6 +107,7 @@ cluster_size: 65536
 Format specific information:
     compat: 1.1
     lazy refcounts: false
+    bitmaps:
     refcount bits: 16
     encrypt:
         ivgen alg: plain64
-- 
1.8.3.1

[Qemu-devel] [PATCH v8 2/2] qemu-img info: bitmaps extension new test 239

[Andrey Shinkevich]

A new test file 239 added to the qemu-iotests set. It checks
the output format of 'qemu-img info' for bitmaps extension of
qcow2 specific information.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/239     | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/239.out | 33 ++++++++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 92 insertions(+)
 create mode 100755 tests/qemu-iotests/239
 create mode 100644 tests/qemu-iotests/239.out

diff --git a/tests/qemu-iotests/239 b/tests/qemu-iotests/239
new file mode 100755
index 0000000..db05096
--- /dev/null
+++ b/tests/qemu-iotests/239
@@ -0,0 +1,58 @@
+#!/usr/bin/env python
+#
+# Test for qcow2 bitmap printed information
+#
+# Copyright (c) 2018 Virtuozzo International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import iotests
+import json
+from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
+                    file_path, log
+
+disk = file_path('disk')
+chunk = 256
+
+def add_bitmap(bitmap_number):
+    granularity = 2**(13 + bitmap_number)
+    bitmap_name = 'bitmap-' + str(bitmap_number)
+    vm = iotests.VM().add_drive(disk)
+    vm.launch()
+    vm.qmp_log('block-dirty-bitmap-add', node='drive0', name=bitmap_name,
+               granularity=granularity, persistent=True)
+    vm.shutdown()
+
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+
+qemu_img_create('-f', iotests.imgfmt, disk, '1M')
+
+
+for num in range(1, 4):
+    add_bitmap(num)
+    write = 'write {} {}K'.format((num-1)*chunk, chunk)
+    qemu_io('-f', iotests.imgfmt, '-c', write, disk)
+
+vm = iotests.VM().add_drive(disk)
+vm.launch()
+
+result = json.loads(qemu_img_pipe('info', '--force-share', '--output=json', disk))
+bitmaps = result['format-specific']['data']['bitmaps']
+
+vm.shutdown()
+
+log('bitmaps')
+log(bitmaps, indent=2)
diff --git a/tests/qemu-iotests/239.out b/tests/qemu-iotests/239.out
new file mode 100644
index 0000000..9085fc7
--- /dev/null
+++ b/tests/qemu-iotests/239.out
@@ -0,0 +1,33 @@
+{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 16384, "name": "bitmap-1", "node": "drive0", "persistent": true}}
+{"return": {}}
+{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 32768, "name": "bitmap-2", "node": "drive0", "persistent": true}}
+{"return": {}}
+{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 65536, "name": "bitmap-3", "node": "drive0", "persistent": true}}
+{"return": {}}
+bitmaps
+[
+  {
+    "flags": [
+      "in-use",
+      "auto"
+    ],
+    "granularity": 16384,
+    "name": "bitmap-1"
+  },
+  {
+    "flags": [
+      "in-use",
+      "auto"
+    ],
+    "granularity": 32768,
+    "name": "bitmap-2"
+  },
+  {
+    "flags": [
+      "in-use",
+      "auto"
+    ],
+    "granularity": 65536,
+    "name": "bitmap-3"
+  }
+]
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 0f1c3f9..3e310c7 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -235,3 +235,4 @@
 235 auto quick
 236 auto quick
 238 auto quick
+239 rw auto quick
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v8 2/2] qemu-img info: bitmaps extension new test 239

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]

On 1/28/19 12:40 PM, Andrey Shinkevich wrote:
> A new test file 239 added to the qemu-iotests set. It checks
> the output format of 'qemu-img info' for bitmaps extension of
> qcow2 specific information.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---

> +++ b/tests/qemu-iotests/239.out
> @@ -0,0 +1,33 @@
> +{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 16384, "name": "bitmap-1", "node": "drive0", "persistent": true}}
> +{"return": {}}
> +{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 32768, "name": "bitmap-2", "node": "drive0", "persistent": true}}
> +{"return": {}}
> +{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 65536, "name": "bitmap-3", "node": "drive0", "persistent": true}}

Should the test also create a disabled bitmap ("enabled":false), to show
the change in flags, and/or a non-persistent bitmap (to show that it
does not affect the qcow2 information, but only the query-blocks output,
because it is transient)?



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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH v8 2/2] qemu-img info: bitmaps extension new test 239

[Andrey Shinkevich]

On 28/01/2019 22:11, Eric Blake wrote:
> On 1/28/19 12:40 PM, Andrey Shinkevich wrote:
>> A new test file 239 added to the qemu-iotests set. It checks
>> the output format of 'qemu-img info' for bitmaps extension of
>> qcow2 specific information.
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
> 
>> +++ b/tests/qemu-iotests/239.out
>> @@ -0,0 +1,33 @@
>> +{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 16384, "name": "bitmap-1", "node": "drive0", "persistent": true}}
>> +{"return": {}}
>> +{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 32768, "name": "bitmap-2", "node": "drive0", "persistent": true}}
>> +{"return": {}}
>> +{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 65536, "name": "bitmap-3", "node": "drive0", "persistent": true}}
> 
> Should the test also create a disabled bitmap ("enabled":false), to show
> the change in flags, and/or a non-persistent bitmap (to show that it
> does not affect the qcow2 information, but only the query-blocks output,
> because it is transient)?
> 
I am about to release the version 9 with showing flag "in-use" in the 
test 239 and will think how to implement the ideas above.
Thank you Eric!
> 
> 

-- 
With the best regards,
Andrey Shinkevich

Re: [Qemu-devel] [PATCH v8 0/2] qemu-img info lists bitmap directory entries

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]

On 1/28/19 12:40 PM, Andrey Shinkevich wrote:
> Here is the update for the bitmap extension of the qcow2 specific
> information as the output of the 'qemu-img info' command.
> The version #7 was in email with the message ID:
> <1544698788-52893-1-git-send-email-andrey.shinkevich@virtuozzo.com>
> 
> v8:
> The output benchmark files for the qemu-iotests, namely 060, 065 082, 198
> and 206, were modified to show the bitmap extension for the qemu specific
> information. A new test file 239 was added to the test set that checks the
> output for the fields of the bitmap section.
> The backward compatibility of the output for images of the version 2
> of qcow2 was added.

So, I'm trying to test this, and I've discovered something rather
annoying about persistent snapshots: they DON'T get written to disk
until the qemu process exits.  In other words, even after creating a
persistent bitmap via QMP (I'm trying to debug my libvirt API for
incremental snapshots, so I did this via 'virsh snapshot-create-as $dom
name', but it boils down to a QMP transaction with
'block-dirty-bitmap-add' as one of the commands), running:

$ qemu-img info -U Active1.qcow2

shows
    bitmaps:
    refcount bits: 16

for as long as the qemu process is running. What does it take to force a
qcow2 image to write out its current set of known bitmaps to disk, even
if they are known to be potentially dirty? Should it happen any time we
flush L1/L2/refcount tables?  On a periodic but slow timer (say once
every 5 minutes)? Other ideas?  I know we don't want to be flushing the
full bitmap contents on every change to the in-memory internal bitmap,
but meta-changes (such as adding a bitmap) seem like the sort of thing
where it's worth flushing the qcow2 file; particularly when adding the
bitmap via QMP 'transaction' which goes to the bother of flushing all
pending I/O in order to properly roll back on failure (which means on
success, it would be nice for the new bitmap name to show up in the
qcow2 header, even if the contents of the bitmap are not up-to-date).

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH v8 0/2] qemu-img info lists bitmap directory entries

[Vladimir Sementsov-Ogievskiy]

30.01.2019 7:01, Eric Blake wrote:
> On 1/28/19 12:40 PM, Andrey Shinkevich wrote:
>> Here is the update for the bitmap extension of the qcow2 specific
>> information as the output of the 'qemu-img info' command.
>> The version #7 was in email with the message ID:
>> <1544698788-52893-1-git-send-email-andrey.shinkevich@virtuozzo.com>
>>
>> v8:
>> The output benchmark files for the qemu-iotests, namely 060, 065 082, 198
>> and 206, were modified to show the bitmap extension for the qemu specific
>> information. A new test file 239 was added to the test set that checks the
>> output for the fields of the bitmap section.
>> The backward compatibility of the output for images of the version 2
>> of qcow2 was added.
> 
> So, I'm trying to test this, and I've discovered something rather
> annoying about persistent snapshots: they DON'T get written to disk
> until the qemu process exits.  In other words, even after creating a
> persistent bitmap via QMP (I'm trying to debug my libvirt API for
> incremental snapshots, so I did this via 'virsh snapshot-create-as $dom
> name', but it boils down to a QMP transaction with
> 'block-dirty-bitmap-add' as one of the commands), running:
> 
> $ qemu-img info -U Active1.qcow2
> 
> shows
>      bitmaps:
>      refcount bits: 16
> 
> for as long as the qemu process is running. What does it take to force a
> qcow2 image to write out its current set of known bitmaps to disk, even
> if they are known to be potentially dirty? Should it happen any time we
> flush L1/L2/refcount tables?  On a periodic but slow timer (say once
> every 5 minutes)? Other ideas?  I know we don't want to be flushing the
> full bitmap contents on every change to the in-memory internal bitmap,
> but meta-changes (such as adding a bitmap) seem like the sort of thing
> where it's worth flushing the qcow2 file; particularly when adding the
> bitmap via QMP 'transaction' which goes to the bother of flushing all
> pending I/O in order to properly roll back on failure (which means on
> success, it would be nice for the new bitmap name to show up in the
> qcow2 header, even if the contents of the bitmap are not up-to-date).
> 

But what is the benefit of it, except qemu-img info with --force-share option,
which of course is not guaranteed to show valid metadata?

While qemu is running valid way to obtain info is qmp. Do libvirt call qemu-img
--fore-share?


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v8 0/2] qemu-img info lists bitmap directory entries

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2198 bytes --]

On 1/30/19 2:00 AM, Vladimir Sementsov-Ogievskiy wrote:

>> So, I'm trying to test this, and I've discovered something rather
>> annoying about persistent snapshots: they DON'T get written to disk
>> until the qemu process exits.  In other words, even after creating a
>> persistent bitmap via QMP (I'm trying to debug my libvirt API for
>> incremental snapshots, so I did this via 'virsh snapshot-create-as $dom
>> name', but it boils down to a QMP transaction with
>> 'block-dirty-bitmap-add' as one of the commands), running:
>>
>> $ qemu-img info -U Active1.qcow2
>>
>> shows
>>      bitmaps:
>>      refcount bits: 16
>>
>> for as long as the qemu process is running.

> 
> But what is the benefit of it, except qemu-img info with --force-share option,
> which of course is not guaranteed to show valid metadata?
> 
> While qemu is running valid way to obtain info is qmp. Do libvirt call qemu-img
> --fore-share?

You're right that libvirt will be using QMP and not qemu-img. But that
does not prevent other clients from using qemu-img on a file, regardless
of whether the file is in use by a guest. There's also the argument that
if qemu dies suddenly (most likely due to a bug or to a host being
fenced), knowing what bitmaps were present in the image is better than
having nothing at all (but conversely, if qemu dies suddenly, you are
likely missing the bitmap data for the most recent changes, so even if
disabled bitmaps are flushed and no longer marked in-use, they still
won't be sufficient to allow an incremental backup, and a full backup
will be necessary regardless of how much or little bitmap information
got persisted).

While you're right that --force-share is not required to show up-to-date
metadata, it also is not required to show nothing at all.  And at least
knowing that a persistent bitmap is associated with a qcow2 file may
make other things obvious - such as the fact that the image can't be
resized (until we implement the functionality to support resize and
bitmaps in the same image).


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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH v8 0/2] qemu-img info lists bitmap directory entries

[Vladimir Sementsov-Ogievskiy]

30.01.2019 15:43, Eric Blake wrote:
> On 1/30/19 2:00 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
>>> So, I'm trying to test this, and I've discovered something rather
>>> annoying about persistent snapshots: they DON'T get written to disk
>>> until the qemu process exits.  In other words, even after creating a
>>> persistent bitmap via QMP (I'm trying to debug my libvirt API for
>>> incremental snapshots, so I did this via 'virsh snapshot-create-as $dom
>>> name', but it boils down to a QMP transaction with
>>> 'block-dirty-bitmap-add' as one of the commands), running:
>>>
>>> $ qemu-img info -U Active1.qcow2
>>>
>>> shows
>>>       bitmaps:
>>>       refcount bits: 16
>>>
>>> for as long as the qemu process is running.
> 
>>
>> But what is the benefit of it, except qemu-img info with --force-share option,
>> which of course is not guaranteed to show valid metadata?
>>
>> While qemu is running valid way to obtain info is qmp. Do libvirt call qemu-img
>> --fore-share?
> 
> You're right that libvirt will be using QMP and not qemu-img. But that
> does not prevent other clients from using qemu-img on a file

I just thing that using --force-share in production is wrong thing to do. So,
it is needed only for debugging.. And, then, flushing bitmaps may be needed
only for debugging. So, why to flush in production?

>, regardless
> of whether the file is in use by a guest. There's also the argument that
> if qemu dies suddenly (most likely due to a bug or to a host being
> fenced), knowing what bitmaps were present in the image

actually, they didn't. They've never been in the image, only in RAM.. Like
non-persistent bitmaps.

  is better than
> having nothing at all (but conversely, if qemu dies suddenly, you are
> likely missing the bitmap data for the most recent changes, so even if
> disabled bitmaps are flushed and no longer marked in-use, they still
> won't be sufficient to allow an incremental backup, and a full backup
> will be necessary regardless of how much or little bitmap information
> got persisted).

exactly. But yes, flushing disabled bitmaps may theoretically make sense, as
they are at least valid. But about disabled bitmaps, I think it would be more
useful to teach Qemu load/unload them on demand, to not use extra RAM when
they are not needed (most of the time they don't).

> 
> While you're right that --force-share is not required to show up-to-date
> metadata, it also is not required to show nothing at all.  And at least
> knowing that a persistent bitmap is associated with a qcow2 file may
> make other things obvious - such as the fact that the image can't be
> resized (until we implement the functionality to support resize and
> bitmaps in the same image).

Hm, yes, if we have invalid IN_USE bitmaps in the image, we can't resize it..
But is it a good reason to store these bitmaps? If we don't store them,
image could be resized, why not (of course, if it is not corrupted, as
image was not closed properly)..


It all depends on the meaning of the 'persistent' bitmap. For me,
persistent means 'bitmap will be saved to the image on close'.
And they are implemented in this way. And documented in this way too:
# @persistent: the bitmap is persistent, i.e. it will be saved to the
#              corresponding block device image file on its close.

But you see them as "bitmaps which exist in the image".

I don't see real reason to flush them while Qemu running. --force-share is a back-door,
it should not be used, and I doubt that implementing something
especially for --force-share is a good idea.
Then, the only difference, is that we'll see IN_USE bitmaps in the
image if qemu stopped unexpectedly. And the only meaningful thing to
do with these invalid bitmaps is removing them.


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v8 0/2] qemu-img info lists bitmap directory entries

[John Snow]

On 1/29/19 11:01 PM, Eric Blake wrote:
> On 1/28/19 12:40 PM, Andrey Shinkevich wrote:
>> Here is the update for the bitmap extension of the qcow2 specific
>> information as the output of the 'qemu-img info' command.
>> The version #7 was in email with the message ID:
>> <1544698788-52893-1-git-send-email-andrey.shinkevich@virtuozzo.com>
>>
>> v8:
>> The output benchmark files for the qemu-iotests, namely 060, 065 082, 198
>> and 206, were modified to show the bitmap extension for the qemu specific
>> information. A new test file 239 was added to the test set that checks the
>> output for the fields of the bitmap section.
>> The backward compatibility of the output for images of the version 2
>> of qcow2 was added.
> 
> So, I'm trying to test this, and I've discovered something rather
> annoying about persistent snapshots: they DON'T get written to disk
> until the qemu process exits.  In other words, even after creating a

Technically, they get written out on qcow2_inactivate -- so there is the
opportunity to write them out at key sync points if you want.

However, like Vladimir says, this data is going to be necessarily wrong
while the image is in-use, so there may not be great value in it. There
may be a lot of anti-value if users learn to use or rely on that data.

I suppose the debugging case you want to assist here is one of trust for
users -- if a user wants to take a quick poke at the image and see if
the persistent bitmap tracking really got enabled before trusting it for
the next week of operations.

> persistent bitmap via QMP (I'm trying to debug my libvirt API for
> incremental snapshots, so I did this via 'virsh snapshot-create-as $dom
> name', but it boils down to a QMP transaction with
> 'block-dirty-bitmap-add' as one of the commands), running:
> 
> $ qemu-img info -U Active1.qcow2
> 
> shows
>     bitmaps:
>     refcount bits: 16
> 
> for as long as the qemu process is running. What does it take to force a
> qcow2 image to write out its current set of known bitmaps to disk, even
> if they are known to be potentially dirty? Should it happen any time we
> flush L1/L2/refcount tables?  On a periodic but slow timer (say once
> every 5 minutes)? Other ideas?  I know we don't want to be flushing the
> full bitmap contents on every change to the in-memory internal bitmap,
> but meta-changes (such as adding a bitmap) seem like the sort of thing
> where it's worth flushing the qcow2 file; particularly when adding the
> bitmap via QMP 'transaction' which goes to the bother of flushing all
> pending I/O in order to properly roll back on failure (which means on
> success, it would be nice for the new bitmap name to show up in the
> qcow2 header, even if the contents of the bitmap are not up-to-date).
>