[Qemu-devel] [PATCH v14 0/3] qcow2: Add list of bitmaps to ImageInfoSpecificQCow2

[Qemu-devel] [PATCH v14 0/3] qcow2: Add list of bitmaps to ImageInfoSpecificQCow2

[Andrey Shinkevich]

v14:
The test 242 was amended to be safer against changes in the bitmap allocation
algorithm because the bitmap directory is reallocated on VM shutdown.
Unfortunately, an empty line at the end of the benchmark file 242.out persists.
The version #13 was discussed in email thread with the message ID:
<1549448589-381285-1-git-send-email-andrey.shinkevich@virtuozzo.com>

v13:
The enhancement of the Python code style for fopen() in the new test 242.
The 'unknown flag' was actually removed from the structure Qcow2BitmapInfo.

v12:
In the function block_crypto_get_specific_info_luks(),
checking the format was replaced with assertion.
The 'unknown flag' was removed from the structure Qcow2BitmapInfo.
A new case added to the test file 242 to check QEMU behavior in case of
unknown flag in a bitmap directory entry of QCOW2 image file.

v11:
An assertion was added to the get_bitmap_info_flags() to check the
completed mapping of all the reserved bitmap BME_ flags.
The heading comment of get_bitmap_info_flags() was changed to
describe the function design properly.
In qcow2_get_specific_info(), two function calls g_free() were
replaced with one call to qapi_free_ImageInfoSpecific() that does
all the cleaning work.
The version #11 was discussed in email thread with the message ID:
<1548942405-760115-1-git-send-email-andrey.shinkevich@virtuozzo.com>

v10:
The 'struct Error' parameter was added to the function prototype
bdrv_query_image_info().
The code refactoring of the function get_bitmap_info_flags().
The comments to the structures ImageInfoSpecificQCow2 and
Qcow2BitmapInfo in the file qapi/block-core.json were corrected.
The changes in the *.out files of the tests 060, 065 082, 198
and 206 were discarded. The new test 239 was enriched by adding human
readable format output and by checking the output with bitmap extra
parameters, such as  non-persistent and disabled.
The version #10 was discussed in email thread with the message ID:
<1548870690-647481-1-git-send-email-andrey.shinkevich@virtuozzo.com>

v9:
The new test 239 of the qemu-iotests set was amended to show the bitmaps
being added and to demonstrate the bitmap flag "in-use".
The version #9 was discussed with the message ID:
<1548705688-1027522-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.
The version #8 was discussed in email thread with the message ID:
<1548700805-1016533-1-git-send-email-andrey.shinkevich@virtuozzo.com>

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.
The version #7 was discussed in email thread with the message ID:
<1544698788-52893-1-git-send-email-andrey.shinkevich@virtuozzo.com>

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 (3):
  bdrv_query_image_info Error parameter added
  qcow2: Add list of bitmaps to ImageInfoSpecificQCow2
  qcow2: list of bitmaps new test 242

 block.c                    |   5 +-
 block/crypto.c             |   9 +--
 block/qapi.c               |   7 +-
 block/qcow2-bitmap.c       |  76 +++++++++++++++++++++
 block/qcow2.c              |  21 +++++-
 block/qcow2.h              |   2 +
 block/vmdk.c               |   3 +-
 include/block/block.h      |   3 +-
 include/block/block_int.h  |   3 +-
 qapi/block-core.json       |  41 ++++++++++-
 qemu-io-cmds.c             |   7 +-
 tests/qemu-iotests/242     | 103 ++++++++++++++++++++++++++++
 tests/qemu-iotests/242.out | 165 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |   1 +
 14 files changed, 429 insertions(+), 17 deletions(-)
 create mode 100755 tests/qemu-iotests/242
 create mode 100644 tests/qemu-iotests/242.out

-- 
1.8.3.1

[Qemu-devel] [PATCH v14 1/3] bdrv_query_image_info Error parameter added

[Andrey Shinkevich]

Inform a user in case qcow2_get_specific_info fails to obtain
QCOW2 image specific information. This patch is preliminary to
the one "qcow2: Add list of bitmaps to ImageInfoSpecificQCow2".

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                   |  5 +++--
 block/crypto.c            |  9 +++------
 block/qapi.c              |  7 ++++++-
 block/qcow2.c             | 10 ++++++++--
 block/vmdk.c              |  3 ++-
 include/block/block.h     |  3 ++-
 include/block/block_int.h |  3 ++-
 qemu-io-cmds.c            |  7 ++++++-
 8 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index 4f5ff2c..1eb35ef 100644
--- a/block.c
+++ b/block.c
@@ -4307,11 +4307,12 @@ int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     return drv->bdrv_get_info(bs, bdi);
 }
 
-ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs)
+ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs,
+                                          Error **errp)
 {
     BlockDriver *drv = bs->drv;
     if (drv && drv->bdrv_get_specific_info) {
-        return drv->bdrv_get_specific_info(bs);
+        return drv->bdrv_get_specific_info(bs, errp);
     }
     return NULL;
 }
diff --git a/block/crypto.c b/block/crypto.c
index f0a5f6b..d5b1da6 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -594,20 +594,17 @@ static int block_crypto_get_info_luks(BlockDriverState *bs,
 }
 
 static ImageInfoSpecific *
-block_crypto_get_specific_info_luks(BlockDriverState *bs)
+block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp)
 {
     BlockCrypto *crypto = bs->opaque;
     ImageInfoSpecific *spec_info;
     QCryptoBlockInfo *info;
 
-    info = qcrypto_block_get_info(crypto->block, NULL);
+    info = qcrypto_block_get_info(crypto->block, errp);
     if (!info) {
         return NULL;
     }
-    if (info->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
-        qapi_free_QCryptoBlockInfo(info);
-        return NULL;
-    }
+    assert(info->format == Q_CRYPTO_BLOCK_FORMAT_LUKS);
 
     spec_info = g_new(ImageInfoSpecific, 1);
     spec_info->type = IMAGE_INFO_SPECIFIC_KIND_LUKS;
diff --git a/block/qapi.c b/block/qapi.c
index c66f949..00291f9 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -282,7 +282,12 @@ void bdrv_query_image_info(BlockDriverState *bs,
         info->dirty_flag = bdi.is_dirty;
         info->has_dirty_flag = true;
     }
-    info->format_specific     = bdrv_get_specific_info(bs);
+    info->format_specific = bdrv_get_specific_info(bs, &err);
+    if (err) {
+        error_propagate(errp, err);
+        qapi_free_ImageInfo(info);
+        goto out;
+    }
     info->has_format_specific = info->format_specific != NULL;
 
     backing_filename = bs->backing_file;
diff --git a/block/qcow2.c b/block/qcow2.c
index 4897aba..27e5a2c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4367,14 +4367,20 @@ static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     return 0;
 }
 
-static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
+static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs,
+                                                  Error **errp)
 {
     BDRVQcow2State *s = bs->opaque;
     ImageInfoSpecific *spec_info;
     QCryptoBlockInfo *encrypt_info = NULL;
+    Error *local_err = NULL;
 
     if (s->crypto != NULL) {
-        encrypt_info = qcrypto_block_get_info(s->crypto, &error_abort);
+        encrypt_info = qcrypto_block_get_info(s->crypto, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return NULL;
+        }
     }
 
     spec_info = g_new(ImageInfoSpecific, 1);
diff --git a/block/vmdk.c b/block/vmdk.c
index 2c9e86d..544c10d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2314,7 +2314,8 @@ static int coroutine_fn vmdk_co_check(BlockDriverState *bs,
     return ret;
 }
 
-static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs)
+static ImageInfoSpecific *vmdk_get_specific_info(BlockDriverState *bs,
+                                                 Error **errp)
 {
     int i;
     BDRVVmdkState *s = bs->opaque;
diff --git a/include/block/block.h b/include/block/block.h
index f70a843..9899c24 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -477,7 +477,8 @@ const char *bdrv_get_device_name(const BlockDriverState *bs);
 const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
 int bdrv_get_flags(BlockDriverState *bs);
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
-ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs);
+ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs,
+                                          Error **errp);
 void bdrv_round_to_clusters(BlockDriverState *bs,
                             int64_t offset, int64_t bytes,
                             int64_t *cluster_offset,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f605622..0075baf 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -319,7 +319,8 @@ struct BlockDriver {
                                   const char *name,
                                   Error **errp);
     int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
-    ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs);
+    ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs,
+                                                 Error **errp);
 
     int coroutine_fn (*bdrv_save_vmstate)(BlockDriverState *bs,
                                           QEMUIOVector *qiov,
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 2c39124..2187036 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1661,6 +1661,7 @@ static int info_f(BlockBackend *blk, int argc, char **argv)
     BlockDriverState *bs = blk_bs(blk);
     BlockDriverInfo bdi;
     ImageInfoSpecific *spec_info;
+    Error *local_err = NULL;
     char s1[64], s2[64];
     int ret;
 
@@ -1682,7 +1683,11 @@ static int info_f(BlockBackend *blk, int argc, char **argv)
     printf("cluster size: %s\n", s1);
     printf("vm state offset: %s\n", s2);
 
-    spec_info = bdrv_get_specific_info(bs);
+    spec_info = bdrv_get_specific_info(bs, &local_err);
+    if (local_err) {
+        error_report_err(local_err);
+        return -EIO;
+    }
     if (spec_info) {
         printf("Format specific information:\n");
         bdrv_image_info_specific_dump(fprintf, stdout, spec_info);
-- 
1.8.3.1

[Qemu-devel] [PATCH v14 2/3] qcow2: Add list of bitmaps to ImageInfoSpecificQCow2

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

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-bitmap.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c        | 11 +++++++-
 block/qcow2.h        |  2 ++
 qapi/block-core.json | 41 +++++++++++++++++++++++++++-
 4 files changed, 128 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index b946301..3ee524d 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -1006,6 +1006,82 @@ fail:
     return false;
 }
 
+
+static Qcow2BitmapInfoFlagsList *get_bitmap_info_flags(uint32_t flags)
+{
+    Qcow2BitmapInfoFlagsList *list = NULL;
+    Qcow2BitmapInfoFlagsList **plist = &list;
+    int i;
+
+    static const struct {
+        int bme;  /* Bitmap directory entry flags */
+        int info; /* The flags to report to the user */
+    } map[] = {
+        { BME_FLAG_IN_USE, QCOW2_BITMAP_INFO_FLAGS_IN_USE },
+        { BME_FLAG_AUTO,   QCOW2_BITMAP_INFO_FLAGS_AUTO },
+    };
+
+    int map_size = ARRAY_SIZE(map);
+
+    for (i = 0; i < map_size; ++i) {
+        if (flags & map[i].bme) {
+            Qcow2BitmapInfoFlagsList *entry =
+                g_new0(Qcow2BitmapInfoFlagsList, 1);
+            entry->value = map[i].info;
+            *plist = entry;
+            plist = &entry->next;
+            flags &= ~map[i].bme;
+        }
+    }
+    /* Check if the BME_* mapping above is complete */
+    assert(!flags);
+
+    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.
+ * When bitmap information can not be obtained, the function returns
+ * NULL and the @errp parameter is set.
+ */
+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 & ~BME_RESERVED_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 27e5a2c..a5607f1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4386,7 +4386,7 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs,
     spec_info = g_new(ImageInfoSpecific, 1);
     *spec_info = (ImageInfoSpecific){
         .type  = IMAGE_INFO_SPECIFIC_KIND_QCOW2,
-        .u.qcow2.data = g_new(ImageInfoSpecificQCow2, 1),
+        .u.qcow2.data = g_new0(ImageInfoSpecificQCow2, 1),
     };
     if (s->qcow_version == 2) {
         *spec_info->u.qcow2.data = (ImageInfoSpecificQCow2){
@@ -4394,6 +4394,13 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs,
             .refcount_bits      = s->refcount_bits,
         };
     } else if (s->qcow_version == 3) {
+        Qcow2BitmapInfoList *bitmaps;
+        bitmaps = qcow2_get_bitmap_info_list(bs, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            qapi_free_ImageInfoSpecific(spec_info);
+            return NULL;
+        }
         *spec_info->u.qcow2.data = (ImageInfoSpecificQCow2){
             .compat             = g_strdup("1.1"),
             .lazy_refcounts     = s->compatible_features &
@@ -4403,6 +4410,8 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs,
                                   QCOW2_INCOMPAT_CORRUPT,
             .has_corrupt        = true,
             .refcount_bits      = s->refcount_bits,
+            .has_bitmaps        = !!bitmaps,
+            .bitmaps            = bitmaps,
         };
     } else {
         /* if this assertion fails, this probably means a new version was
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..683215c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -69,6 +69,8 @@
 # @encrypt: details about encryption parameters; only set if image
 #           is encrypted (since 2.10)
 #
+# @bitmaps: A list of qcow2 bitmap 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']
   } }
 
 ##
@@ -454,6 +457,42 @@
            'status': 'DirtyBitmapStatus'} }
 
 ##
+# @Qcow2BitmapInfoFlags:
+#
+# An enumeration of flags that a bitmap can report to the user.
+#
+# @in-use: This flag is set by any process actively modifying the qcow2 file,
+#          and cleared when the updated bitmap is flushed to the qcow2 image.
+#          The presence of this flag in an offline image means that the bitmap
+#          was not saved correctly after its last usage, and may contain
+#          inconsistent data.
+#
+# @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
+#
+# Since: 4.0
+##
+{ 'struct': 'Qcow2BitmapInfo',
+  'data': {'name': 'str', 'granularity': 'uint32',
+           'flags': ['Qcow2BitmapInfoFlags'] } }
+
+##
 # @BlockLatencyHistogramInfo:
 #
 # Block latency histogram.
-- 
1.8.3.1

[Qemu-devel] [PATCH v14 3/3] qcow2: list of bitmaps new test 242

[Andrey Shinkevich]

A new test file 242 added to the qemu-iotests set. It checks
the format of qcow2 specific information for the new added
section that lists details of bitmaps.

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

diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242
new file mode 100755
index 0000000..75f05bc
--- /dev/null
+++ b/tests/qemu-iotests/242
@@ -0,0 +1,103 @@
+#!/usr/bin/env python
+#
+# Test for qcow2 bitmap printed information
+#
+# Copyright (c) 2019 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, img_info_log, log, filter_qemu_io
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+
+disk = file_path('disk')
+chunk = 256*1024
+bitmap_flag_unknown = 1 << 2
+#flag_offset = 5*cluster_size + flag_offset_in_bitmap_directory_entry
+flag_offset = 0x5000f
+
+
+def print_bitmap(extra_args):
+    log('qemu-img info dump:\n')
+    img_info_log(disk, extra_args=extra_args)
+    result = json.loads(qemu_img_pipe('info', '--force-share',
+                                      '--output=json', disk))
+    if 'bitmaps' in result['format-specific']['data']:
+        bitmaps = result['format-specific']['data']['bitmaps']
+        log('The same bitmaps in JSON format:')
+        log(bitmaps, indent=2)
+    else:
+        log('No bitmap in JSON format output')
+
+
+def add_bitmap(bitmap_number, persistent, disabled):
+    granularity = 1 << (13 + bitmap_number)
+    bitmap_name = 'bitmap-' + str(bitmap_number-1)
+    vm = iotests.VM().add_drive(disk)
+    vm.launch()
+    vm.qmp_log('block-dirty-bitmap-add', node='drive0', name=bitmap_name,
+               granularity=granularity, persistent=persistent,
+               disabled=disabled)
+    vm.shutdown()
+
+
+def write_to_disk(offset, size):
+    write = 'write {} {}'.format(offset, size)
+    log(qemu_io('-c', write, disk), filters=[filter_qemu_io])
+
+
+def toggle_flag(offset):
+    with open(disk, "r+b") as f:
+        f.seek(offset, 0)
+        c = f.read(1)
+        toggled = chr(ord(c) ^ bitmap_flag_unknown)
+        f.seek(-1, 1)
+        f.write(toggled)
+
+
+qemu_img_create('-f', iotests.imgfmt, disk, '1M')
+
+for num in range(1, 4):
+    disabled = False
+    if num == 2:
+        disabled = True
+    log('Test {}'.format(num))
+    add_bitmap(num, num > 1, disabled)
+    write_to_disk((num-1) * chunk, chunk)
+    print_bitmap([])
+    log('')
+
+vm = iotests.VM().add_drive(disk)
+vm.launch()
+num += 1
+log('Test {}\nChecking "in-use" flag...'.format(num))
+print_bitmap(['--force-share'])
+vm.shutdown()
+
+num += 1
+log('\nTest {}'.format(num))
+qemu_img_create('-f', iotests.imgfmt, disk, '1M')
+add_bitmap(1, True, False)
+log('Write an unknown bitmap flag \'{}\' into a new QCOW2 image at offset {}'
+    .format(hex(bitmap_flag_unknown), flag_offset))
+toggle_flag(flag_offset)
+img_info_log(disk)
+toggle_flag(flag_offset)
+log('Unset the unknown bitmap flag \'{}\' in the bitmap directory entry:\n'
+    .format(hex(bitmap_flag_unknown)))
+img_info_log(disk)
diff --git a/tests/qemu-iotests/242.out b/tests/qemu-iotests/242.out
new file mode 100644
index 0000000..21ecabd
--- /dev/null
+++ b/tests/qemu-iotests/242.out
@@ -0,0 +1,165 @@
+Test 1
+{"execute": "block-dirty-bitmap-add", "arguments": {"disabled": false, "granularity": 16384, "name": "bitmap-0", "node": "drive0", "persistent": false}}
+{"return": {}}
+wrote 262144/262144 bytes at offset 0
+256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+qemu-img info dump:
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 1.0M (1048576 bytes)
+cluster_size: 65536
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+    refcount bits: 16
+    corrupt: false
+
+No bitmap in JSON format output
+
+Test 2
+{"execute": "block-dirty-bitmap-add", "arguments": {"disabled": true, "granularity": 32768, "name": "bitmap-1", "node": "drive0", "persistent": true}}
+{"return": {}}
+wrote 262144/262144 bytes at offset 262144
+256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+qemu-img info dump:
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 1.0M (1048576 bytes)
+cluster_size: 65536
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+    bitmaps:
+        [0]:
+            flags:
+            name: bitmap-1
+            granularity: 32768
+    refcount bits: 16
+    corrupt: false
+
+The same bitmaps in JSON format:
+[
+  {
+    "flags": [],
+    "granularity": 32768,
+    "name": "bitmap-1"
+  }
+]
+
+Test 3
+{"execute": "block-dirty-bitmap-add", "arguments": {"disabled": false, "granularity": 65536, "name": "bitmap-2", "node": "drive0", "persistent": true}}
+{"return": {}}
+wrote 262144/262144 bytes at offset 524288
+256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+qemu-img info dump:
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 1.0M (1048576 bytes)
+cluster_size: 65536
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+    bitmaps:
+        [0]:
+            flags:
+            name: bitmap-1
+            granularity: 32768
+        [1]:
+            flags:
+                [0]: auto
+            name: bitmap-2
+            granularity: 65536
+    refcount bits: 16
+    corrupt: false
+
+The same bitmaps in JSON format:
+[
+  {
+    "flags": [],
+    "granularity": 32768,
+    "name": "bitmap-1"
+  },
+  {
+    "flags": [
+      "auto"
+    ],
+    "granularity": 65536,
+    "name": "bitmap-2"
+  }
+]
+
+Test 4
+Checking "in-use" flag...
+qemu-img info dump:
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 1.0M (1048576 bytes)
+cluster_size: 65536
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+    bitmaps:
+        [0]:
+            flags:
+                [0]: in-use
+            name: bitmap-1
+            granularity: 32768
+        [1]:
+            flags:
+                [0]: in-use
+                [1]: auto
+            name: bitmap-2
+            granularity: 65536
+    refcount bits: 16
+    corrupt: false
+
+The same bitmaps in JSON format:
+[
+  {
+    "flags": [
+      "in-use"
+    ],
+    "granularity": 32768,
+    "name": "bitmap-1"
+  },
+  {
+    "flags": [
+      "in-use",
+      "auto"
+    ],
+    "granularity": 65536,
+    "name": "bitmap-2"
+  }
+]
+
+Test 5
+{"execute": "block-dirty-bitmap-add", "arguments": {"disabled": false, "granularity": 16384, "name": "bitmap-0", "node": "drive0", "persistent": true}}
+{"return": {}}
+Write an unknown bitmap flag '0x4' into a new QCOW2 image at offset 327695
+qemu-img: Could not open 'TEST_IMG': Bitmap 'bitmap-0' doesn't satisfy the constraints
+
+Unset the unknown bitmap flag '0x4' in the bitmap directory entry:
+
+image: TEST_IMG
+file format: IMGFMT
+virtual size: 1.0M (1048576 bytes)
+cluster_size: 65536
+Format specific information:
+    compat: 1.1
+    lazy refcounts: false
+    bitmaps:
+        [0]:
+            flags:
+                [0]: auto
+            name: bitmap-0
+            granularity: 16384
+    refcount bits: 16
+    corrupt: false
+
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 0f1c3f9..32acdd7 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -235,3 +235,4 @@
 235 auto quick
 236 auto quick
 238 auto quick
+242 rw auto quick
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v14 0/3] qcow2: Add list of bitmaps to ImageInfoSpecificQCow2

[Eric Blake]

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

On 2/8/19 9:06 AM, Andrey Shinkevich wrote:
> v14:
> The test 242 was amended to be safer against changes in the bitmap allocation
> algorithm because the bitmap directory is reallocated on VM shutdown.
> Unfortunately, an empty line at the end of the benchmark file 242.out persists.

Easy solution: in 242, add an extra log("Test complete") or similar
after the output that produces the blank line, then 242.out will no
longer have a trailing blank line.

-- 
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 v14 0/3] qcow2: Add list of bitmaps to ImageInfoSpecificQCow2

[Andrey Shinkevich]

On 08/02/2019 18:25, Eric Blake wrote:
> On 2/8/19 9:06 AM, Andrey Shinkevich wrote:
>> v14:
>> The test 242 was amended to be safer against changes in the bitmap allocation
>> algorithm because the bitmap directory is reallocated on VM shutdown.
>> Unfortunately, an empty line at the end of the benchmark file 242.out persists.
> 
> Easy solution: in 242, add an extra log("Test complete") or similar
> after the output that produces the blank line, then 242.out will no
> longer have a trailing blank line.
> 
Thank you, Eric. It is going to be version 15.
-- 
With the best regards,
Andrey Shinkevich

Re: [Qemu-devel] [PATCH v14 3/3] qcow2: list of bitmaps new test 242

[Eric Blake]

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

On 2/8/19 9:06 AM, Andrey Shinkevich wrote:
> A new test file 242 added to the qemu-iotests set. It checks
> the format of qcow2 specific information for the new added
> section that lists details of bitmaps.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>  tests/qemu-iotests/242     | 103 ++++++++++++++++++++++++++++
>  tests/qemu-iotests/242.out | 165 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 269 insertions(+)
>  create mode 100755 tests/qemu-iotests/242
>  create mode 100644 tests/qemu-iotests/242.out
> 

> +
> +iotests.verify_image_format(supported_fmts=['qcow2'])
> +
> +disk = file_path('disk')
> +chunk = 256*1024
> +bitmap_flag_unknown = 1 << 2
> +#flag_offset = 5*cluster_size + flag_offset_in_bitmap_directory_entry

pep8 didn't like the comment (missing space after #). I guess it doesn't
care about spaces or lack of spaces around binary operators (I might
have pug spaces around * in the assignment to chunk, but since pep8
didn't complain, I didn't change it).

As mentioned in the cover letter, adding:

log('Test complete')

at the end of 242, and adjusting 242.out accordingly, silences the
checkpatch warning. So with that change, I've now queued your series for
my next NBD pull request, probably on Monday.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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 v14 0/3] qcow2: Add list of bitmaps to ImageInfoSpecificQCow2

[Vladimir Sementsov-Ogievskiy]

08.02.2019 18:25, Eric Blake wrote:
> On 2/8/19 9:06 AM, Andrey Shinkevich wrote:
>> v14:
>> The test 242 was amended to be safer against changes in the bitmap allocation
>> algorithm because the bitmap directory is reallocated on VM shutdown.
>> Unfortunately, an empty line at the end of the benchmark file 242.out persists.
> 
> Easy solution: in 242, add an extra log("Test complete") or similar
> after the output that produces the blank line, then 242.out will no
> longer have a trailing blank line.
> 

with this fixed:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v14 0/3] qcow2: Add list of bitmaps to ImageInfoSpecificQCow2

[Eric Blake]

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

On 2/8/19 9:37 AM, Andrey Shinkevich wrote:
> 
> 
> On 08/02/2019 18:25, Eric Blake wrote:
>> On 2/8/19 9:06 AM, Andrey Shinkevich wrote:
>>> v14:
>>> The test 242 was amended to be safer against changes in the bitmap allocation
>>> algorithm because the bitmap directory is reallocated on VM shutdown.
>>> Unfortunately, an empty line at the end of the benchmark file 242.out persists.
>>
>> Easy solution: in 242, add an extra log("Test complete") or similar
>> after the output that produces the blank line, then 242.out will no
>> longer have a trailing blank line.
>>
> Thank you, Eric. It is going to be version 15.

If that's the only change, then no need to post v15; you can check what
I'm preparing to include in Monday's pull request:

https://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/nbd

-- 
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 v14 0/3] qcow2: Add list of bitmaps to ImageInfoSpecificQCow2

[Andrey Shinkevich]

On 08/02/2019 18:53, Eric Blake wrote:
> On 2/8/19 9:37 AM, Andrey Shinkevich wrote:
>>
>>
>> On 08/02/2019 18:25, Eric Blake wrote:
>>> On 2/8/19 9:06 AM, Andrey Shinkevich wrote:
>>>> v14:
>>>> The test 242 was amended to be safer against changes in the bitmap allocation
>>>> algorithm because the bitmap directory is reallocated on VM shutdown.
>>>> Unfortunately, an empty line at the end of the benchmark file 242.out persists.
>>>
>>> Easy solution: in 242, add an extra log("Test complete") or similar
>>> after the output that produces the blank line, then 242.out will no
>>> longer have a trailing blank line.
>>>
>> Thank you, Eric. It is going to be version 15.
> 
> If that's the only change, then no need to post v15; you can check what
> I'm preparing to include in Monday's pull request:
> 
> https://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/nbd
> 
Thank you again.
With those minor amendments (+spaces) and a couple of the significant
"Reviewed-by", the version 15 has been prepared. May I send it
or you are OK with the version 14?
-- 
With the best regards,
Andrey Shinkevich

Re: [Qemu-devel] [PATCH v14 0/3] qcow2: Add list of bitmaps to ImageInfoSpecificQCow2

[Andrey Shinkevich]

On 08/02/2019 18:53, Eric Blake wrote:
> On 2/8/19 9:37 AM, Andrey Shinkevich wrote:
>>
>>
>> On 08/02/2019 18:25, Eric Blake wrote:
>>> On 2/8/19 9:06 AM, Andrey Shinkevich wrote:
>>>> v14:
>>>> The test 242 was amended to be safer against changes in the bitmap allocation
>>>> algorithm because the bitmap directory is reallocated on VM shutdown.
>>>> Unfortunately, an empty line at the end of the benchmark file 242.out persists.
>>>
>>> Easy solution: in 242, add an extra log("Test complete") or similar
>>> after the output that produces the blank line, then 242.out will no
>>> longer have a trailing blank line.
>>>
>> Thank you, Eric. It is going to be version 15.
> 
> If that's the only change, then no need to post v15; you can check what
> I'm preparing to include in Monday's pull request:
> 
> https://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/nbd
> 
I followed the link above and saw, thank you.
The only difference with my version #15 is that I added the spaces
around '*' in the chunk assignment. That's it!
-- 
With the best regards,
Andrey Shinkevich

Re: [Qemu-devel] [PATCH v14 3/3] qcow2: list of bitmaps new test 242

[Kevin Wolf]

Am 08.02.2019 um 16:06 hat Andrey Shinkevich geschrieben:
> A new test file 242 added to the qemu-iotests set. It checks
> the format of qcow2 specific information for the new added
> section that lists details of bitmaps.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>

This doesn't seem to be Python 3 compatible.

Kevin


242                [17:09:27] [17:09:29] [failed, exit status 1] - output mismatch (see 242.out.bad)
--- /home/kwolf/source/qemu/tests/qemu-iotests/242.out  2019-02-13 12:59:38.748262364 +0100
+++ /home/kwolf/source/qemu/tests/qemu-iotests/242.out.bad      2019-02-18 17:09:29.314637597 +0100
@@ -143,24 +143,9 @@
 {"execute": "block-dirty-bitmap-add", "arguments": {"disabled": false, "granularity": 16384, "name": "bitmap-0", "node": "drive0", "persistent": true}}
 {"return": {}}
 Write an unknown bitmap flag '0x4' into a new QCOW2 image at offset 327695
-qemu-img: Could not open 'TEST_IMG': Bitmap 'bitmap-0' doesn't satisfy the constraints
-
-Unset the unknown bitmap flag '0x4' in the bitmap directory entry:
-
-image: TEST_IMG
-file format: IMGFMT
-virtual size: 1.0M (1048576 bytes)
-cluster_size: 65536
-Format specific information:
-    compat: 1.1
-    lazy refcounts: false
-    bitmaps:
-        [0]:
-            flags:
-                [0]: auto
-            name: bitmap-0
-            granularity: 16384
-    refcount bits: 16
-    corrupt: false
-
-Test complete
+Traceback (most recent call last):
+  File "242", line 98, in <module>
+    toggle_flag(flag_offset)
+  File "242", line 70, in toggle_flag
+    f.write(toggled)
+TypeError: a bytes-like object is required, not 'str'
Failures: 242
Failed 1 of 1 tests

Re: [Qemu-devel] [PATCH v14 3/3] qcow2: list of bitmaps new test 242

[Andrey Shinkevich]

On 18/02/2019 19:12, Kevin Wolf wrote:
> Am 08.02.2019 um 16:06 hat Andrey Shinkevich geschrieben:
>> A new test file 242 added to the qemu-iotests set. It checks
>> the format of qcow2 specific information for the new added
>> section that lists details of bitmaps.
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> 
> This doesn't seem to be Python 3 compatible.
> 
> Kevin
> 

Thank you, Kevin.
Fixed. Please review the patch
0001-iotests-handle-TypeError-for-Python3-in-test-242.patch

> 
> 242                [17:09:27] [17:09:29] [failed, exit status 1] - output mismatch (see 242.out.bad)
> --- /home/kwolf/source/qemu/tests/qemu-iotests/242.out  2019-02-13 12:59:38.748262364 +0100
> +++ /home/kwolf/source/qemu/tests/qemu-iotests/242.out.bad      2019-02-18 17:09:29.314637597 +0100
> @@ -143,24 +143,9 @@
>   {"execute": "block-dirty-bitmap-add", "arguments": {"disabled": false, "granularity": 16384, "name": "bitmap-0", "node": "drive0", "persistent": true}}
>   {"return": {}}
>   Write an unknown bitmap flag '0x4' into a new QCOW2 image at offset 327695
> -qemu-img: Could not open 'TEST_IMG': Bitmap 'bitmap-0' doesn't satisfy the constraints
> -
> -Unset the unknown bitmap flag '0x4' in the bitmap directory entry:
> -
> -image: TEST_IMG
> -file format: IMGFMT
> -virtual size: 1.0M (1048576 bytes)
> -cluster_size: 65536
> -Format specific information:
> -    compat: 1.1
> -    lazy refcounts: false
> -    bitmaps:
> -        [0]:
> -            flags:
> -                [0]: auto
> -            name: bitmap-0
> -            granularity: 16384
> -    refcount bits: 16
> -    corrupt: false
> -
> -Test complete
> +Traceback (most recent call last):
> +  File "242", line 98, in <module>
> +    toggle_flag(flag_offset)
> +  File "242", line 70, in toggle_flag
> +    f.write(toggled)
> +TypeError: a bytes-like object is required, not 'str'
> Failures: 242
> Failed 1 of 1 tests
> 

-- 
With the best regards,
Andrey Shinkevich