[Qemu-devel] [PATCH v5 00/22] qcow2: persistent dirty bitmaps

[Qemu-devel] [PATCH v5 00/22] qcow2: persistent dirty bitmaps

[Vladimir Sementsov-Ogievskiy]

This series add persistent dirty bitmaps feature to qcow2.
Specification is in docs/spec/qcow2.txt 

v5:
https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=refs%2Ftags%2Fqcow2-bitmap-v5
Rebased on master. RFC removed. All necessary preparations are included.

changes:

patches 01-05 was not included in v4, so:

01: by Fam. only context changed, so I didn't add my s.o.b.
02: was "block: fix bdrv_dirty_bitmap_granularity()". Improve commit message
    and add lost Eric's r.b.
03: was on list. Not mentioned previously change, made after John's r.b.:
    split qemu-qtest by vm number too. Hope you don't mind
04: rewrite to not break 055
05: add lost Eric's r.b.
    changes after r.b. (hope, you don't mind):
    # @md5: md5 checksum of the last bitmap level (since 2.4)
    to
    # @md5: md5 checksum (as a hexadecimal string) of the last bitmap level
    #       (since 2.6)
  

06: new static function hb_restore_levels is actually
    hbitmap_deserialize_finish from list. I've included it here as it
    is the only dependence on serialization interface which is now on
    list in Fam's series. If Fam's series will be merged first I'll rebase
    and switch back to hbitmap_deserialize_finish(). Otherwise, Fam can
    rewrite hbitmap_deserialize_finish() to just call hb_restore_levels().
08: rebase: add #include "qemu/osdep.h"
13: rebase: move bdrv_finalize_persistent_dirty_bitmaps so that it called before
    bdrv_release_named_dirty_bitmaps =)
16: change error handling in dirty_bitmap_func: switch from errp to direct error
    reporting. This is more common approach in vl.c and also it allows to bypass
    one bug (see *)
19: test_launch: in qemu error output: s/qemu-system-\w*/qemu-system-*/ for test
    portability
20: accordingly to previous change: s/x86_64/*/ in 160.out

*: intersting bug, not related to these series:

./configure --target-list=x86_64-softmmu
make
./x86_64-softmmu/qemu-system-x86_64 -object asd
>> qemu-system-x86_64: -object asd: Parameter 'id' is missing

but:

./configure --target-list=x86_64-softmmu --enable-debug
make 
./x86_64-softmmu/qemu-system-x86_64 -object asd
>> qemu-system-x86_64: Parameter 'id' is missing (null): Parameter 'id' is missing

v4: 
https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=refs%2Ftags%2Fqcow2-bitmap-v4
Previous version was posted more than five months ago, so I will not
carefully list all changes.

What should be noted:
 - some changes are made to sutisfy last version of specification
   - removed staff, related to possibility of saving bitmaps for one
     disk in the other qcow2.
 - to make bitmap store/load zero-copy, I've moved load/store code to
   HBitmap - this is new patch 01.
   so, bdrv_dirty_bitmap_serialize_part and friends are not needed,
   only hbitmap_deserialize_finish, to repair bitmap consistency after
   loading its last level.
 - two patches added about AUTO and EXTRA_DATA_COMPATIBLE flags
 - some fixes made after John's comments on v3

Fam Zheng (1):
  block: Add two dirty bitmap getters

Vladimir Sementsov-Ogievskiy (21):
  block: fix bdrv_dirty_bitmap_granularity signature
  iotests: maintain several vms in test
  iotests: add default node-name
  qapi: add md5 checksum of last dirty bitmap level to query-block
  hbitmap: load/store
  qcow2: Bitmaps extension: structs and consts
  qcow2-dirty-bitmap: read dirty bitmap directory
  qcow2-dirty-bitmap: add qcow2_bitmap_load()
  qcow2-dirty-bitmap: add qcow2_bitmap_store()
  qcow2: add dirty bitmaps extension
  qcow2-dirty-bitmap: add qcow2_bitmap_load_check()
  block: store persistent dirty bitmaps
  block: add bdrv_load_dirty_bitmap()
  qcow2-dirty-bitmap: add autoclear bit
  qemu: command line option for dirty bitmaps
  qcow2-dirty-bitmap: add IN_USE flag
  qcow2-dirty-bitmaps: disallow stroing bitmap to other bs
  iotests: add VM.test_launcn()
  iotests: test internal persistent dirty bitmap
  qcow2-dirty-bitmap: add AUTO flag
  qcow2-dirty-bitmap: add EXTRA_DATA_COMPATIBLE flag

 block.c                       |   2 +
 block/Makefile.objs           |   2 +-
 block/dirty-bitmap.c          | 114 +++++-
 block/qcow2-dirty-bitmap.c    | 841 ++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c                 | 105 +++++-
 block/qcow2.h                 |  59 +++
 blockdev.c                    |  36 ++
 include/block/block_int.h     |   9 +
 include/block/dirty-bitmap.h  |  25 +-
 include/qemu/hbitmap.h        |  20 +
 include/sysemu/blockdev.h     |   1 +
 include/sysemu/sysemu.h       |   1 +
 qapi/block-core.json          |   5 +-
 qemu-options.hx               |  35 ++
 tests/qemu-iotests/160        | 112 ++++++
 tests/qemu-iotests/160.out    |  21 ++
 tests/qemu-iotests/group      |   1 +
 tests/qemu-iotests/iotests.py |  35 +-
 util/hbitmap.c                | 217 +++++++++++
 vl.c                          |  79 ++++
 20 files changed, 1711 insertions(+), 9 deletions(-)
 create mode 100644 block/qcow2-dirty-bitmap.c
 create mode 100755 tests/qemu-iotests/160
 create mode 100644 tests/qemu-iotests/160.out

-- 
1.8.3.1

[Qemu-devel] [PATCH 01/22] block: Add two dirty bitmap getters

[Vladimir Sementsov-Ogievskiy]

From: Fam Zheng <famz@redhat.com>

For dirty bitmap users to get the size and the name of a
BdrvDirtyBitmap.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/dirty-bitmap.c         | 10 ++++++++++
 include/block/dirty-bitmap.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 556e1d1..45cfa3b 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -97,6 +97,16 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
     return bitmap;
 }
 
+int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
+{
+    return bitmap->size;
+}
+
+const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
+{
+    return bitmap->name;
+}
+
 bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
 {
     return bitmap->successor;
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 80afe60..4dc8750 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -29,6 +29,8 @@ uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
 uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
+const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
+int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap);
 DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
                    int64_t sector);
-- 
1.8.3.1

[Qemu-devel] [PATCH 02/22] block: fix bdrv_dirty_bitmap_granularity signature

[Vladimir Sementsov-Ogievskiy]

Make getter signature const-correct. This allows other functions with
const dirty bitmap parameter use bdrv_dirty_bitmap_granularity().

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/dirty-bitmap.c         | 2 +-
 include/block/dirty-bitmap.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 45cfa3b..88d7967 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -325,7 +325,7 @@ uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
     return granularity;
 }
 
-uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap)
+uint32_t bdrv_dirty_bitmap_granularity(const BdrvDirtyBitmap *bitmap)
 {
     return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
 }
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 4dc8750..27515af 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -26,7 +26,7 @@ void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
-uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
+uint32_t bdrv_dirty_bitmap_granularity(const BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
 bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
 const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
-- 
1.8.3.1

[Qemu-devel] [PATCH 03/22] iotests: maintain several vms in test

[Vladimir Sementsov-Ogievskiy]

The only problem with it is the same qmp socket name (which is
vm._monitor_path) for all vms. And because of this second vm couldn't be
lauched (vm.launch() fails because of socket is already in use).
This patch adds a number of vm into vm._monitor_path

Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 0a238ec..2445cf2 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -120,11 +120,12 @@ def event_match(event, match=None):
 
 class VM(object):
     '''A QEMU VM'''
+    nb_vms = 0
 
     def __init__(self):
-        self._monitor_path = os.path.join(test_dir, 'qemu-mon.%d' % os.getpid())
-        self._qemu_log_path = os.path.join(test_dir, 'qemu-log.%d' % os.getpid())
-        self._qtest_path = os.path.join(test_dir, 'qemu-qtest.%d' % os.getpid())
+        self._monitor_path = os.path.join(test_dir, 'qemu-mon.%d.%d' % (os.getpid(), VM.nb_vms))
+        self._qemu_log_path = os.path.join(test_dir, 'qemu-log.%d.%d' % (os.getpid(), VM.nb_vms))
+        self._qtest_path = os.path.join(test_dir, 'qemu-qtest.%d.%d' % (os.getpid(), VM.nb_vms))
         self._args = qemu_args + ['-chardev',
                      'socket,id=mon,path=' + self._monitor_path,
                      '-mon', 'chardev=mon,mode=control',
@@ -133,6 +134,7 @@ class VM(object):
                      '-display', 'none', '-vga', 'none']
         self._num_drives = 0
         self._events = []
+        VM.nb_vms += 1
 
     # This can be used to add an unused monitor instance.
     def add_monitor_telnet(self, ip, port):
-- 
1.8.3.1

[Qemu-devel] [PATCH 04/22] iotests: add default node-name

[Vladimir Sementsov-Ogievskiy]

When testing migration, auto-generated by qemu node-names differs in
source and destination qemu and migration fails. After this patch,
auto-generated by iotest nodenames will be the same.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 2445cf2..6807b07 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -156,6 +156,7 @@ class VM(object):
             options.append('file=%s' % path)
             options.append('format=%s' % imgfmt)
             options.append('cache=%s' % cachemode)
+            options.append('node-name=drivenode%d' % self._num_drives)
 
         if opts:
             options.append(opts)
-- 
1.8.3.1

[Qemu-devel] [PATCH 05/22] qapi: add md5 checksum of last dirty bitmap level to query-block

[Vladimir Sementsov-Ogievskiy]

Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/dirty-bitmap.c   | 1 +
 include/qemu/hbitmap.h | 8 ++++++++
 qapi/block-core.json   | 5 ++++-
 util/hbitmap.c         | 8 ++++++++
 4 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 88d7967..e68c177 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -287,6 +287,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
         info->has_name = !!bm->name;
         info->name = g_strdup(bm->name);
         info->status = bdrv_dirty_bitmap_status(bm);
+        info->md5 = hbitmap_md5(bm->bitmap);
         entry->value = info;
         *plist = entry;
         plist = &entry->next;
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index e29188c..6d1da4d 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -146,6 +146,14 @@ void hbitmap_reset_all(HBitmap *hb);
 bool hbitmap_get(const HBitmap *hb, uint64_t item);
 
 /**
+ * hbitmap_md5:
+ * @bitmap: HBitmap to operate on.
+ *
+ * Returns md5 checksum of the last level.
+ */
+char *hbitmap_md5(const HBitmap *bitmap);
+
+/**
  * hbitmap_free:
  * @hb: HBitmap to operate on.
  *
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9bf1b22..11b0aed 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -414,11 +414,14 @@
 #
 # @status: current status of the dirty bitmap (since 2.4)
 #
+# @md5: md5 checksum (as a hexadecimal string) of the last bitmap level
+#       (since 2.6)
+#
 # Since: 1.3
 ##
 { 'struct': 'BlockDirtyInfo',
   'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
-           'status': 'DirtyBitmapStatus'} }
+           'status': 'DirtyBitmapStatus', 'md5': 'str'} }
 
 ##
 # @BlockInfo:
diff --git a/util/hbitmap.c b/util/hbitmap.c
index b22b87d..28595fb 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -491,3 +491,11 @@ bool hbitmap_merge(HBitmap *a, const HBitmap *b)
 
     return true;
 }
+
+char *hbitmap_md5(const HBitmap *bitmap)
+{
+    uint64_t size =
+        MAX((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+    const guchar *data = (const guchar *)bitmap->levels[HBITMAP_LEVELS - 1];
+    return g_compute_checksum_for_data(G_CHECKSUM_MD5, data, size);
+}
-- 
1.8.3.1

[Qemu-devel] [PATCH 06/22] hbitmap: load/store

[Vladimir Sementsov-Ogievskiy]

Add functions for load/store HBitmap to BDS, using clusters table:
Last level of the bitmap is splitted into chunks of 'cluster_size'
size. Each cell of the table contains offset in bds, to load/store
corresponding chunk.

Also,
    0 in cell means all-zeroes-chunk (should not be saved)
    1 in cell means all-ones-chunk (should not be saved)
    hbitmap_prepare_store() fills table with
      0 for all-zeroes chunks
      1 for all-ones chunks
      2 for others

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/dirty-bitmap.c         |  23 +++++
 include/block/dirty-bitmap.h |  11 +++
 include/qemu/hbitmap.h       |  12 +++
 util/hbitmap.c               | 209 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 255 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index e68c177..816c6ee 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -396,3 +396,26 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
 {
     return hbitmap_count(bitmap->bitmap);
 }
+
+int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
+                           const uint64_t *table, uint32_t table_size,
+                           uint32_t cluster_size)
+{
+    return hbitmap_load(bitmap->bitmap, bs, table, table_size, cluster_size);
+}
+
+int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
+                                    uint32_t cluster_size,
+                                    uint64_t *table,
+                                    uint32_t *table_size)
+{
+    return hbitmap_prepare_store(bitmap->bitmap, cluster_size,
+                                 table, table_size);
+}
+
+int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
+                            const uint64_t *table, uint32_t table_size,
+                            uint32_t cluster_size)
+{
+    return hbitmap_store(bitmap->bitmap, bs, table, table_size, cluster_size);
+}
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 27515af..20cb540 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -43,4 +43,15 @@ void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
 int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
 
+int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
+                           const uint64_t *table, uint32_t table_size,
+                           uint32_t cluster_size);
+int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
+                                    uint32_t cluster_size,
+                                    uint64_t *table,
+                                    uint32_t *table_size);
+int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
+                            const uint64_t *table, uint32_t table_size,
+                            uint32_t cluster_size);
+
 #endif
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 6d1da4d..d83bb79 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -241,5 +241,17 @@ static inline size_t hbitmap_iter_next_word(HBitmapIter *hbi, unsigned long *p_c
     return hbi->pos;
 }
 
+typedef struct BlockDriverState BlockDriverState;
+
+int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
+                 const uint64_t *table, uint32_t table_size,
+                 uint32_t cluster_size);
+
+int hbitmap_prepare_store(const HBitmap *bitmap, uint32_t cluster_size,
+                          uint64_t *table, uint32_t *table_size);
+
+int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
+                  const uint64_t *table, uint32_t table_size,
+                  uint32_t cluster_size);
 
 #endif
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 28595fb..1960e4f 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -15,6 +15,8 @@
 #include "qemu/host-utils.h"
 #include "trace.h"
 
+#include "block/block.h"
+
 /* HBitmaps provides an array of bits.  The bits are stored as usual in an
  * array of unsigned longs, but HBitmap is also optimized to provide fast
  * iteration over set bits; going from one bit to the next is O(logB n)
@@ -499,3 +501,210 @@ char *hbitmap_md5(const HBitmap *bitmap)
     const guchar *data = (const guchar *)bitmap->levels[HBITMAP_LEVELS - 1];
     return g_compute_checksum_for_data(G_CHECKSUM_MD5, data, size);
 }
+
+/* hb_restore_levels()
+ * Using the last level restore all other levels
+ */
+static void hb_restore_levels(HBitmap *bitmap)
+{
+    int64_t i, size, prev_size;
+    int lev;
+
+    /* restore levels starting from penultimate to zero level, assuming
+     * that the last level is ok */
+    size = MAX((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+    for (lev = HBITMAP_LEVELS - 1; lev-- > 0; ) {
+        prev_size = size;
+        size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+        memset(bitmap->levels[lev], 0, size * sizeof(unsigned long));
+
+        for (i = 0; i < prev_size; ++i) {
+            if (bitmap->levels[lev + 1][i]) {
+                bitmap->levels[lev][i >> BITS_PER_LEVEL] |=
+                    1UL << (i & (BITS_PER_LONG - 1));
+            }
+        }
+    }
+
+    bitmap->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
+}
+
+/* load_bitmap()
+ * Load dirty bitmap from file, using table with cluster offsets.
+ * Table entries are assumed to be in little endian format.
+ */
+int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
+                 const uint64_t *table, uint32_t table_size,
+                 uint32_t cluster_size)
+{
+    uint32_t i;
+    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
+    uint8_t *end = cur + ((bitmap->size + 7) >> 3);
+
+    hbitmap_reset_all(bitmap);
+
+    for (i = 0; i < table_size && cur < end; ++i) {
+        uint64_t offset = table[i];
+        uint64_t count = MIN(cluster_size, end - cur);
+
+        /* Zero offset means zero region, offset = 1 means filled region.
+         * Cluster is not allocated in both cases. */
+        if (offset == 1) {
+            memset(cur, 0xff, count);
+        } else if (offset) {
+            int ret = bdrv_pread(bs, offset, cur, count);
+            if (ret < 0) {
+                return ret;
+            }
+        }
+
+        cur += cluster_size;
+    }
+
+    cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
+    while (cur < end) {
+        if (BITS_PER_LONG == 32) {
+            le32_to_cpus((uint32_t *)cur);
+        } else {
+            le64_to_cpus((uint64_t *)cur);
+        }
+
+        cur += sizeof(unsigned long);
+    }
+
+    hb_restore_levels(bitmap);
+
+    return 0;
+}
+
+static bool buffer_is_all_ones(void *buf, size_t len)
+{
+    /* FIXME */
+    return false;
+}
+
+/* hbitmap_prepare_store()
+ * Devide bitmap data into clusters, and then,
+ * for zero cluster: table[i] = 0
+ * for all-ones cluster: table[i] = 1
+ * for other clusters: table[i] = 2
+ */
+int hbitmap_prepare_store(const HBitmap *bitmap,
+                          uint32_t cluster_size,
+                          uint64_t *table,
+                          uint32_t *table_size)
+{
+    HBitmapIter hbi;
+    hbitmap_iter_init(&hbi, bitmap, 0);
+    uint64_t nb_bits_in_cl = (uint64_t)cluster_size << 3;
+    uint32_t need_table_size =
+            (bitmap->size + nb_bits_in_cl - 1) / nb_bits_in_cl;
+
+    if (table == NULL && *table_size == 0) {
+        *table_size = need_table_size;
+        return 0;
+    }
+
+    if (*table_size < need_table_size) {
+        return -ENOMEM;
+    }
+
+    memset(table, 0, *table_size * sizeof(table[0]));
+
+    for (;;) {
+        unsigned long cur;
+        size_t pos = hbitmap_iter_next_word(&hbi, &cur);
+        size_t byte = pos * sizeof(unsigned long);
+        uint64_t bit = byte << 3;
+        uint64_t nbits = MIN(cluster_size << 3, bitmap->size - bit), next_bit;
+        size_t i = byte / cluster_size;
+
+        if (pos == -1) {
+            break;
+        }
+
+        if (pos % cluster_size != 0) {
+            table[i] = 2;
+        } else if (buffer_is_all_ones(&bitmap->levels[HBITMAP_LEVELS - 1][pos],
+                               nbits >> 3)) {
+            table[i] = 1;
+            if (nbits & 7) {
+                uint8_t last_byte =
+                        *(((uint8_t *)&bitmap->levels[HBITMAP_LEVELS - 1][pos])
+                                + (nbits >> 3));
+                if (last_byte != ((1 << (nbits & 7)) - 1)) {
+                    table[i] = 2;
+                }
+            }
+        } else {
+            table[i] = 2;
+        }
+
+        next_bit = (i + 1) * cluster_size << 3;
+
+        if (next_bit >= bitmap->size) {
+            break;
+        }
+
+        hbitmap_iter_init(&hbi, bitmap, next_bit << bitmap->granularity);
+    }
+
+    return 0;
+}
+
+static void longs_to_le(unsigned long *longs, size_t count)
+{
+    unsigned long *end = longs + count;
+    while (longs < end) {
+        if (BITS_PER_LONG == 32) {
+            cpu_to_le32s((uint32_t *)longs);
+        } else {
+            cpu_to_le64s((uint64_t *)longs);
+        }
+
+        longs++;
+    }
+}
+
+/* store_bitmap()
+ * update bitmap table by storing bitmap to it.
+ * bitmap table entries are assumed to be in big endian format
+ * On the error, the resulting bitmap table is valid for clearing, but
+ * may contain invalid bitmap */
+int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
+                  const uint64_t *table, uint32_t table_size,
+                  uint32_t cluster_size)
+{
+    int i;
+    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
+    uint8_t *end = cur +
+        ((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL) * (sizeof(long));
+
+    for (i = 0; i < table_size && cur < end; ++i) {
+        uint64_t offset = table[i];
+        uint64_t count = MIN(cluster_size, end - cur);
+
+        /* Zero offset means zero region, offset = 1 means filled region.
+         * Cluster is not allocated in both cases. */
+        if (offset > 1) {
+            int ret;
+            if (cpu_to_le16(1) == 1) {
+                ret = bdrv_pwrite(bs, offset, cur, count);
+            } else {
+                void *tmp = g_memdup(cur, count);
+                longs_to_le((unsigned long *)cur,
+                            count / sizeof(unsigned long));
+                ret = bdrv_pwrite(bs, offset, tmp, count);
+                g_free(tmp);
+            }
+
+            if (ret < 0) {
+                return ret;
+            }
+        }
+
+        cur += cluster_size;
+    }
+
+    return 0;
+}
-- 
1.8.3.1

[Qemu-devel] [PATCH 07/22] qcow2: Bitmaps extension: structs and consts

[Vladimir Sementsov-Ogievskiy]

Add data structures and constraints accordingly to docs/specs/qcow2.txt

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/Makefile.objs        |  2 +-
 block/qcow2-dirty-bitmap.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.h              | 34 +++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 block/qcow2-dirty-bitmap.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index cdd8655..c4dcf7c 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -1,5 +1,5 @@
 block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o
-block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
+block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o qcow2-dirty-bitmap.o
 block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-obj-y += qed-check.o
 block-obj-$(CONFIG_VHDX) += vhdx.o vhdx-endian.o vhdx-log.o
diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
new file mode 100644
index 0000000..2c749ab
--- /dev/null
+++ b/block/qcow2-dirty-bitmap.c
@@ -0,0 +1,47 @@
+/*
+ * Bitmaps for the QCOW version 2 format
+ *
+ * Copyright (c) 2014-2015 Vladimir Sementsov-Ogievskiy
+ *
+ * This file is derived from qcow2-snapshot.c, original copyright:
+ * Copyright (c) 2004-2006 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+/* NOTICE: BME here means Bitmaps Extension and used as a namespace for
+ * _internal_ constants. Please do not use this _internal_ abbreviation for
+ * other needs and/or outside of this file. */
+
+/* Bitmap directory entry constraints */
+#define BME_MAX_TABLE_SIZE 0x8000000
+#define BME_MAX_PHYS_SIZE 0x20000000 /* 512 mb */
+#define BME_MAX_GRANULARITY_BITS 31
+#define BME_MIN_GRANULARITY_BITS 9
+#define BME_MAX_NAME_SIZE 1023
+
+/* Bitmap directory entry flags */
+#define BME_RESERVED_FLAGS 0xffffffff
+
+/* bits [1, 8] U [56, 63] are reserved */
+#define BME_TABLE_ENTRY_RESERVED_MASK 0xff000000000001fe
+
+typedef enum BitmapType {
+    BT_DIRTY_TRACKING_BITMAP = 1
+} BitmapType;
diff --git a/block/qcow2.h b/block/qcow2.h
index a063a3c..3f7429e 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -52,6 +52,10 @@
  * space for snapshot names and IDs */
 #define QCOW_MAX_SNAPSHOTS_SIZE (1024 * QCOW_MAX_SNAPSHOTS)
 
+/* Bitmap header extension constraints */
+#define QCOW_MAX_DIRTY_BITMAPS 65535
+#define QCOW_MAX_DIRTY_BITMAP_DIRECTORY_SIZE (1024 * QCOW_MAX_DIRTY_BITMAPS)
+
 /* indicate that the refcount of the referenced cluster is exactly one. */
 #define QCOW_OFLAG_COPIED     (1ULL << 63)
 /* indicate that the cluster is compressed (they never have the copied flag) */
@@ -142,6 +146,22 @@ typedef struct QEMU_PACKED QCowSnapshotHeader {
     /* name follows  */
 } QCowSnapshotHeader;
 
+/* QCow2BitmapHeader is actually a bitmap directory entry */
+typedef struct QEMU_PACKED QCow2BitmapHeader {
+    /* header is 8 byte aligned */
+    uint64_t bitmap_table_offset;
+
+    uint32_t bitmap_table_size;
+    uint32_t flags;
+
+    uint8_t type;
+    uint8_t granularity_bits;
+    uint16_t name_size;
+    uint32_t extra_data_size;
+    /* extra data follows  */
+    /* name follows  */
+} QCow2BitmapHeader;
+
 typedef struct QEMU_PACKED QCowSnapshotExtraData {
     uint64_t vm_state_size_large;
     uint64_t disk_size;
@@ -160,6 +180,11 @@ typedef struct QCowSnapshot {
     uint64_t vm_clock_nsec;
 } QCowSnapshot;
 
+typedef struct QCow2Bitmap {
+    uint64_t offset;
+    char *name;
+} QCow2Bitmap;
+
 struct Qcow2Cache;
 typedef struct Qcow2Cache Qcow2Cache;
 
@@ -222,6 +247,15 @@ typedef uint64_t Qcow2GetRefcountFunc(const void *refcount_array,
 typedef void Qcow2SetRefcountFunc(void *refcount_array,
                                   uint64_t index, uint64_t value);
 
+/* Be careful, Qcow2BitmapHeaderExt is not an extension of QCow2BitmapHeader, it
+ * is Qcow2 header extension */
+typedef struct Qcow2BitmapHeaderExt {
+    uint32_t nb_bitmaps;
+    uint32_t reserved32;
+    uint64_t bitmap_directory_size;
+    uint64_t bitmap_directory_offset;
+} QEMU_PACKED Qcow2BitmapHeaderExt;
+
 typedef struct BDRVQcow2State {
     int cluster_bits;
     int cluster_size;
-- 
1.8.3.1

[Qemu-devel] [PATCH 08/22] qcow2-dirty-bitmap: read dirty bitmap directory

[Vladimir Sementsov-Ogievskiy]

Adds qcow2_read_bitmaps, reading bitmap directory as
specified in docs/specs/qcow2.txt

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-dirty-bitmap.c | 165 +++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.h              |  10 +++
 2 files changed, 175 insertions(+)

diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index 2c749ab..e57058c 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -25,6 +25,11 @@
  * THE SOFTWARE.
  */
 
+#include "qemu/osdep.h"
+
+#include "block/block_int.h"
+#include "block/qcow2.h"
+
 /* NOTICE: BME here means Bitmaps Extension and used as a namespace for
  * _internal_ constants. Please do not use this _internal_ abbreviation for
  * other needs and/or outside of this file. */
@@ -45,3 +50,163 @@
 typedef enum BitmapType {
     BT_DIRTY_TRACKING_BITMAP = 1
 } BitmapType;
+
+void qcow2_free_bitmaps(BlockDriverState *bs)
+{
+    BDRVQcow2State *s = bs->opaque;
+    int i;
+
+    for (i = 0; i < s->nb_bitmaps; i++) {
+        g_free(s->bitmaps[i].name);
+    }
+    g_free(s->bitmaps);
+    s->bitmaps = NULL;
+    s->nb_bitmaps = 0;
+
+    g_free(s->bitmap_directory);
+    s->bitmap_directory = NULL;
+}
+
+static void bitmap_header_to_cpu(QCow2BitmapHeader *h)
+{
+    be64_to_cpus(&h->bitmap_table_offset);
+    be32_to_cpus(&h->bitmap_table_size);
+    be32_to_cpus(&h->flags);
+    be16_to_cpus(&h->name_size);
+    be32_to_cpus(&h->extra_data_size);
+}
+
+static int calc_dir_entry_size(size_t name_size)
+{
+    return align_offset(sizeof(QCow2BitmapHeader) + name_size, 8);
+}
+
+static int dir_entry_size(QCow2BitmapHeader *h)
+{
+    return calc_dir_entry_size(h->name_size);
+}
+
+static int check_constraints(QCow2BitmapHeader *h, int cluster_size,
+                             uint64_t disk_size)
+{
+    uint64_t phys_bitmap_bytes =
+        (uint64_t)h->bitmap_table_size * cluster_size;
+    uint64_t max_virtual_bits = (phys_bitmap_bytes * 8) << h->granularity_bits;
+
+    int fail =
+            (h->bitmap_table_offset % cluster_size) ||
+            (h->bitmap_table_size > BME_MAX_TABLE_SIZE) ||
+            (phys_bitmap_bytes > BME_MAX_PHYS_SIZE) ||
+            (disk_size > max_virtual_bits) ||
+            (h->granularity_bits > BME_MAX_GRANULARITY_BITS) ||
+            (h->granularity_bits < BME_MIN_GRANULARITY_BITS) ||
+            (h->flags & BME_RESERVED_FLAGS) ||
+            (h->name_size > BME_MAX_NAME_SIZE) ||
+            (h->type != BT_DIRTY_TRACKING_BITMAP);
+
+    return fail ? -EINVAL : 0;
+}
+
+static int directory_read(BlockDriverState *bs, Error **errp)
+{
+    int ret;
+    BDRVQcow2State *s = bs->opaque;
+    QCow2Bitmap *bm, *end;
+    int64_t nb_sectors = bdrv_nb_sectors(bs);
+    size_t offset;
+
+    if (nb_sectors < 0) {
+        error_setg(errp, "Can't calculate number of disk sectors.");
+        return nb_sectors;
+    }
+
+    if (s->bitmap_directory != NULL) {
+        /* already read */
+        error_setg(errp, "Try read bitmaps, when they are already read.");
+        return -EEXIST;
+    }
+
+    s->bitmap_directory = g_try_malloc0(s->bitmap_directory_size);
+    if (s->bitmap_directory == NULL) {
+        error_setg(errp, "Can't allocate space for bitmap directory.");
+        return -ENOMEM;
+    }
+
+    ret = bdrv_pread(bs->file->bs,
+                     s->bitmap_directory_offset,
+                     s->bitmap_directory,
+                     s->bitmap_directory_size);
+    if (ret < 0) {
+        error_setg(errp, "Can't read bitmap directory.");
+        goto fail;
+    }
+
+    offset = 0;
+    end = s->bitmaps + s->nb_bitmaps;
+    for (bm = s->bitmaps; bm < end; ++bm) {
+        QCow2BitmapHeader *h =
+                (QCow2BitmapHeader *)(s->bitmap_directory + offset);
+
+        if (offset >= s->bitmap_directory_size) {
+            error_setg(errp, "Broken bitmap directory.");
+            goto fail;
+        }
+
+        bitmap_header_to_cpu(h);
+
+        ret = check_constraints(h, s->cluster_size,
+                                nb_sectors << BDRV_SECTOR_BITS);
+        if (ret < 0) {
+            error_setg(errp, "Bitmap doesn't satisfy the constraints.");
+            goto fail;
+        }
+
+        bm->offset = offset;
+        bm->name = g_strndup((char *)(h + 1), h->name_size);
+
+        offset += dir_entry_size(h);
+    }
+    return 0;
+
+fail:
+    g_free(s->bitmap_directory);
+    s->bitmap_directory = NULL;
+
+    return ret;
+}
+
+int qcow2_read_bitmaps(BlockDriverState *bs, Error **errp)
+{
+    int ret;
+    BDRVQcow2State *s = bs->opaque;
+
+    if (s->bitmap_directory != NULL || s->bitmaps != NULL) {
+        /* already read */
+        error_setg(errp, "Try read bitmaps, when they are already read.");
+        return -EEXIST;
+    }
+
+    if (s->nb_bitmaps == 0) {
+        /* No bitmaps - nothing to do */
+        return 0;
+    }
+
+    s->bitmaps = g_try_new0(QCow2Bitmap, s->nb_bitmaps);
+    if (s->bitmaps == NULL) {
+        error_setg(errp, "Can't allocate space for qcow2 bitmaps.");
+        ret = -ENOMEM;
+        goto fail;
+    }
+
+    ret = directory_read(bs, errp);
+    if (ret < 0) {
+        goto fail;
+    }
+
+    return 0;
+
+fail:
+    qcow2_free_bitmaps(bs);
+
+    return ret;
+}
diff --git a/block/qcow2.h b/block/qcow2.h
index 3f7429e..48fb2a5 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -297,6 +297,12 @@ typedef struct BDRVQcow2State {
     unsigned int nb_snapshots;
     QCowSnapshot *snapshots;
 
+    uint64_t bitmap_directory_offset;
+    uint64_t bitmap_directory_size;
+    uint8_t *bitmap_directory;
+    unsigned int nb_bitmaps;
+    QCow2Bitmap *bitmaps;
+
     int flags;
     int qcow_version;
     bool use_lazy_refcounts;
@@ -610,6 +616,10 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,
 void qcow2_free_snapshots(BlockDriverState *bs);
 int qcow2_read_snapshots(BlockDriverState *bs);
 
+/* qcow2-dirty-bitmap.c functions */
+void qcow2_free_bitmaps(BlockDriverState *bs);
+int qcow2_read_bitmaps(BlockDriverState *bs, Error **errp);
+
 /* qcow2-cache.c functions */
 Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables);
 int qcow2_cache_destroy(BlockDriverState* bs, Qcow2Cache *c);
-- 
1.8.3.1

[Qemu-devel] [PATCH 09/22] qcow2-dirty-bitmap: add qcow2_bitmap_load()

[Vladimir Sementsov-Ogievskiy]

This function loads block dirty bitmap from qcow2.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-dirty-bitmap.c | 110 +++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c              |   2 +
 block/qcow2.h              |   3 ++
 include/block/block_int.h  |   4 ++
 4 files changed, 119 insertions(+)

diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index e57058c..c9f7ef1 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -107,6 +107,13 @@ static int check_constraints(QCow2BitmapHeader *h, int cluster_size,
     return fail ? -EINVAL : 0;
 }
 
+static QCow2BitmapHeader *bitmap_header(BDRVQcow2State *s,
+                                        QCow2Bitmap *bitmap)
+{
+    return (QCow2BitmapHeader *)
+           (s->bitmap_directory + bitmap->offset);
+}
+
 static int directory_read(BlockDriverState *bs, Error **errp)
 {
     int ret;
@@ -210,3 +217,106 @@ fail:
 
     return ret;
 }
+
+static QCow2Bitmap *find_bitmap_by_name(BlockDriverState *bs, const char *name)
+{
+    BDRVQcow2State *s = bs->opaque;
+    QCow2Bitmap *bm, *end = s->bitmaps + s->nb_bitmaps;
+
+    for (bm = s->bitmaps; bm < end; ++bm) {
+        if (strcmp(bm->name, name) == 0) {
+            return bm;
+        }
+    }
+
+    return NULL;
+}
+
+/* load_bitmap_data()
+ * load dirty bitmap from bitmap table
+ * Bitmap table entries are assumed to be in big endian format */
+static int load_bitmap_data(BlockDriverState *bs, const uint64_t *bitmap_table,
+                            uint32_t bitmap_table_size, BdrvDirtyBitmap *bitmap)
+{
+    int ret = 0;
+    BDRVQcow2State *s = bs->opaque;
+    uint32_t i;
+    uint64_t *tab = g_memdup(bitmap_table,
+                             bitmap_table_size * sizeof(uint64_t));
+
+    for (i = 0; i < bitmap_table_size; ++i) {
+        be64_to_cpus(tab + i);
+    }
+
+    ret = bdrv_dirty_bitmap_load(bitmap, bs->file->bs, tab, bitmap_table_size,
+                                 s->cluster_size);
+
+    g_free(tab);
+
+    return ret;
+}
+
+static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs, QCow2Bitmap *bm,
+                                    Error **errp)
+{
+    BDRVQcow2State *s = bs->opaque;
+    int ret;
+    QCow2BitmapHeader *bmh;
+    uint64_t *bitmap_table = NULL;
+    uint32_t granularity;
+    BdrvDirtyBitmap *bitmap = NULL;
+
+    bmh = bitmap_header(s, bm);
+
+    bitmap_table = g_try_malloc(bmh->bitmap_table_size * sizeof(uint64_t));
+    if (bitmap_table == NULL) {
+        error_setg_errno(errp, -ENOMEM,
+                         "Could not allocate bitmap table");
+        return NULL;
+    }
+
+    ret = bdrv_pread(bs->file->bs, bmh->bitmap_table_offset,
+                     bitmap_table,
+                     bmh->bitmap_table_size * sizeof(uint64_t));
+    if (ret < 0) {
+        error_setg_errno(errp, -ret,
+                         "Could not read bitmap_table table from image");
+        goto fail;
+    }
+
+    granularity = 1U << bmh->granularity_bits;
+    bitmap = bdrv_create_dirty_bitmap(bs, granularity, bm->name, errp);
+    if (bitmap == NULL) {
+        goto fail;
+    }
+
+    ret = load_bitmap_data(bs, bitmap_table, bmh->bitmap_table_size, bitmap);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not read bitmap from image");
+        goto fail;
+    }
+
+    g_free(bitmap_table);
+    return bitmap;
+
+fail:
+    g_free(bitmap_table);
+    bdrv_release_dirty_bitmap(bs, bitmap);
+
+    return NULL;
+}
+
+BdrvDirtyBitmap *qcow2_bitmap_load(BlockDriverState *bs, const char *name,
+                                   Error **errp)
+{
+    QCow2Bitmap *bm;
+
+    bm = find_bitmap_by_name(bs, name);
+    if (bm == NULL) {
+        error_setg(errp, "Could not find bitmap '%s' in the node '%s'", name,
+                   bdrv_get_device_or_node_name(bs));
+        return NULL;
+    }
+
+    return load_bitmap(bs, bm, errp);
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index 1ce6264..5f54528 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3352,6 +3352,8 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_get_info          = qcow2_get_info,
     .bdrv_get_specific_info = qcow2_get_specific_info,
 
+    .bdrv_dirty_bitmap_load = qcow2_bitmap_load,
+
     .bdrv_save_vmstate    = qcow2_save_vmstate,
     .bdrv_load_vmstate    = qcow2_load_vmstate,
 
diff --git a/block/qcow2.h b/block/qcow2.h
index 48fb2a5..cc4c776 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -620,6 +620,9 @@ int qcow2_read_snapshots(BlockDriverState *bs);
 void qcow2_free_bitmaps(BlockDriverState *bs);
 int qcow2_read_bitmaps(BlockDriverState *bs, Error **errp);
 
+BdrvDirtyBitmap *qcow2_bitmap_load(BlockDriverState *bs, const char *name,
+                                   Error **errp);
+
 /* qcow2-cache.c functions */
 Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables);
 int qcow2_cache_destroy(BlockDriverState* bs, Qcow2Cache *c);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index dda5ba0..d0e3db8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -215,6 +215,10 @@ struct BlockDriver {
     int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
     ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs);
 
+    BdrvDirtyBitmap *(*bdrv_dirty_bitmap_load)(BlockDriverState *bs,
+                                               const char *name,
+                                               Error **errp);
+
     int (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov,
                              int64_t pos);
     int (*bdrv_load_vmstate)(BlockDriverState *bs, uint8_t *buf,
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/22] qcow2-dirty-bitmap: add qcow2_bitmap_store()

[Vladimir Sementsov-Ogievskiy]

This function stores block dirty bitmap to qcow2. If the bitmap with
the same name, size and granularity already exists, it will be
rewritten, if the bitmap with the same name exists but granularity or
size does not match, an error will be genrated.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-dirty-bitmap.c | 443 +++++++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c              |   1 +
 block/qcow2.h              |   2 +
 include/block/block_int.h  |   3 +
 4 files changed, 449 insertions(+)

diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index c9f7ef1..28ed309 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -76,6 +76,15 @@ static void bitmap_header_to_cpu(QCow2BitmapHeader *h)
     be32_to_cpus(&h->extra_data_size);
 }
 
+static void bitmap_header_to_be(QCow2BitmapHeader *h)
+{
+    cpu_to_be64s(&h->bitmap_table_offset);
+    cpu_to_be32s(&h->bitmap_table_size);
+    cpu_to_be32s(&h->flags);
+    cpu_to_be16s(&h->name_size);
+    cpu_to_be32s(&h->extra_data_size);
+}
+
 static int calc_dir_entry_size(size_t name_size)
 {
     return align_offset(sizeof(QCow2BitmapHeader) + name_size, 8);
@@ -86,6 +95,17 @@ static int dir_entry_size(QCow2BitmapHeader *h)
     return calc_dir_entry_size(h->name_size);
 }
 
+static void directory_to_be(uint8_t *dir, size_t size)
+{
+    uint8_t *end = dir + size;
+    while (dir < end) {
+        QCow2BitmapHeader *h = (QCow2BitmapHeader *)dir;
+        dir += dir_entry_size(h);
+
+        bitmap_header_to_be(h);
+    }
+}
+
 static int check_constraints(QCow2BitmapHeader *h, int cluster_size,
                              uint64_t disk_size)
 {
@@ -320,3 +340,426 @@ BdrvDirtyBitmap *qcow2_bitmap_load(BlockDriverState *bs, const char *name,
 
     return load_bitmap(bs, bm, errp);
 }
+
+static int update_header_sync(BlockDriverState *bs)
+{
+    int ret;
+
+    ret = qcow2_update_header(bs);
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = bdrv_flush(bs);
+    if (ret < 0) {
+        return ret;
+    }
+
+    return 0;
+}
+
+/* write bitmap directory from the state to new allocated clusters */
+static int64_t directory_write(BlockDriverState *bs, const uint8_t *dir,
+                               size_t size)
+{
+    int ret = 0;
+    uint8_t *dir_be = NULL;
+    int64_t dir_offset = 0;
+
+    dir_be = g_try_malloc(size);
+    if (dir_be == NULL) {
+        return -ENOMEM;
+    }
+    memcpy(dir_be, dir, size);
+    directory_to_be(dir_be, size);
+
+    /* Allocate space for the new bitmap directory */
+    dir_offset = qcow2_alloc_clusters(bs, size);
+    if (dir_offset < 0) {
+        ret = dir_offset;
+        goto out;
+    }
+
+    /* The bitmap directory position has not yet been updated, so these
+     * clusters must indeed be completely free */
+    ret = qcow2_pre_write_overlap_check(bs, 0, dir_offset, size);
+    if (ret < 0) {
+        goto out;
+    }
+
+    ret = bdrv_pwrite(bs->file->bs, dir_offset, dir_be, size);
+    if (ret < 0) {
+        goto out;
+    }
+
+out:
+    g_free(dir_be);
+
+    if (ret < 0) {
+        if (dir_offset > 0) {
+            qcow2_free_clusters(bs, dir_offset, size, QCOW2_DISCARD_ALWAYS);
+        }
+
+        return ret;
+    }
+
+    return dir_offset;
+}
+
+static int directory_push_entry(BlockDriverState *bs, QCow2BitmapHeader *header)
+{
+    BDRVQcow2State *s = bs->opaque;
+    int ret;
+    int entry_size = dir_entry_size(header);
+    int64_t new_offset = 0, old_offset = 0;
+    uint64_t new_size = s->bitmap_directory_size + entry_size, old_size = 0;
+    void *p;
+    int64_t nb_sectors = bdrv_nb_sectors(bs);
+
+    if (nb_sectors < 0) {
+        return nb_sectors;
+    }
+
+    if (new_size > QCOW_MAX_DIRTY_BITMAP_DIRECTORY_SIZE) {
+        return -EINVAL;
+    }
+
+    ret = check_constraints(header, s->cluster_size,
+                            nb_sectors << BDRV_SECTOR_BITS);
+    if (ret < 0) {
+        return -EINVAL;
+    }
+
+    old_offset = s->bitmap_directory_offset;
+    old_size = s->bitmap_directory_size;
+
+    uint8_t *new_dir = g_try_malloc(new_size);
+    if (new_dir == NULL) {
+        return -ENOMEM;
+    }
+    memcpy(new_dir, s->bitmap_directory, s->bitmap_directory_size);
+    memcpy(new_dir + s->bitmap_directory_size, header, entry_size);
+
+    new_offset = directory_write(bs, new_dir, new_size);
+    if (new_offset < 0) {
+        ret = new_offset;
+        goto fail;
+    }
+
+    ret = bdrv_flush(bs);
+    if (ret < 0) {
+        goto fail;
+    }
+
+    s->bitmap_directory_offset = new_offset;
+    s->bitmap_directory_size = new_size;
+
+    ret = update_header_sync(bs);
+    if (ret < 0) {
+        goto fail;
+    }
+
+    if (old_size) {
+        qcow2_free_clusters(bs, old_offset, old_size, QCOW2_DISCARD_ALWAYS);
+    }
+
+    g_free(s->bitmap_directory);
+    s->bitmap_directory = new_dir;
+
+    return 0;
+
+fail:
+    g_free(new_dir);
+    if (new_offset > 0) {
+        qcow2_free_clusters(bs, new_offset, new_size, QCOW2_DISCARD_ALWAYS);
+        s->bitmap_directory_offset = old_offset;
+        s->bitmap_directory_size = old_size;
+    }
+
+    p = g_try_realloc(s->bitmap_directory, s->bitmap_directory_size);
+    if (p != NULL) {
+        s->bitmap_directory = p;
+    }
+
+    return ret;
+}
+
+/* store_bitmap()
+ * update bitmap table by storing bitmap to it.
+ * Bitmap table entries are assumed to be in big endian format
+ * On the error, the resulting bitmap table is valid for clearing, but
+ * may contain invalid bitmap */
+static int store_bitmap(BlockDriverState *bs, uint64_t *bitmap_table,
+                        uint32_t bitmap_table_size,
+                        const BdrvDirtyBitmap *bitmap)
+{
+    int ret;
+    BDRVQcow2State *s = bs->opaque;
+    int cl_size = s->cluster_size;
+    uint32_t i, tab_size = 0;
+    uint64_t *tab;
+    uint32_t j;
+
+    bdrv_dirty_bitmap_prepare_store(bitmap, s->cluster_size, NULL, &tab_size);
+    tab = g_try_malloc(tab_size * sizeof(tab[0]));
+    if (tab == NULL) {
+        return -ENOMEM;
+    }
+
+    ret = bdrv_dirty_bitmap_prepare_store(bitmap, s->cluster_size,
+                                          tab, &tab_size);
+    if (ret != 0) {
+        g_free(tab);
+        return ret;
+    }
+
+    for (i = 0, j = 0; i < bitmap_table_size; ++i) {
+        if (tab[i] <= 1) {
+            continue;
+        }
+
+        for ( ; j < bitmap_table_size && bitmap_table[j] <= 1; ++j) {
+            ;
+        }
+
+        if (j < bitmap_table_size) {
+            tab[i] = be64_to_cpu(bitmap_table[j]);
+        } else {
+            tab[i] = qcow2_alloc_clusters(bs, cl_size);
+        }
+    }
+
+    for ( ; j < bitmap_table_size; ++j) {
+        uint64_t addr = be64_to_cpu(bitmap_table[j]);
+        if (addr <= 1) {
+            continue;
+        }
+
+        qcow2_free_clusters(bs, addr, cl_size, QCOW2_DISCARD_ALWAYS);
+        bitmap_table[j] = 0;
+    }
+
+    bdrv_dirty_bitmap_store(bitmap, bs->file->bs, tab,
+                            bitmap_table_size, s->cluster_size);
+
+    for (i = 0; i < bitmap_table_size; ++i) {
+        bitmap_table[i] = cpu_to_be64(tab[i]);
+    }
+
+    g_free(tab);
+
+    return 0;
+}
+
+static int64_t alloc_zeroed_clusters(BlockDriverState *bs, uint64_t size)
+{
+    int ret = 0;
+    void *buf = NULL;
+    int64_t offset = qcow2_alloc_clusters(bs, size);
+    if (offset < 0) {
+        return offset;
+    }
+
+    buf = g_try_malloc0(size);
+    if (buf == NULL) {
+        ret = -ENOMEM;
+        goto out;
+    }
+
+    ret = qcow2_pre_write_overlap_check(bs, 0, offset, size);
+    if (ret < 0) {
+        goto out;
+    }
+
+    ret = bdrv_pwrite(bs->file->bs, offset, buf, size);
+    if (ret < 0) {
+        goto out;
+    }
+
+    ret = bdrv_flush(bs);
+    if (ret < 0) {
+        goto out;
+    }
+
+out:
+    g_free(buf);
+
+    if (ret < 0) {
+        qcow2_free_clusters(bs, offset, size, QCOW2_DISCARD_ALWAYS);
+        return ret;
+    }
+
+    return offset;
+}
+
+static int directory_push(BlockDriverState *bs, const char *name,
+                          uint64_t size, int granularity)
+{
+    int ret;
+    BDRVQcow2State *s = bs->opaque;
+    int sector_granularity = granularity >> BDRV_SECTOR_BITS;
+    size_t name_size = strlen(name);
+    size_t entry_size = calc_dir_entry_size(name_size);
+    QCow2BitmapHeader *entry = g_malloc0(entry_size);
+    int64_t table_offset = 0;
+
+    entry->granularity_bits = ctz32(granularity);
+    entry->type = BT_DIRTY_TRACKING_BITMAP;
+    entry->name_size = name_size;
+    memcpy(entry + 1, name, name_size);
+
+    entry->bitmap_table_size =
+            size_to_clusters(s, (((size - 1) / sector_granularity) >> 3) + 1);
+    table_offset = alloc_zeroed_clusters(bs, entry->bitmap_table_size *
+                                  sizeof(uint64_t));
+    if (table_offset < 0) {
+        ret = table_offset;
+        goto out;
+    }
+    entry->bitmap_table_offset = table_offset;
+
+    ret = directory_push_entry(bs, entry);
+    if (ret < 0) {
+        goto out;
+    }
+
+out:
+    g_free(entry);
+    if (ret < 0 && table_offset > 0) {
+        qcow2_free_clusters(bs, table_offset, entry->bitmap_table_size *
+                            sizeof(uint64_t), QCOW2_DISCARD_ALWAYS);
+    }
+
+    return ret;
+}
+
+static int bitmaps_push(BDRVQcow2State *s, const char *name, uint64_t offset)
+{
+    QCow2Bitmap *bm;
+    QCow2Bitmap *p;
+
+    printf("dirty bitmaps push\n");
+    p = g_try_renew(QCow2Bitmap, s->bitmaps, s->nb_bitmaps + 1);
+    if (p == NULL) {
+        return -ENOMEM;
+    }
+    s->bitmaps = p;
+    s->nb_bitmaps++;
+
+    bm = s->bitmaps + s->nb_bitmaps - 1;
+    bm->name = g_strdup(name);
+    bm->offset = offset;
+
+
+    return 0;
+}
+
+static void bitmaps_pop(BDRVQcow2State *s)
+{
+    QCow2Bitmap *p;
+
+    if (s->nb_bitmaps == 0) {
+        return;
+    }
+
+    p = g_try_renew(QCow2Bitmap, s->bitmaps, s->nb_bitmaps - 1);
+    if (p != NULL) {
+        s->bitmaps = p;
+    }
+
+    s->nb_bitmaps--;
+}
+
+/* if no id is provided, a new one is constructed */
+static int qcow2_bitmap_create(BlockDriverState *bs, const char *name,
+                               uint64_t size, int granularity)
+{
+    int ret;
+    BDRVQcow2State *s = bs->opaque;
+
+    if (s->nb_bitmaps >= QCOW_MAX_DIRTY_BITMAPS) {
+        return -EFBIG;
+    }
+
+    /* Check that the name is unique */
+    if (find_bitmap_by_name(bs, name) != NULL) {
+        return -EEXIST;
+    }
+
+    ret = bitmaps_push(s, name, s->bitmap_directory_size);
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = directory_push(bs, name, size, granularity);
+    if (ret < 0) {
+        bitmaps_pop(s);
+        return ret;
+    }
+
+    return 0;
+}
+
+void qcow2_bitmap_store(BlockDriverState *bs,
+                        const BdrvDirtyBitmap *bitmap, Error **errp)
+{
+    BDRVQcow2State *s = bs->opaque;
+    int ret = 0;
+    uint64_t *bitmap_table;
+    QCow2Bitmap *bm;
+    QCow2BitmapHeader *bmh;
+    const char *name = bdrv_dirty_bitmap_name(bitmap);
+    uint64_t size = bdrv_dirty_bitmap_size(bitmap);
+    int granularity = bdrv_dirty_bitmap_granularity(bitmap);
+
+    /* find/create dirty bitmap */
+    bm = find_bitmap_by_name(bs, name);
+    if (bm == NULL) {
+        ret = qcow2_bitmap_create(bs, name, size, granularity);
+        if (ret < 0) {
+            error_setg_errno(errp, ret, "Can't create dirty bitmap in qcow2.");
+        }
+        bm = s->bitmaps + s->nb_bitmaps - 1;
+        bmh = bitmap_header(s, bm);
+    } else {
+        bmh = bitmap_header(s, bm);
+
+        if (granularity != (1U << bmh->granularity_bits)) {
+            error_setg(errp,
+                       "The bitmap with same name (but other granularity) "
+                       "already exists.");
+            return;
+        }
+    }
+
+    bitmap_table = g_try_new(uint64_t, bmh->bitmap_table_size);
+    if (bitmap_table == NULL) {
+        error_setg(errp, "No memory.");
+        return;
+    }
+    ret = bdrv_pread(bs->file->bs, bmh->bitmap_table_offset,
+                     bitmap_table,
+                     bmh->bitmap_table_size * sizeof(uint64_t));
+    if (ret < 0) {
+        error_setg_errno(errp, ret, "Can't read dirty bitmap table.");
+        goto finish;
+    }
+
+    ret = store_bitmap(bs, bitmap_table, bmh->bitmap_table_size,
+                       bitmap);
+    if (ret < 0) {
+        error_setg_errno(errp, ret, "Can't store bitmap table.");
+        goto finish;
+    }
+
+    ret = bdrv_pwrite(bs->file->bs, bmh->bitmap_table_offset,
+                      bitmap_table,
+                      bmh->bitmap_table_size * sizeof(uint64_t));
+    if (ret < 0) {
+        error_setg_errno(errp, ret, "Can't write dirty bitmap table.");
+        goto finish;
+    }
+
+finish:
+    g_free(bitmap_table);
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index 5f54528..20d095b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3353,6 +3353,7 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_get_specific_info = qcow2_get_specific_info,
 
     .bdrv_dirty_bitmap_load = qcow2_bitmap_load,
+    .bdrv_dirty_bitmap_store = qcow2_bitmap_store,
 
     .bdrv_save_vmstate    = qcow2_save_vmstate,
     .bdrv_load_vmstate    = qcow2_load_vmstate,
diff --git a/block/qcow2.h b/block/qcow2.h
index cc4c776..e4a517c 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -622,6 +622,8 @@ int qcow2_read_bitmaps(BlockDriverState *bs, Error **errp);
 
 BdrvDirtyBitmap *qcow2_bitmap_load(BlockDriverState *bs, const char *name,
                                    Error **errp);
+void qcow2_bitmap_store(BlockDriverState *bs, const BdrvDirtyBitmap *bitmap,
+                        Error **errp);
 
 /* qcow2-cache.c functions */
 Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index d0e3db8..7cd05e1 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -218,6 +218,9 @@ struct BlockDriver {
     BdrvDirtyBitmap *(*bdrv_dirty_bitmap_load)(BlockDriverState *bs,
                                                const char *name,
                                                Error **errp);
+    void (*bdrv_dirty_bitmap_store)(BlockDriverState *bs,
+                                    const BdrvDirtyBitmap *bitmap,
+                                    Error **errp);
 
     int (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov,
                              int64_t pos);
-- 
1.8.3.1

[Qemu-devel] [PATCH 11/22] qcow2: add dirty bitmaps extension

[Vladimir Sementsov-Ogievskiy]

Add dirty bitmap extension as specified in docs/specs/qcow2.txt.

Load bitmap headers on open. Handle close and update_header.

Handle resize: for now, just block resize if there are dirty bitmaps.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 20d095b..bda3026 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -62,6 +62,7 @@ typedef struct {
 #define  QCOW2_EXT_MAGIC_END 0
 #define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
 #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
+#define  QCOW2_EXT_MAGIC_DIRTY_BITMAPS 0x23852875
 
 static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
 {
@@ -91,6 +92,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
     QCowExtension ext;
     uint64_t offset;
     int ret;
+    Qcow2BitmapHeaderExt bitmaps_ext;
 
 #ifdef DEBUG_EXT
     printf("qcow2_read_extensions: start=%ld end=%ld\n", start_offset, end_offset);
@@ -161,6 +163,67 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
             }
             break;
 
+        case QCOW2_EXT_MAGIC_DIRTY_BITMAPS:
+            ret = bdrv_pread(bs->file->bs, offset, &bitmaps_ext, ext.len);
+            if (ret < 0) {
+                error_setg_errno(errp, -ret, "ERROR: bitmaps_ext: "
+                                 "Could not read ext header");
+                return ret;
+            }
+
+            if (bitmaps_ext.reserved32 != 0) {
+                error_setg_errno(errp, -ret, "ERROR: bitmaps_ext: "
+                                 "Reserved field is not zero.");
+                return -EINVAL;
+            }
+
+            be32_to_cpus(&bitmaps_ext.nb_bitmaps);
+            be64_to_cpus(&bitmaps_ext.bitmap_directory_size);
+            be64_to_cpus(&bitmaps_ext.bitmap_directory_offset);
+
+            if (bitmaps_ext.nb_bitmaps > QCOW_MAX_DIRTY_BITMAPS) {
+                error_setg(errp, "ERROR: bitmaps_ext: "
+                                 "too many dirty bitmaps");
+                return -EINVAL;
+            }
+
+            if (bitmaps_ext.nb_bitmaps == 0) {
+                error_setg(errp, "ERROR: bitmaps_ext: "
+                                 "found bitmaps extension with zero bitmaps");
+                return -EINVAL;
+            }
+
+            if (bitmaps_ext.bitmap_directory_offset & (s->cluster_size - 1)) {
+                error_setg(errp, "ERROR: bitmaps_ext: "
+                                 "wrong dirty bitmap directory offset");
+                return -EINVAL;
+            }
+
+            if (bitmaps_ext.bitmap_directory_size >
+                QCOW_MAX_DIRTY_BITMAP_DIRECTORY_SIZE) {
+                error_setg(errp, "ERROR: bitmaps_ext: "
+                                 "too large dirty bitmap directory");
+                return -EINVAL;
+            }
+
+            s->nb_bitmaps = bitmaps_ext.nb_bitmaps;
+            s->bitmap_directory_offset =
+                    bitmaps_ext.bitmap_directory_offset;
+            s->bitmap_directory_size =
+                    bitmaps_ext.bitmap_directory_size;
+
+            ret = qcow2_read_bitmaps(bs, errp);
+            if (ret < 0) {
+                return ret;
+            }
+
+#ifdef DEBUG_EXT
+            printf("Qcow2: Got dirty bitmaps extension:"
+                   " offset=%" PRIu64 " nb_bitmaps=%" PRIu32 "\n",
+                   s->bitmaps_offset, s->nb_bitmaps);
+#endif
+            break;
+
         default:
             /* unknown magic - save it in case we need to rewrite the header */
             {
@@ -1178,6 +1241,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     g_free(s->unknown_header_fields);
     cleanup_unknown_header_ext(bs);
     qcow2_free_snapshots(bs);
+    qcow2_free_bitmaps(bs);
     qcow2_refcount_close(bs);
     qemu_vfree(s->l1_table);
     /* else pre-write overlap checks in cache_destroy may crash */
@@ -1742,6 +1806,7 @@ static void qcow2_close(BlockDriverState *bs)
     qemu_vfree(s->cluster_data);
     qcow2_refcount_close(bs);
     qcow2_free_snapshots(bs);
+    qcow2_free_bitmaps(bs);
 }
 
 static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
@@ -1939,6 +2004,24 @@ int qcow2_update_header(BlockDriverState *bs)
         buflen -= ret;
     }
 
+    if (s->nb_bitmaps > 0) {
+        Qcow2BitmapHeaderExt bitmaps_header = {
+            .nb_bitmaps = cpu_to_be32(s->nb_bitmaps),
+            .bitmap_directory_size =
+                    cpu_to_be64(s->bitmap_directory_size),
+            .bitmap_directory_offset =
+                    cpu_to_be64(s->bitmap_directory_offset)
+        };
+        ret = header_ext_add(buf, QCOW2_EXT_MAGIC_DIRTY_BITMAPS,
+                             &bitmaps_header, sizeof(bitmaps_header),
+                             buflen);
+        if (ret < 0) {
+            goto fail;
+        }
+        buf += ret;
+        buflen -= ret;
+    }
+
     /* Keep unknown header extensions */
     QLIST_FOREACH(uext, &s->unknown_header_ext, next) {
         ret = header_ext_add(buf, uext->magic, uext->data, uext->len, buflen);
@@ -2466,6 +2549,12 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset)
         return -ENOTSUP;
     }
 
+    /* cannot proceed if image has bitmaps */
+    if (s->nb_bitmaps) {
+        error_report("Can't resize an image which has dirty bitmaps");
+        return -ENOTSUP;
+    }
+
     /* shrinking is currently not supported */
     if (offset < bs->total_sectors * 512) {
         error_report("qcow2 doesn't support shrinking images yet");
-- 
1.8.3.1

[Qemu-devel] [PATCH 12/22] qcow2-dirty-bitmap: add qcow2_bitmap_load_check()

[Vladimir Sementsov-Ogievskiy]

The function checks existing of the bitmap without loading it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/dirty-bitmap.c         | 15 +++++++++++++++
 block/qcow2-dirty-bitmap.c   |  5 +++++
 block/qcow2.c                |  1 +
 block/qcow2.h                |  1 +
 include/block/block_int.h    |  2 ++
 include/block/dirty-bitmap.h |  1 +
 6 files changed, 25 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 816c6ee..7a44722 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -419,3 +419,18 @@ int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
 {
     return hbitmap_store(bitmap->bitmap, bs, table, table_size, cluster_size);
 }
+
+bool bdrv_load_check_dirty_bitmap(BlockDriverState *file, const char *name)
+{
+    BlockDriver *drv = file->drv;
+    if (!drv) {
+        return false;
+    }
+    if (drv->bdrv_dirty_bitmap_load_check) {
+        return drv->bdrv_dirty_bitmap_load_check(file, name);
+    }
+    if (file->file)  {
+        return bdrv_load_check_dirty_bitmap(file->file->bs, name);
+    }
+    return false;
+}
diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index 28ed309..24415df 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -276,6 +276,11 @@ static int load_bitmap_data(BlockDriverState *bs, const uint64_t *bitmap_table,
     return ret;
 }
 
+bool qcow2_bitmap_load_check(BlockDriverState *file, const char *name)
+{
+    return find_bitmap_by_name(file, name) != NULL;
+}
+
 static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs, QCow2Bitmap *bm,
                                     Error **errp)
 {
diff --git a/block/qcow2.c b/block/qcow2.c
index bda3026..7a342c2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3442,6 +3442,7 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_get_specific_info = qcow2_get_specific_info,
 
     .bdrv_dirty_bitmap_load = qcow2_bitmap_load,
+    .bdrv_dirty_bitmap_load_check = qcow2_bitmap_load_check,
     .bdrv_dirty_bitmap_store = qcow2_bitmap_store,
 
     .bdrv_save_vmstate    = qcow2_save_vmstate,
diff --git a/block/qcow2.h b/block/qcow2.h
index e4a517c..423c279 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -620,6 +620,7 @@ int qcow2_read_snapshots(BlockDriverState *bs);
 void qcow2_free_bitmaps(BlockDriverState *bs);
 int qcow2_read_bitmaps(BlockDriverState *bs, Error **errp);
 
+bool qcow2_bitmap_load_check(BlockDriverState *file, const char *name);
 BdrvDirtyBitmap *qcow2_bitmap_load(BlockDriverState *bs, const char *name,
                                    Error **errp);
 void qcow2_bitmap_store(BlockDriverState *bs, const BdrvDirtyBitmap *bitmap,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7cd05e1..66a388a 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -215,6 +215,8 @@ struct BlockDriver {
     int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
     ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs);
 
+    bool (*bdrv_dirty_bitmap_load_check)(BlockDriverState *file,
+                                         const char *name);
     BdrvDirtyBitmap *(*bdrv_dirty_bitmap_load)(BlockDriverState *bs,
                                                const char *name,
                                                Error **errp);
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 20cb540..f3cedaa 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -53,5 +53,6 @@ int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
 int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
                             const uint64_t *table, uint32_t table_size,
                             uint32_t cluster_size);
+bool bdrv_load_check_dirty_bitmap(BlockDriverState *file, const char *name);
 
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 13/22] block: store persistent dirty bitmaps

[Vladimir Sementsov-Ogievskiy]

Persistent dirty bitmaps are the bitmaps, for which the new field
BdrvDirtyBitmap.file is not NULL. We save all persistent dirty bitmaps
owned by BlockDriverState in corresponding bdrv_close().
BdrvDirtyBitmap.file is a BlockDriverState, where we want to save the
bitmap. It may be set in bdrv_dirty_bitmap_set_file() only once.
bdrv_ref/bdrv_unref are used for BdrvDirtyBitmap.file to be sure that
files will be closed and resources will be freed.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block.c                      |  2 ++
 block/dirty-bitmap.c         | 35 +++++++++++++++++++++++++++++++++++
 include/block/dirty-bitmap.h |  5 +++++
 3 files changed, 42 insertions(+)

diff --git a/block.c b/block.c
index 59a18a3..b54875e 100644
--- a/block.c
+++ b/block.c
@@ -2144,6 +2144,8 @@ static void bdrv_close(BlockDriverState *bs)
     bdrv_flush(bs);
     bdrv_drain(bs); /* in case flush left pending I/O */
 
+    /* save and release persistent dirty bitmaps */
+    bdrv_finalize_persistent_dirty_bitmaps(bs);
     bdrv_release_named_dirty_bitmaps(bs);
     assert(QLIST_EMPTY(&bs->dirty_bitmaps));
 
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 7a44722..c9e999f 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -42,6 +42,7 @@ struct BdrvDirtyBitmap {
     char *name;                 /* Optional non-empty unique ID */
     int64_t size;               /* Size of the bitmap (Number of sectors) */
     bool disabled;              /* Bitmap is read-only */
+    bool internal_persistent;   /* bitmap must be saved to owner disk image */
     QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
@@ -434,3 +435,37 @@ bool bdrv_load_check_dirty_bitmap(BlockDriverState *file, const char *name)
     }
     return false;
 }
+
+void bdrv_store_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+                             Error **errp)
+{
+    if (bs == NULL || bs->drv == NULL ||
+            bs->drv->bdrv_dirty_bitmap_store == NULL) {
+        error_setg(errp, "Storing bitmap is unsupported for the format.");
+        return;
+    }
+
+    bs->drv->bdrv_dirty_bitmap_store(bs, bitmap, errp);
+}
+
+void bdrv_dirty_bitmap_set_internal_persistance(BdrvDirtyBitmap *bitmap,
+                                                bool persistent)
+{
+    bitmap->internal_persistent = persistent;
+}
+
+void bdrv_finalize_persistent_dirty_bitmaps(BlockDriverState *bs)
+{
+    BdrvDirtyBitmap *bm, *bm_next;
+
+    QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, bm_next) {
+        if (bm->internal_persistent) {
+            Error *local_err = NULL;
+            bdrv_store_dirty_bitmap(bs, bm, &local_err);
+            if (local_err) {
+                error_report_err(local_err);
+            }
+            bdrv_release_dirty_bitmap(bs, bm);
+        }
+    }
+}
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index f3cedaa..37b5f23 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -54,5 +54,10 @@ int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
                             const uint64_t *table, uint32_t table_size,
                             uint32_t cluster_size);
 bool bdrv_load_check_dirty_bitmap(BlockDriverState *file, const char *name);
+void bdrv_dirty_bitmap_set_internal_persistance(BdrvDirtyBitmap *bitmap,
+                                                bool persistent);
+void bdrv_store_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
+                             Error **errp);
+void bdrv_finalize_persistent_dirty_bitmaps(BlockDriverState *bs);
 
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 14/22] block: add bdrv_load_dirty_bitmap()

[Vladimir Sementsov-Ogievskiy]

The funcion loads dirty bitmap from file, using underlying driver
function.

Note: the function doesn't change BdrvDirtyBitmap.file field. This field
is only used by bdrv_store_dirty_bitmap() function and is ONLY written
by bdrv_dirty_bitmap_set_file() function.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/dirty-bitmap.c         | 16 ++++++++++++++++
 include/block/dirty-bitmap.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index c9e999f..87ee4d7 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -469,3 +469,19 @@ void bdrv_finalize_persistent_dirty_bitmaps(BlockDriverState *bs)
         }
     }
 }
+
+BdrvDirtyBitmap *bdrv_load_dirty_bitmap(BlockDriverState *bs, const char *name,
+                                        Error **errp)
+{
+    BlockDriver *drv = bs->drv;
+    if (!drv) {
+        return NULL;
+    }
+    if (drv->bdrv_dirty_bitmap_load) {
+        return drv->bdrv_dirty_bitmap_load(bs, name, errp);
+    }
+    if (bs->file)  {
+        return bdrv_load_dirty_bitmap(bs, name, errp);
+    }
+    return NULL;
+}
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 37b5f23..66ba3f8 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -59,5 +59,7 @@ void bdrv_dirty_bitmap_set_internal_persistance(BdrvDirtyBitmap *bitmap,
 void bdrv_store_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
                              Error **errp);
 void bdrv_finalize_persistent_dirty_bitmaps(BlockDriverState *bs);
+BdrvDirtyBitmap *bdrv_load_dirty_bitmap(BlockDriverState *bs, const char *name,
+                                        Error **errp);
 
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 15/22] qcow2-dirty-bitmap: add autoclear bit

[Vladimir Sementsov-Ogievskiy]

Add autoclear bit for handling rewriting image by old qemu version.

If autoclear bit is not set, but bitmaps extension is found it
would not be loaded and warning will be generated.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-dirty-bitmap.c |  4 ++++
 block/qcow2.c              | 12 ++++++++++--
 block/qcow2.h              |  9 +++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index 24415df..384ccea 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -419,6 +419,7 @@ static int directory_push_entry(BlockDriverState *bs, QCow2BitmapHeader *header)
     int64_t new_offset = 0, old_offset = 0;
     uint64_t new_size = s->bitmap_directory_size + entry_size, old_size = 0;
     void *p;
+    uint64_t old_autocl;
     int64_t nb_sectors = bdrv_nb_sectors(bs);
 
     if (nb_sectors < 0) {
@@ -437,6 +438,7 @@ static int directory_push_entry(BlockDriverState *bs, QCow2BitmapHeader *header)
 
     old_offset = s->bitmap_directory_offset;
     old_size = s->bitmap_directory_size;
+    old_autocl = s->autoclear_features;
 
     uint8_t *new_dir = g_try_malloc(new_size);
     if (new_dir == NULL) {
@@ -458,6 +460,7 @@ static int directory_push_entry(BlockDriverState *bs, QCow2BitmapHeader *header)
 
     s->bitmap_directory_offset = new_offset;
     s->bitmap_directory_size = new_size;
+    s->autoclear_features |= QCOW2_AUTOCLEAR_DIRTY_BITMAPS;
 
     ret = update_header_sync(bs);
     if (ret < 0) {
@@ -479,6 +482,7 @@ fail:
         qcow2_free_clusters(bs, new_offset, new_size, QCOW2_DISCARD_ALWAYS);
         s->bitmap_directory_offset = old_offset;
         s->bitmap_directory_size = old_size;
+        s->autoclear_features = old_autocl;
     }
 
     p = g_try_realloc(s->bitmap_directory, s->bitmap_directory_size);
diff --git a/block/qcow2.c b/block/qcow2.c
index 7a342c2..f269bab 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -164,6 +164,13 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
             break;
 
         case QCOW2_EXT_MAGIC_DIRTY_BITMAPS:
+            if (!(s->autoclear_features & QCOW2_AUTOCLEAR_DIRTY_BITMAPS)) {
+                fprintf(stderr,
+                        "WARNING: bitmaps_ext: autoclear flag is not "
+                        "set, all bitmaps will be considered as inconsistent");
+                break;
+            }
+
             ret = bdrv_pread(bs->file->bs, offset, &bitmaps_ext, ext.len);
             if (ret < 0) {
                 error_setg_errno(errp, -ret, "ERROR: bitmaps_ext: "
@@ -1205,8 +1212,9 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* Clear unknown autoclear feature bits */
-    if (!bs->read_only && !(flags & BDRV_O_INACTIVE) && s->autoclear_features) {
-        s->autoclear_features = 0;
+    if (!bs->read_only && !(flags & BDRV_O_INACTIVE) &&
+        (s->autoclear_features & ~QCOW2_AUTOCLEAR_MASK)) {
+        s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
         ret = qcow2_update_header(bs);
         if (ret < 0) {
             error_setg_errno(errp, -ret, "Could not update qcow2 header");
diff --git a/block/qcow2.h b/block/qcow2.h
index 423c279..63ea543 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -220,6 +220,15 @@ enum {
     QCOW2_COMPAT_FEAT_MASK            = QCOW2_COMPAT_LAZY_REFCOUNTS,
 };
 
+/* Autoclear feature bits */
+enum {
+    QCOW2_AUTOCLEAR_DIRTY_BITMAPS_BITNR = 0,
+    QCOW2_AUTOCLEAR_DIRTY_BITMAPS       =
+        1 << QCOW2_AUTOCLEAR_DIRTY_BITMAPS_BITNR,
+
+    QCOW2_AUTOCLEAR_MASK                = QCOW2_AUTOCLEAR_DIRTY_BITMAPS,
+};
+
 enum qcow2_discard_type {
     QCOW2_DISCARD_NEVER = 0,
     QCOW2_DISCARD_ALWAYS,
-- 
1.8.3.1

[Qemu-devel] [PATCH 16/22] qemu: command line option for dirty bitmaps

[Vladimir Sementsov-Ogievskiy]

The patch adds the following command line option:

-dirty-bitmap [option1=val1][,option2=val2]...

Avaliable options are:

name
The name of the bitmap.
Should be unique per 'file'/'node' and per 'for_node'.

node
The node to load and bind the bitmap.
It should be specified as 'id' suboption of one of '-node' options.

granularity
Granularity (in bytes) for created dirty bitmap.
If the bitmap is already exists in specified 'file'/'file_id'/device
it's granularity will not be changed but only checked (an error will be
generated if this check fails).

enabled
on|off
Enabled flag for the bitmap.
By default the bitmap will be enabled.

create
on|off
By default is off.
If on, then new bitmap will be created in the image, if the bitmap with
same name is already exists an error will be generated.
If off, then the bitmap will be loaded from the image, if there is no
one an error will be generated.
If create=off and granularity is specified then granularity will be
checked for loaded bitmap and if not match an error will be generated.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 blockdev.c                | 36 +++++++++++++++++++++
 include/sysemu/blockdev.h |  1 +
 include/sysemu/sysemu.h   |  1 +
 qemu-options.hx           | 35 +++++++++++++++++++++
 vl.c                      | 79 +++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 152 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 322ca03..3a3d71c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -185,6 +185,12 @@ QemuOpts *drive_def(const char *optstr)
     return qemu_opts_parse_noisily(qemu_find_opts("drive"), optstr, false);
 }
 
+QemuOpts *dirty_bitmap_def(const char *optstr)
+{
+    return qemu_opts_parse_noisily(qemu_find_opts("dirty-bitmap"), optstr,
+                                   false);
+}
+
 QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
                     const char *optstr)
 {
@@ -4084,6 +4090,36 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
     return head;
 }
 
+QemuOptsList qemu_dirty_bitmap_opts = {
+    .name = "dirty-bitmap",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_dirty_bitmap_opts.head),
+    .desc = {
+        {
+            .name = "name",
+            .type = QEMU_OPT_STRING,
+            .help = "Name of the dirty bitmap",
+        },{
+            .name = "node",
+            .type = QEMU_OPT_STRING,
+            .help = "node name to bind the bitmap to (and load it from it)",
+        },{
+            .name = "granularity",
+            .type = QEMU_OPT_NUMBER,
+            .help = "granularity",
+        },{
+            .name = "enabled",
+            .type = QEMU_OPT_BOOL,
+            .help = "enabled flag (default is 'on')",
+        },{
+            .name = "create",
+            .type = QEMU_OPT_BOOL,
+            .help = "create flag (default is 'off'), "
+                    "if on, new dirty bitmap will be created, "
+                    "else the existing one will be loaded"
+        }
+    }
+};
+
 QemuOptsList qemu_common_drive_opts = {
     .name = "drive",
     .head = QTAILQ_HEAD_INITIALIZER(qemu_common_drive_opts.head),
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 16432f3..105d11b 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -56,6 +56,7 @@ int drive_get_max_devs(BlockInterfaceType type);
 DriveInfo *drive_get_next(BlockInterfaceType type);
 
 QemuOpts *drive_def(const char *optstr);
+QemuOpts *dirty_bitmap_def(const char *optstr);
 QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,
                     const char *optstr);
 DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 3bb8897..7dc3980 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -248,6 +248,7 @@ bool usb_enabled(void);
 
 extern QemuOptsList qemu_legacy_drive_opts;
 extern QemuOptsList qemu_common_drive_opts;
+extern QemuOptsList qemu_dirty_bitmap_opts;
 extern QemuOptsList qemu_drive_opts;
 extern QemuOptsList qemu_chardev_opts;
 extern QemuOptsList qemu_device_opts;
diff --git a/qemu-options.hx b/qemu-options.hx
index 0cf7bb9..e750ebc 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -680,6 +680,41 @@ qemu-system-i386 -hda a -hdb b
 @end example
 ETEXI
 
+DEF("dirty-bitmap", HAS_ARG, QEMU_OPTION_dirty_bitmap,
+    "-dirty-bitmap name=name,node=@var{id}\n"
+    "              [,granularity=granularity][,enabled=on|off][,create=on|off]\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -dirty-bitmap @var{option}[,@var{option}[,@var{option}[,...]]]
+@findex -dirty-bitmap
+
+Define a dirty-bitmap. Valid options are:
+
+@table @option
+@item name=@var{name}
+The name of the bitmap. Should be unique per @var{file}/@var{node} and per
+@var{for_node}.
+@item node=@var{node}
+The node to load and bind the bitmap. It should be specified as @var{id} suboption
+of one of @option{-node} options.
+@item granularity=@var{granularity}
+Granularity (in bytes) for created dirty bitmap. If the bitmap is already
+exists in specified @var{file}/@var{file_id}/@var{device} it's granularity will
+not be changed but only checked (an error will be generated if this check
+fails).
+@item enabled=@var{enabled}
+Enabled flag for the bitmap. By default the bitmap will be enabled.
+@item create=@var{create}
+By default is off.
+If on, then new bitmap will be created in the image, if the bitmap with same
+name is already exists an error will be generated.
+If off, then the bitmap will be loaded from the image, if there is no one an
+error will be generated.
+If create=off and granularity is specified then granularity will be checked for
+loaded bitmap and if not match an error will be generated.
+@end table
+ETEXI
+
 DEF("mtdblock", HAS_ARG, QEMU_OPTION_mtdblock,
     "-mtdblock file  use 'file' as on-board Flash memory image\n",
     QEMU_ARCH_ALL)
diff --git a/vl.c b/vl.c
index 7a28982..31f5cdf 100644
--- a/vl.c
+++ b/vl.c
@@ -1155,6 +1155,74 @@ static int cleanup_add_fd(void *opaque, QemuOpts *opts, Error **errp)
 #define MTD_OPTS ""
 #define SD_OPTS ""
 
+static int dirty_bitmap_func(void *opaque, QemuOpts *opts, Error **errp_unused)
+{
+    Error *local_err = NULL;
+    BlockDriverState *bs = NULL;
+    BdrvDirtyBitmap *bitmap = NULL;
+
+    const char *name = qemu_opt_get(opts, "name");
+    const char *node = qemu_opt_get(opts, "node");
+
+    uint64_t granularity = qemu_opt_get_number(opts, "granularity", 0);
+    bool enabled = qemu_opt_get_bool(opts, "enabled", true);
+    bool create = qemu_opt_get_bool(opts, "create", false);
+
+    if (name == NULL) {
+        error_report("'name' option is necessary");
+        return -1;
+    }
+
+    if (node == NULL) {
+        error_report("'node' option is necessary");
+        return -1;
+    }
+
+    bs = bdrv_lookup_bs(node, node, &local_err);
+    if (bs == NULL) {
+        error_report_err(local_err);
+        return -1;
+    }
+
+    if (create) {
+        if (bdrv_load_check_dirty_bitmap(bs, name)) {
+            error_report("bitmap '%s' already exists", name);
+            return -1;
+        }
+
+        if (granularity == 0) {
+            granularity = bdrv_get_default_bitmap_granularity(bs);
+        }
+
+        bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, &local_err);
+        if (bitmap == NULL) {
+            error_report_err(local_err);
+            return -1;
+        }
+    } else {
+        bitmap = bdrv_load_dirty_bitmap(bs, name, &local_err);
+        if (bitmap == NULL) {
+            error_report_err(local_err);
+            return -1;
+        }
+
+        if (granularity != 0 &&
+            granularity != bdrv_dirty_bitmap_granularity(bitmap)) {
+            bdrv_release_dirty_bitmap(bs, bitmap);
+            error_report("granularity doesn't match");
+            return -1;
+        }
+    }
+
+    bdrv_dirty_bitmap_set_internal_persistance(bitmap, true);
+
+    if (!enabled) {
+        bdrv_disable_dirty_bitmap(bitmap);
+    }
+
+    return 0;
+}
+
 static int drive_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
     BlockInterfaceType *block_default_type = opaque;
@@ -2994,6 +3062,7 @@ int main(int argc, char **argv, char **envp)
     module_call_init(MODULE_INIT_QOM);
 
     qemu_add_opts(&qemu_drive_opts);
+    qemu_add_opts(&qemu_dirty_bitmap_opts);
     qemu_add_drive_opts(&qemu_legacy_drive_opts);
     qemu_add_drive_opts(&qemu_common_drive_opts);
     qemu_add_drive_opts(&qemu_drive_opts);
@@ -3126,6 +3195,11 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
+            case QEMU_OPTION_dirty_bitmap:
+                if (dirty_bitmap_def(optarg) == NULL) {
+                    exit(1);
+                }
+                break;
             case QEMU_OPTION_set:
                 if (qemu_set_option(optarg) != 0)
                     exit(1);
@@ -4456,6 +4530,11 @@ int main(int argc, char **argv, char **envp)
 
     parse_numa_opts(machine_class);
 
+    if (qemu_opts_foreach(qemu_find_opts("dirty-bitmap"),
+                          dirty_bitmap_func, NULL, NULL)) {
+        exit(1);
+    }
+
     if (qemu_opts_foreach(qemu_find_opts("mon"),
                           mon_init_func, NULL, NULL)) {
         exit(1);
-- 
1.8.3.1

[Qemu-devel] [PATCH 17/22] qcow2-dirty-bitmap: add IN_USE flag

[Vladimir Sementsov-Ogievskiy]

This flag is set on bitmap load and unset on store. If it is already
set when loading the bitmap, the bitmap should not be load (it is in
use by other drive or it is inconsistent (was not successfully saved))

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-dirty-bitmap.c | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index 384ccea..f210fee 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -42,7 +42,8 @@
 #define BME_MAX_NAME_SIZE 1023
 
 /* Bitmap directory entry flags */
-#define BME_RESERVED_FLAGS 0xffffffff
+#define BME_RESERVED_FLAGS 0xfffffffe
+#define BME_FLAG_IN_USE 1
 
 /* bits [1, 8] U [56, 63] are reserved */
 #define BME_TABLE_ENTRY_RESERVED_MASK 0xff000000000001fe
@@ -134,6 +135,29 @@ static QCow2BitmapHeader *bitmap_header(BDRVQcow2State *s,
            (s->bitmap_directory + bitmap->offset);
 }
 
+static int update_bitmap_header_sync(BlockDriverState *bs, QCow2Bitmap *bitmap)
+{
+    int ret;
+    BDRVQcow2State *s = bs->opaque;
+    QCow2BitmapHeader *h = bitmap_header(s, bitmap);
+
+    bitmap_header_to_be(h);
+    ret = bdrv_pwrite(bs->file->bs,
+                      s->bitmap_directory_offset + bitmap->offset,
+                      h, dir_entry_size(h));
+    bitmap_header_to_cpu(h);
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = bdrv_flush(bs);
+    if (ret < 0) {
+        return ret;
+    }
+
+    return 0;
+}
+
 static int directory_read(BlockDriverState *bs, Error **errp)
 {
     int ret;
@@ -293,6 +317,11 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs, QCow2Bitmap *bm,
 
     bmh = bitmap_header(s, bm);
 
+    if (bmh->flags & BME_FLAG_IN_USE) {
+        error_setg(errp, "Bitmap '%s' is in use", bm->name);
+        return NULL;
+    }
+
     bitmap_table = g_try_malloc(bmh->bitmap_table_size * sizeof(uint64_t));
     if (bitmap_table == NULL) {
         error_setg_errno(errp, -ENOMEM,
@@ -321,6 +350,13 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs, QCow2Bitmap *bm,
         goto fail;
     }
 
+    bmh->flags |= BME_FLAG_IN_USE;
+    ret = update_bitmap_header_sync(bs, bm);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not set in_use in bitmap header");
+        goto fail;
+    }
+
     g_free(bitmap_table);
     return bitmap;
 
@@ -769,6 +805,13 @@ void qcow2_bitmap_store(BlockDriverState *bs,
         goto finish;
     }
 
+    bmh->flags &= ~BME_FLAG_IN_USE;
+    ret = update_bitmap_header_sync(bs, bm);
+    if (ret < 0) {
+        error_setg_errno(errp, ret, "Can't update bitmap header.");
+        goto finish;
+    }
+
 finish:
     g_free(bitmap_table);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 18/22] qcow2-dirty-bitmaps: disallow stroing bitmap to other bs

[Vladimir Sementsov-Ogievskiy]

Check, that bitmap is stored to the owning bs.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/dirty-bitmap.c         | 12 ++++++++++++
 block/qcow2-dirty-bitmap.c   |  5 +++++
 include/block/dirty-bitmap.h |  2 ++
 3 files changed, 19 insertions(+)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 87ee4d7..9625f4a 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -485,3 +485,15 @@ BdrvDirtyBitmap *bdrv_load_dirty_bitmap(BlockDriverState *bs, const char *name,
     }
     return NULL;
 }
+
+bool bdrv_has_dirty_bitmap(BlockDriverState *bs, const BdrvDirtyBitmap *bitmap)
+{
+    BdrvDirtyBitmap *bm, *next;
+    QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
+        if (bm == bitmap) {
+            return true;
+        }
+    }
+
+    return false;
+}
diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index f210fee..70c6e36 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -757,6 +757,11 @@ void qcow2_bitmap_store(BlockDriverState *bs,
     uint64_t size = bdrv_dirty_bitmap_size(bitmap);
     int granularity = bdrv_dirty_bitmap_granularity(bitmap);
 
+    if (!bdrv_has_dirty_bitmap(bs, bitmap)) {
+        error_setg(errp, "Can't store bitmap to the other node.");
+        return;
+    }
+
     /* find/create dirty bitmap */
     bm = find_bitmap_by_name(bs, name);
     if (bm == NULL) {
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 66ba3f8..af76ac1 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -62,4 +62,6 @@ void bdrv_finalize_persistent_dirty_bitmaps(BlockDriverState *bs);
 BdrvDirtyBitmap *bdrv_load_dirty_bitmap(BlockDriverState *bs, const char *name,
                                         Error **errp);
 
+bool bdrv_has_dirty_bitmap(BlockDriverState *bs, const BdrvDirtyBitmap *bitmap);
+
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 19/22] iotests: add VM.test_launcn()

[Vladimir Sementsov-Ogievskiy]

Test vm can launch and print output in case of fail. This function is
needed for testing erroneous cases

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6807b07..187b434 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -226,6 +226,26 @@ class VM(object):
             os.remove(self._monitor_path)
             raise
 
+    def test_launch(self):
+        '''Launch the VM, an error is expected'''
+        try:
+            self.launch()
+        except:
+            self._popen.wait()
+            regex = re.compile(r"qemu-system-\w+")
+            print "Test launch failed: %d" % self._popen.returncode
+            print "--- qemu output ---"
+            for line in open(self._qemu_log_path):
+                #filter qtest comments
+                if not "] OPENED" in line:
+                    print regex.sub("qemu-system-*", line)
+            print "--- end qemu output ---"
+            return False
+
+        print "Tast launch successed!"
+        self.shutdown()
+        return True
+
     def shutdown(self):
         '''Terminate the VM and clean up'''
         if not self._popen is None:
-- 
1.8.3.1

[Qemu-devel] [PATCH 20/22] iotests: test internal persistent dirty bitmap

[Vladimir Sementsov-Ogievskiy]

Add simple test cases for testing persistent dirty bitmaps.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/160        | 112 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/160.out    |  21 ++++++++
 tests/qemu-iotests/group      |   1 +
 tests/qemu-iotests/iotests.py |   6 +++
 4 files changed, 140 insertions(+)
 create mode 100755 tests/qemu-iotests/160
 create mode 100644 tests/qemu-iotests/160.out

diff --git a/tests/qemu-iotests/160 b/tests/qemu-iotests/160
new file mode 100755
index 0000000..f9843da
--- /dev/null
+++ b/tests/qemu-iotests/160
@@ -0,0 +1,112 @@
+#!/usr/bin/env python
+#
+# Tests for persistent dirty bitmaps.
+#
+# Copyright: Vladimir Sementsov-Ogievskiy 2015
+#
+# 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 os
+import iotests
+import time
+from iotests import qemu_img
+
+disk = os.path.join(iotests.test_dir, 'disk')
+
+size   = 0x40000000 # 1G
+sector_size = 512
+granularity = 0x10000
+regions1 = [
+    { 'start': 0,          'count': 0x100000 },
+    { 'start': 0x200000,   'count': 0x100000 }
+    ]
+regions2 = [
+    { 'start': 0x10000000, 'count': 0x20000  },
+    { 'start': 0x39990000, 'count': 0x10000  }
+    ]
+
+class TestPersistentDirtyBitmap(iotests.QMPTestCase):
+
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, disk, str(size))
+
+    def mkVm(self, create_bitmap):
+        vm = iotests.VM().add_drive(disk)
+        vm.add_dirty_bitmap('12300000-0000-0000-1230-000001230000', 'drive0', create_bitmap)
+        return vm
+
+    def tearDown(self):
+        os.remove(disk)
+
+    def getMd5(self):
+        result = self.vm.qmp('query-block');
+        return result['return'][0]['dirty-bitmaps'][0]['md5']
+
+    def checkBitmap(self, md5):
+        result = self.vm.qmp('query-block');
+        self.assert_qmp(result, 'return[0]/dirty-bitmaps[0]/md5', md5);
+
+    def writeRegions(self, regions):
+        for r in regions:
+          self.vm.hmp_qemu_io('drive0',
+                                'write %d %d' % (r['start'], r['count']))
+
+    def test_persistent(self):
+        self.vm = self.mkVm(True)
+        self.vm.launch()
+
+        self.writeRegions(regions1)
+        md5 = self.getMd5()
+
+        self.vm.shutdown()
+        self.vm = self.mkVm(False)
+        self.vm.launch()
+
+        self.checkBitmap(md5)
+        self.writeRegions(regions2)
+        md5 = self.getMd5()
+
+        self.vm.shutdown()
+        self.vm.launch()
+
+        self.checkBitmap(md5)
+
+        self.vm.shutdown()
+
+    def test_not_exist(self):
+        vm = self.mkVm(False)
+        vm.test_launch()
+
+    def test_already_exists(self):
+        vm = self.mkVm(True)
+        vm.test_launch()
+        vm.test_launch()
+
+    def test_in_use(self):
+        vm = self.mkVm(True)
+        vm.launch()
+        vm.shutdown()
+
+        vm1 = self.mkVm(False)
+        vm1.launch()
+
+        vm2 = self.mkVm(False)
+        vm2.test_launch()
+
+        vm1.shutdown()
+
+
+if __name__ == '__main__':
+    iotests.main()
diff --git a/tests/qemu-iotests/160.out b/tests/qemu-iotests/160.out
new file mode 100644
index 0000000..653d21b
--- /dev/null
+++ b/tests/qemu-iotests/160.out
@@ -0,0 +1,21 @@
+....
+----------------------------------------------------------------------
+Ran 4 tests
+
+OK
+Tast launch successed!
+Test launch failed: 1
+--- qemu output ---
+qemu-system-*: -dirty-bitmap name=12300000-0000-0000-1230-000001230000,node=drive0,create=on: bitmap '12300000-0000-0000-1230-000001230000' already exists
+
+--- end qemu output ---
+Test launch failed: 1
+--- qemu output ---
+qemu-system-*: -dirty-bitmap name=12300000-0000-0000-1230-000001230000,node=drive0,create=off: Bitmap '12300000-0000-0000-1230-000001230000' is in use
+
+--- end qemu output ---
+Test launch failed: 1
+--- qemu output ---
+qemu-system-*: -dirty-bitmap name=12300000-0000-0000-1230-000001230000,node=drive0,create=off: Could not find bitmap '12300000-0000-0000-1230-000001230000' in the node 'drive0'
+
+--- end qemu output ---
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index faf0f21..641106a 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -150,3 +150,4 @@
 145 auto quick
 146 auto quick
 148 rw auto quick
+160 rw auto quick
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 187b434..5b4492c 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -166,6 +166,12 @@ class VM(object):
         self._num_drives += 1
         return self
 
+    def add_dirty_bitmap(self, name, node, create=False):
+        '''Add dirty bitmap parameter to VM cmd'''
+        self._args.append('-dirty-bitmap')
+        self._args.append('name=%s,node=%s,create=%s' % (name, node, 'on' if create else 'off'))
+        return self
+
     def pause_drive(self, drive, event=None):
         '''Pause drive r/w operations'''
         if not event:
-- 
1.8.3.1

[Qemu-devel] [PATCH 21/22] qcow2-dirty-bitmap: add AUTO flag

[Vladimir Sementsov-Ogievskiy]

The bitmap should be auto-loaded if auto flag is set.
For now, actually, there are no methods to set it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-dirty-bitmap.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index 70c6e36..159e935 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -42,8 +42,9 @@
 #define BME_MAX_NAME_SIZE 1023
 
 /* Bitmap directory entry flags */
-#define BME_RESERVED_FLAGS 0xfffffffe
+#define BME_RESERVED_FLAGS 0xfffffffc
 #define BME_FLAG_IN_USE 1
+#define BME_FLAG_AUTO   (1U << 1)
 
 /* bits [1, 8] U [56, 63] are reserved */
 #define BME_TABLE_ENTRY_RESERVED_MASK 0xff000000000001fe
@@ -52,6 +53,9 @@ typedef enum BitmapType {
     BT_DIRTY_TRACKING_BITMAP = 1
 } BitmapType;
 
+static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs, QCow2Bitmap *bm,
+                                    Error **errp);
+
 void qcow2_free_bitmaps(BlockDriverState *bs)
 {
     BDRVQcow2State *s = bs->opaque;
@@ -215,6 +219,13 @@ static int directory_read(BlockDriverState *bs, Error **errp)
         bm->offset = offset;
         bm->name = g_strndup((char *)(h + 1), h->name_size);
 
+        if (h->flags & BME_FLAG_AUTO) {
+            load_bitmap(bs, bm, errp);
+            if (*errp != NULL) {
+                goto fail;
+            }
+        }
+
         offset += dir_entry_size(h);
     }
     return 0;
-- 
1.8.3.1

[Qemu-devel] [PATCH 22/22] qcow2-dirty-bitmap: add EXTRA_DATA_COMPATIBLE flag

[Vladimir Sementsov-Ogievskiy]

If this flag is unset and exta data present the bitmap should be
read-only. For now just return error for this case.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2-dirty-bitmap.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
index 159e935..95c166c 100644
--- a/block/qcow2-dirty-bitmap.c
+++ b/block/qcow2-dirty-bitmap.c
@@ -45,6 +45,7 @@
 #define BME_RESERVED_FLAGS 0xfffffffc
 #define BME_FLAG_IN_USE 1
 #define BME_FLAG_AUTO   (1U << 1)
+#define BME_FLAG_EXTRA_DATA_COMPATIBLE   (1U << 1)
 
 /* bits [1, 8] U [56, 63] are reserved */
 #define BME_TABLE_ENTRY_RESERVED_MASK 0xff000000000001fe
@@ -333,6 +334,13 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs, QCow2Bitmap *bm,
         return NULL;
     }
 
+    if (!(bmh->flags & BME_FLAG_EXTRA_DATA_COMPATIBLE) &&
+            bmh->extra_data_size != 0) {
+        error_setg(errp, "Uncompatible extra data found for bitmap '%s'",
+                   bm->name);
+        return NULL;
+    }
+
     bitmap_table = g_try_malloc(bmh->bitmap_table_size * sizeof(uint64_t));
     if (bitmap_table == NULL) {
         error_setg_errno(errp, -ENOMEM,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 01/22] block: Add two dirty bitmap getters

[Vladimir Sementsov-Ogievskiy]

On 15.03.2016 23:04, Vladimir Sementsov-Ogievskiy wrote:
> From: Fam Zheng <famz@redhat.com>
>
> For dirty bitmap users to get the size and the name of a
> BdrvDirtyBitmap.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Reviewed-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

it's an accidental s.o.b., actually there are no changes by me.

> ---
>   block/dirty-bitmap.c         | 10 ++++++++++
>   include/block/dirty-bitmap.h |  2 ++
>   2 files changed, 12 insertions(+)
>
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 556e1d1..45cfa3b 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -97,6 +97,16 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>       return bitmap;
>   }
>   
> +int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
> +{
> +    return bitmap->size;
> +}
> +
> +const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
> +{
> +    return bitmap->name;
> +}
> +
>   bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
>   {
>       return bitmap->successor;
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 80afe60..4dc8750 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -29,6 +29,8 @@ uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
>   uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
>   bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
>   bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
> +const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
> +int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap);
>   DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
>   int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
>                      int64_t sector);


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 06/22] hbitmap: load/store

[John Snow]

On 03/15/2016 04:04 PM, Vladimir Sementsov-Ogievskiy wrote:
> Add functions for load/store HBitmap to BDS, using clusters table:
> Last level of the bitmap is splitted into chunks of 'cluster_size'
> size. Each cell of the table contains offset in bds, to load/store
> corresponding chunk.
> 
> Also,
>     0 in cell means all-zeroes-chunk (should not be saved)
>     1 in cell means all-ones-chunk (should not be saved)
>     hbitmap_prepare_store() fills table with
>       0 for all-zeroes chunks
>       1 for all-ones chunks
>       2 for others
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/dirty-bitmap.c         |  23 +++++
>  include/block/dirty-bitmap.h |  11 +++
>  include/qemu/hbitmap.h       |  12 +++
>  util/hbitmap.c               | 209 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 255 insertions(+)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index e68c177..816c6ee 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -396,3 +396,26 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
>  {
>      return hbitmap_count(bitmap->bitmap);
>  }
> +
> +int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
> +                           const uint64_t *table, uint32_t table_size,
> +                           uint32_t cluster_size)
> +{
> +    return hbitmap_load(bitmap->bitmap, bs, table, table_size, cluster_size);
> +}
> +
> +int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
> +                                    uint32_t cluster_size,
> +                                    uint64_t *table,
> +                                    uint32_t *table_size)
> +{
> +    return hbitmap_prepare_store(bitmap->bitmap, cluster_size,
> +                                 table, table_size);
> +}
> +
> +int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
> +                            const uint64_t *table, uint32_t table_size,
> +                            uint32_t cluster_size)
> +{
> +    return hbitmap_store(bitmap->bitmap, bs, table, table_size, cluster_size);
> +}
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 27515af..20cb540 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -43,4 +43,15 @@ void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>  void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
>  
> +int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
> +                           const uint64_t *table, uint32_t table_size,
> +                           uint32_t cluster_size);
> +int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
> +                                    uint32_t cluster_size,
> +                                    uint64_t *table,
> +                                    uint32_t *table_size);
> +int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
> +                            const uint64_t *table, uint32_t table_size,
> +                            uint32_t cluster_size);
> +
>  #endif
> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
> index 6d1da4d..d83bb79 100644
> --- a/include/qemu/hbitmap.h
> +++ b/include/qemu/hbitmap.h
> @@ -241,5 +241,17 @@ static inline size_t hbitmap_iter_next_word(HBitmapIter *hbi, unsigned long *p_c
>      return hbi->pos;
>  }
>  
> +typedef struct BlockDriverState BlockDriverState;
> +
> +int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
> +                 const uint64_t *table, uint32_t table_size,
> +                 uint32_t cluster_size);
> +
> +int hbitmap_prepare_store(const HBitmap *bitmap, uint32_t cluster_size,
> +                          uint64_t *table, uint32_t *table_size);
> +
> +int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
> +                  const uint64_t *table, uint32_t table_size,
> +                  uint32_t cluster_size);
>  
>  #endif
> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index 28595fb..1960e4f 100644
> --- a/util/hbitmap.c
> +++ b/util/hbitmap.c
> @@ -15,6 +15,8 @@
>  #include "qemu/host-utils.h"
>  #include "trace.h"
>  
> +#include "block/block.h"
> +

This is a bit of a red flag -- we shouldn't need block layer specifics
in the subcomponent-agnostic HBitmap utility.

Further, by relying on these facilities here in hbitmap.c, "make check"
no longer can compile the relevant hbitmap tests.

Make sure that each intermediate commit here passes these necessary
tests, test-hbitmap in particular for each, and a "make check" overall
at the end of your series.

--js

>  /* HBitmaps provides an array of bits.  The bits are stored as usual in an
>   * array of unsigned longs, but HBitmap is also optimized to provide fast
>   * iteration over set bits; going from one bit to the next is O(logB n)
> @@ -499,3 +501,210 @@ char *hbitmap_md5(const HBitmap *bitmap)
>      const guchar *data = (const guchar *)bitmap->levels[HBITMAP_LEVELS - 1];
>      return g_compute_checksum_for_data(G_CHECKSUM_MD5, data, size);
>  }
> +
> +/* hb_restore_levels()
> + * Using the last level restore all other levels
> + */
> +static void hb_restore_levels(HBitmap *bitmap)
> +{
> +    int64_t i, size, prev_size;
> +    int lev;
> +
> +    /* restore levels starting from penultimate to zero level, assuming
> +     * that the last level is ok */
> +    size = MAX((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
> +    for (lev = HBITMAP_LEVELS - 1; lev-- > 0; ) {
> +        prev_size = size;
> +        size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
> +        memset(bitmap->levels[lev], 0, size * sizeof(unsigned long));
> +
> +        for (i = 0; i < prev_size; ++i) {
> +            if (bitmap->levels[lev + 1][i]) {
> +                bitmap->levels[lev][i >> BITS_PER_LEVEL] |=
> +                    1UL << (i & (BITS_PER_LONG - 1));
> +            }
> +        }
> +    }
> +
> +    bitmap->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
> +}
> +
> +/* load_bitmap()
> + * Load dirty bitmap from file, using table with cluster offsets.
> + * Table entries are assumed to be in little endian format.
> + */
> +int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
> +                 const uint64_t *table, uint32_t table_size,
> +                 uint32_t cluster_size)
> +{
> +    uint32_t i;
> +    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
> +    uint8_t *end = cur + ((bitmap->size + 7) >> 3);
> +
> +    hbitmap_reset_all(bitmap);
> +
> +    for (i = 0; i < table_size && cur < end; ++i) {
> +        uint64_t offset = table[i];
> +        uint64_t count = MIN(cluster_size, end - cur);
> +
> +        /* Zero offset means zero region, offset = 1 means filled region.
> +         * Cluster is not allocated in both cases. */
> +        if (offset == 1) {
> +            memset(cur, 0xff, count);
> +        } else if (offset) {
> +            int ret = bdrv_pread(bs, offset, cur, count);
> +            if (ret < 0) {
> +                return ret;
> +            }
> +        }
> +
> +        cur += cluster_size;
> +    }
> +
> +    cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
> +    while (cur < end) {
> +        if (BITS_PER_LONG == 32) {
> +            le32_to_cpus((uint32_t *)cur);
> +        } else {
> +            le64_to_cpus((uint64_t *)cur);
> +        }
> +
> +        cur += sizeof(unsigned long);
> +    }
> +
> +    hb_restore_levels(bitmap);
> +
> +    return 0;
> +}
> +
> +static bool buffer_is_all_ones(void *buf, size_t len)
> +{
> +    /* FIXME */
> +    return false;
> +}
> +
> +/* hbitmap_prepare_store()
> + * Devide bitmap data into clusters, and then,
> + * for zero cluster: table[i] = 0
> + * for all-ones cluster: table[i] = 1
> + * for other clusters: table[i] = 2
> + */
> +int hbitmap_prepare_store(const HBitmap *bitmap,
> +                          uint32_t cluster_size,
> +                          uint64_t *table,
> +                          uint32_t *table_size)
> +{
> +    HBitmapIter hbi;
> +    hbitmap_iter_init(&hbi, bitmap, 0);
> +    uint64_t nb_bits_in_cl = (uint64_t)cluster_size << 3;
> +    uint32_t need_table_size =
> +            (bitmap->size + nb_bits_in_cl - 1) / nb_bits_in_cl;
> +
> +    if (table == NULL && *table_size == 0) {
> +        *table_size = need_table_size;
> +        return 0;
> +    }
> +
> +    if (*table_size < need_table_size) {
> +        return -ENOMEM;
> +    }
> +
> +    memset(table, 0, *table_size * sizeof(table[0]));
> +
> +    for (;;) {
> +        unsigned long cur;
> +        size_t pos = hbitmap_iter_next_word(&hbi, &cur);
> +        size_t byte = pos * sizeof(unsigned long);
> +        uint64_t bit = byte << 3;
> +        uint64_t nbits = MIN(cluster_size << 3, bitmap->size - bit), next_bit;
> +        size_t i = byte / cluster_size;
> +
> +        if (pos == -1) {
> +            break;
> +        }
> +
> +        if (pos % cluster_size != 0) {
> +            table[i] = 2;
> +        } else if (buffer_is_all_ones(&bitmap->levels[HBITMAP_LEVELS - 1][pos],
> +                               nbits >> 3)) {
> +            table[i] = 1;
> +            if (nbits & 7) {
> +                uint8_t last_byte =
> +                        *(((uint8_t *)&bitmap->levels[HBITMAP_LEVELS - 1][pos])
> +                                + (nbits >> 3));
> +                if (last_byte != ((1 << (nbits & 7)) - 1)) {
> +                    table[i] = 2;
> +                }
> +            }
> +        } else {
> +            table[i] = 2;
> +        }
> +
> +        next_bit = (i + 1) * cluster_size << 3;
> +
> +        if (next_bit >= bitmap->size) {
> +            break;
> +        }
> +
> +        hbitmap_iter_init(&hbi, bitmap, next_bit << bitmap->granularity);
> +    }
> +
> +    return 0;
> +}
> +
> +static void longs_to_le(unsigned long *longs, size_t count)
> +{
> +    unsigned long *end = longs + count;
> +    while (longs < end) {
> +        if (BITS_PER_LONG == 32) {
> +            cpu_to_le32s((uint32_t *)longs);
> +        } else {
> +            cpu_to_le64s((uint64_t *)longs);
> +        }
> +
> +        longs++;
> +    }
> +}
> +
> +/* store_bitmap()
> + * update bitmap table by storing bitmap to it.
> + * bitmap table entries are assumed to be in big endian format
> + * On the error, the resulting bitmap table is valid for clearing, but
> + * may contain invalid bitmap */
> +int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
> +                  const uint64_t *table, uint32_t table_size,
> +                  uint32_t cluster_size)
> +{
> +    int i;
> +    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
> +    uint8_t *end = cur +
> +        ((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL) * (sizeof(long));
> +
> +    for (i = 0; i < table_size && cur < end; ++i) {
> +        uint64_t offset = table[i];
> +        uint64_t count = MIN(cluster_size, end - cur);
> +
> +        /* Zero offset means zero region, offset = 1 means filled region.
> +         * Cluster is not allocated in both cases. */
> +        if (offset > 1) {
> +            int ret;
> +            if (cpu_to_le16(1) == 1) {
> +                ret = bdrv_pwrite(bs, offset, cur, count);
> +            } else {
> +                void *tmp = g_memdup(cur, count);
> +                longs_to_le((unsigned long *)cur,
> +                            count / sizeof(unsigned long));
> +                ret = bdrv_pwrite(bs, offset, tmp, count);
> +                g_free(tmp);
> +            }
> +
> +            if (ret < 0) {
> +                return ret;
> +            }
> +        }
> +
> +        cur += cluster_size;
> +    }
> +
> +    return 0;
> +}
> 

Re: [Qemu-devel] [PATCH 06/22] hbitmap: load/store

[Vladimir Sementsov-Ogievskiy]

On 22.03.2016 01:42, John Snow wrote:
>
> On 03/15/2016 04:04 PM, Vladimir Sementsov-Ogievskiy wrote:
>> Add functions for load/store HBitmap to BDS, using clusters table:
>> Last level of the bitmap is splitted into chunks of 'cluster_size'
>> size. Each cell of the table contains offset in bds, to load/store
>> corresponding chunk.
>>
>> Also,
>>      0 in cell means all-zeroes-chunk (should not be saved)
>>      1 in cell means all-ones-chunk (should not be saved)
>>      hbitmap_prepare_store() fills table with
>>        0 for all-zeroes chunks
>>        1 for all-ones chunks
>>        2 for others
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/dirty-bitmap.c         |  23 +++++
>>   include/block/dirty-bitmap.h |  11 +++
>>   include/qemu/hbitmap.h       |  12 +++
>>   util/hbitmap.c               | 209 +++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 255 insertions(+)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index e68c177..816c6ee 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -396,3 +396,26 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
>>   {
>>       return hbitmap_count(bitmap->bitmap);
>>   }
>> +
>> +int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
>> +                           const uint64_t *table, uint32_t table_size,
>> +                           uint32_t cluster_size)
>> +{
>> +    return hbitmap_load(bitmap->bitmap, bs, table, table_size, cluster_size);
>> +}
>> +
>> +int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
>> +                                    uint32_t cluster_size,
>> +                                    uint64_t *table,
>> +                                    uint32_t *table_size)
>> +{
>> +    return hbitmap_prepare_store(bitmap->bitmap, cluster_size,
>> +                                 table, table_size);
>> +}
>> +
>> +int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
>> +                            const uint64_t *table, uint32_t table_size,
>> +                            uint32_t cluster_size)
>> +{
>> +    return hbitmap_store(bitmap->bitmap, bs, table, table_size, cluster_size);
>> +}
>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>> index 27515af..20cb540 100644
>> --- a/include/block/dirty-bitmap.h
>> +++ b/include/block/dirty-bitmap.h
>> @@ -43,4 +43,15 @@ void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
>>   int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>>   void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
>>   
>> +int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
>> +                           const uint64_t *table, uint32_t table_size,
>> +                           uint32_t cluster_size);
>> +int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
>> +                                    uint32_t cluster_size,
>> +                                    uint64_t *table,
>> +                                    uint32_t *table_size);
>> +int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
>> +                            const uint64_t *table, uint32_t table_size,
>> +                            uint32_t cluster_size);
>> +
>>   #endif
>> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
>> index 6d1da4d..d83bb79 100644
>> --- a/include/qemu/hbitmap.h
>> +++ b/include/qemu/hbitmap.h
>> @@ -241,5 +241,17 @@ static inline size_t hbitmap_iter_next_word(HBitmapIter *hbi, unsigned long *p_c
>>       return hbi->pos;
>>   }
>>   
>> +typedef struct BlockDriverState BlockDriverState;
>> +
>> +int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
>> +                 const uint64_t *table, uint32_t table_size,
>> +                 uint32_t cluster_size);
>> +
>> +int hbitmap_prepare_store(const HBitmap *bitmap, uint32_t cluster_size,
>> +                          uint64_t *table, uint32_t *table_size);
>> +
>> +int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
>> +                  const uint64_t *table, uint32_t table_size,
>> +                  uint32_t cluster_size);
>>   
>>   #endif
>> diff --git a/util/hbitmap.c b/util/hbitmap.c
>> index 28595fb..1960e4f 100644
>> --- a/util/hbitmap.c
>> +++ b/util/hbitmap.c
>> @@ -15,6 +15,8 @@
>>   #include "qemu/host-utils.h"
>>   #include "trace.h"
>>   
>> +#include "block/block.h"
>> +
> This is a bit of a red flag -- we shouldn't need block layer specifics
> in the subcomponent-agnostic HBitmap utility.
>
> Further, by relying on these facilities here in hbitmap.c, "make check"
> no longer can compile the relevant hbitmap tests.

Thanks. Locally I've already fixed it (add entity into tests Makefile). 
But in fact, it may be better to move these functions into 
block/dirty_bitmap.c or to new file like block/hbitmap.c.

>
> Make sure that each intermediate commit here passes these necessary
> tests, test-hbitmap in particular for each, and a "make check" overall
> at the end of your series.
>
> --js
>
>>   /* HBitmaps provides an array of bits.  The bits are stored as usual in an
>>    * array of unsigned longs, but HBitmap is also optimized to provide fast
>>    * iteration over set bits; going from one bit to the next is O(logB n)
>> @@ -499,3 +501,210 @@ char *hbitmap_md5(const HBitmap *bitmap)
>>       const guchar *data = (const guchar *)bitmap->levels[HBITMAP_LEVELS - 1];
>>       return g_compute_checksum_for_data(G_CHECKSUM_MD5, data, size);
>>   }
>> +
>> +/* hb_restore_levels()
>> + * Using the last level restore all other levels
>> + */
>> +static void hb_restore_levels(HBitmap *bitmap)
>> +{
>> +    int64_t i, size, prev_size;
>> +    int lev;
>> +
>> +    /* restore levels starting from penultimate to zero level, assuming
>> +     * that the last level is ok */
>> +    size = MAX((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
>> +    for (lev = HBITMAP_LEVELS - 1; lev-- > 0; ) {
>> +        prev_size = size;
>> +        size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
>> +        memset(bitmap->levels[lev], 0, size * sizeof(unsigned long));
>> +
>> +        for (i = 0; i < prev_size; ++i) {
>> +            if (bitmap->levels[lev + 1][i]) {
>> +                bitmap->levels[lev][i >> BITS_PER_LEVEL] |=
>> +                    1UL << (i & (BITS_PER_LONG - 1));
>> +            }
>> +        }
>> +    }
>> +
>> +    bitmap->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
>> +}
>> +
>> +/* load_bitmap()
>> + * Load dirty bitmap from file, using table with cluster offsets.
>> + * Table entries are assumed to be in little endian format.
>> + */
>> +int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
>> +                 const uint64_t *table, uint32_t table_size,
>> +                 uint32_t cluster_size)
>> +{
>> +    uint32_t i;
>> +    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
>> +    uint8_t *end = cur + ((bitmap->size + 7) >> 3);
>> +
>> +    hbitmap_reset_all(bitmap);
>> +
>> +    for (i = 0; i < table_size && cur < end; ++i) {
>> +        uint64_t offset = table[i];
>> +        uint64_t count = MIN(cluster_size, end - cur);
>> +
>> +        /* Zero offset means zero region, offset = 1 means filled region.
>> +         * Cluster is not allocated in both cases. */
>> +        if (offset == 1) {
>> +            memset(cur, 0xff, count);
>> +        } else if (offset) {
>> +            int ret = bdrv_pread(bs, offset, cur, count);
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
>> +        }
>> +
>> +        cur += cluster_size;
>> +    }
>> +
>> +    cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
>> +    while (cur < end) {
>> +        if (BITS_PER_LONG == 32) {
>> +            le32_to_cpus((uint32_t *)cur);
>> +        } else {
>> +            le64_to_cpus((uint64_t *)cur);
>> +        }
>> +
>> +        cur += sizeof(unsigned long);
>> +    }
>> +
>> +    hb_restore_levels(bitmap);
>> +
>> +    return 0;
>> +}
>> +
>> +static bool buffer_is_all_ones(void *buf, size_t len)
>> +{
>> +    /* FIXME */
>> +    return false;
>> +}
>> +
>> +/* hbitmap_prepare_store()
>> + * Devide bitmap data into clusters, and then,
>> + * for zero cluster: table[i] = 0
>> + * for all-ones cluster: table[i] = 1
>> + * for other clusters: table[i] = 2
>> + */
>> +int hbitmap_prepare_store(const HBitmap *bitmap,
>> +                          uint32_t cluster_size,
>> +                          uint64_t *table,
>> +                          uint32_t *table_size)
>> +{
>> +    HBitmapIter hbi;
>> +    hbitmap_iter_init(&hbi, bitmap, 0);
>> +    uint64_t nb_bits_in_cl = (uint64_t)cluster_size << 3;
>> +    uint32_t need_table_size =
>> +            (bitmap->size + nb_bits_in_cl - 1) / nb_bits_in_cl;
>> +
>> +    if (table == NULL && *table_size == 0) {
>> +        *table_size = need_table_size;
>> +        return 0;
>> +    }
>> +
>> +    if (*table_size < need_table_size) {
>> +        return -ENOMEM;
>> +    }
>> +
>> +    memset(table, 0, *table_size * sizeof(table[0]));
>> +
>> +    for (;;) {
>> +        unsigned long cur;
>> +        size_t pos = hbitmap_iter_next_word(&hbi, &cur);
>> +        size_t byte = pos * sizeof(unsigned long);
>> +        uint64_t bit = byte << 3;
>> +        uint64_t nbits = MIN(cluster_size << 3, bitmap->size - bit), next_bit;
>> +        size_t i = byte / cluster_size;
>> +
>> +        if (pos == -1) {
>> +            break;
>> +        }
>> +
>> +        if (pos % cluster_size != 0) {
>> +            table[i] = 2;
>> +        } else if (buffer_is_all_ones(&bitmap->levels[HBITMAP_LEVELS - 1][pos],
>> +                               nbits >> 3)) {
>> +            table[i] = 1;
>> +            if (nbits & 7) {
>> +                uint8_t last_byte =
>> +                        *(((uint8_t *)&bitmap->levels[HBITMAP_LEVELS - 1][pos])
>> +                                + (nbits >> 3));
>> +                if (last_byte != ((1 << (nbits & 7)) - 1)) {
>> +                    table[i] = 2;
>> +                }
>> +            }
>> +        } else {
>> +            table[i] = 2;
>> +        }
>> +
>> +        next_bit = (i + 1) * cluster_size << 3;
>> +
>> +        if (next_bit >= bitmap->size) {
>> +            break;
>> +        }
>> +
>> +        hbitmap_iter_init(&hbi, bitmap, next_bit << bitmap->granularity);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void longs_to_le(unsigned long *longs, size_t count)
>> +{
>> +    unsigned long *end = longs + count;
>> +    while (longs < end) {
>> +        if (BITS_PER_LONG == 32) {
>> +            cpu_to_le32s((uint32_t *)longs);
>> +        } else {
>> +            cpu_to_le64s((uint64_t *)longs);
>> +        }
>> +
>> +        longs++;
>> +    }
>> +}
>> +
>> +/* store_bitmap()
>> + * update bitmap table by storing bitmap to it.
>> + * bitmap table entries are assumed to be in big endian format
>> + * On the error, the resulting bitmap table is valid for clearing, but
>> + * may contain invalid bitmap */
>> +int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
>> +                  const uint64_t *table, uint32_t table_size,
>> +                  uint32_t cluster_size)
>> +{
>> +    int i;
>> +    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
>> +    uint8_t *end = cur +
>> +        ((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL) * (sizeof(long));
>> +
>> +    for (i = 0; i < table_size && cur < end; ++i) {
>> +        uint64_t offset = table[i];
>> +        uint64_t count = MIN(cluster_size, end - cur);
>> +
>> +        /* Zero offset means zero region, offset = 1 means filled region.
>> +         * Cluster is not allocated in both cases. */
>> +        if (offset > 1) {
>> +            int ret;
>> +            if (cpu_to_le16(1) == 1) {
>> +                ret = bdrv_pwrite(bs, offset, cur, count);
>> +            } else {
>> +                void *tmp = g_memdup(cur, count);
>> +                longs_to_le((unsigned long *)cur,
>> +                            count / sizeof(unsigned long));
>> +                ret = bdrv_pwrite(bs, offset, tmp, count);
>> +                g_free(tmp);
>> +            }
>> +
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
>> +        }
>> +
>> +        cur += cluster_size;
>> +    }
>> +
>> +    return 0;
>> +}
>>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 01/22] block: Add two dirty bitmap getters

[Eric Blake]

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

On 03/15/2016 02:08 PM, Vladimir Sementsov-Ogievskiy wrote:
> On 15.03.2016 23:04, Vladimir Sementsov-Ogievskiy wrote:
>> From: Fam Zheng <famz@redhat.com>
>>
>> For dirty bitmap users to get the size and the name of a
>> BdrvDirtyBitmap.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> Reviewed-by: John Snow <jsnow@redhat.com>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> it's an accidental s.o.b., actually there are no changes by me.

But if you are emailing the current version of the series, you should at
least have your name associated with the commit message - if not s.o.b,
then Reviewed-by.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH 01/22] block: Add two dirty bitmap getters

[John Snow]

On 03/15/2016 04:08 PM, Vladimir Sementsov-Ogievskiy wrote:
> On 15.03.2016 23:04, Vladimir Sementsov-Ogievskiy wrote:
>> From: Fam Zheng <famz@redhat.com>
>>
>> For dirty bitmap users to get the size and the name of a
>> BdrvDirtyBitmap.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> Reviewed-by: John Snow <jsnow@redhat.com>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> it's an accidental s.o.b., actually there are no changes by me.
> 

You can add an S-o-B even when you don't make changes to indicate a
"chain of custody" for the patch. You are certifying that the patch is,
to the best of your knowledge, properly credited and licensed.

It's not required for you to add your /own/ S-o-B to every patch you
send, but it's not hurting anything here either.

At least, that was my understanding of it. I know as a maintainer I add
my S-o-B to everything that goes through my tree even if I don't modify
it just to signify where the patch has been and who has handled it.

--js

>> ---
>>   block/dirty-bitmap.c         | 10 ++++++++++
>>   include/block/dirty-bitmap.h |  2 ++
>>   2 files changed, 12 insertions(+)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 556e1d1..45cfa3b 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -97,6 +97,16 @@ BdrvDirtyBitmap
>> *bdrv_create_dirty_bitmap(BlockDriverState *bs,
>>       return bitmap;
>>   }
>>   +int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap)
>> +{
>> +    return bitmap->size;
>> +}
>> +
>> +const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
>> +{
>> +    return bitmap->name;
>> +}
>> +
>>   bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
>>   {
>>       return bitmap->successor;
>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>> index 80afe60..4dc8750 100644
>> --- a/include/block/dirty-bitmap.h
>> +++ b/include/block/dirty-bitmap.h
>> @@ -29,6 +29,8 @@ uint32_t
>> bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
>>   uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
>>   bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
>>   bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
>> +const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
>> +int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap);
>>   DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
>>   int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap,
>>                      int64_t sector);
> 
> 

Re: [Qemu-devel] [PATCH 04/22] iotests: add default node-name

[John Snow]

On 03/15/2016 04:04 PM, Vladimir Sementsov-Ogievskiy wrote:
> When testing migration, auto-generated by qemu node-names differs in
> source and destination qemu and migration fails. After this patch,
> auto-generated by iotest nodenames will be the same.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  tests/qemu-iotests/iotests.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 2445cf2..6807b07 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -156,6 +156,7 @@ class VM(object):
>              options.append('file=%s' % path)
>              options.append('format=%s' % imgfmt)
>              options.append('cache=%s' % cachemode)
> +            options.append('node-name=drivenode%d' % self._num_drives)
>  
>          if opts:
>              options.append(opts)
> 

Tested-by: John Snow <jsnow@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH 19/22] iotests: add VM.test_launcn()

[Eric Blake]

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

On 03/15/2016 02:04 PM, Vladimir Sementsov-Ogievskiy wrote:

in subject: s/launcn/launch/

> Test vm can launch and print output in case of fail. This function is
> needed for testing erroneous cases
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  tests/qemu-iotests/iotests.py | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 

> +    def test_launch(self):
> +        '''Launch the VM, an error is expected'''
> +        try:
> +            self.launch()
> +        except:
> +            self._popen.wait()
> +            regex = re.compile(r"qemu-system-\w+")
> +            print "Test launch failed: %d" % self._popen.returncode
> +            print "--- qemu output ---"
> +            for line in open(self._qemu_log_path):
> +                #filter qtest comments
> +                if not "] OPENED" in line:
> +                    print regex.sub("qemu-system-*", line)
> +            print "--- end qemu output ---"
> +            return False
> +
> +        print "Tast launch successed!"

What? Did you mean "Test launch succeeded" ?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH 20/22] iotests: test internal persistent dirty bitmap

[Eric Blake]

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

On 03/15/2016 02:04 PM, Vladimir Sementsov-Ogievskiy wrote:
> Add simple test cases for testing persistent dirty bitmaps.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  tests/qemu-iotests/160        | 112 ++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/160.out    |  21 ++++++++
>  tests/qemu-iotests/group      |   1 +
>  tests/qemu-iotests/iotests.py |   6 +++
>  4 files changed, 140 insertions(+)
>  create mode 100755 tests/qemu-iotests/160
>  create mode 100644 tests/qemu-iotests/160.out
> 
> diff --git a/tests/qemu-iotests/160 b/tests/qemu-iotests/160
> new file mode 100755
> index 0000000..f9843da
> --- /dev/null
> +++ b/tests/qemu-iotests/160
> @@ -0,0 +1,112 @@
> +#!/usr/bin/env python
> +#
> +# Tests for persistent dirty bitmaps.
> +#
> +# Copyright: Vladimir Sementsov-Ogievskiy 2015

Do you want to also claim 2016?

> +++ b/tests/qemu-iotests/group
> @@ -150,3 +150,4 @@
>  145 auto quick
>  146 auto quick
>  148 rw auto quick
> +160 rw auto quick

Do we really have that many pending patch series with reserved test ids?


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH 10/22] qcow2-dirty-bitmap: add qcow2_bitmap_store()

[Eric Blake]

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

On 03/15/2016 02:04 PM, Vladimir Sementsov-Ogievskiy wrote:
> This function stores block dirty bitmap to qcow2. If the bitmap with
> the same name, size and granularity already exists, it will be
> rewritten, if the bitmap with the same name exists but granularity or
> size does not match, an error will be genrated.

s/genrated/generated/

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---

> +
> +/* if no id is provided, a new one is constructed */
> +static int qcow2_bitmap_create(BlockDriverState *bs, const char *name,
> +                               uint64_t size, int granularity)
> +{
> +    int ret;
> +    BDRVQcow2State *s = bs->opaque;
> +
> +    if (s->nb_bitmaps >= QCOW_MAX_DIRTY_BITMAPS) {
> +        return -EFBIG;
> +    }
> +
> +    /* Check that the name is unique */
> +    if (find_bitmap_by_name(bs, name) != NULL) {
> +        return -EEXIST;
> +    }
> +

Is the comment about constructing a name stale or misplaced?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH 12/22] qcow2-dirty-bitmap: add qcow2_bitmap_load_check()

[Eric Blake]

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

On 03/15/2016 02:04 PM, Vladimir Sementsov-Ogievskiy wrote:
> The function checks existing of the bitmap without loading it.

s/existing/the existence/

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH 18/22] qcow2-dirty-bitmaps: disallow stroing bitmap to other bs

[Eric Blake]

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

On 03/15/2016 02:04 PM, Vladimir Sementsov-Ogievskiy wrote:

In the subject: s/stroing/storing/

> Check, that bitmap is stored to the owning bs.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH 22/22] qcow2-dirty-bitmap: add EXTRA_DATA_COMPATIBLE flag

[Eric Blake]

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

On 03/15/2016 02:04 PM, Vladimir Sementsov-Ogievskiy wrote:
> If this flag is unset and exta data present the bitmap should be

s/exta/extra/
s/present/is present/

> read-only. For now just return error for this case.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/qcow2-dirty-bitmap.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/block/qcow2-dirty-bitmap.c b/block/qcow2-dirty-bitmap.c
> index 159e935..95c166c 100644
> --- a/block/qcow2-dirty-bitmap.c
> +++ b/block/qcow2-dirty-bitmap.c
> @@ -45,6 +45,7 @@
>  #define BME_RESERVED_FLAGS 0xfffffffc
>  #define BME_FLAG_IN_USE 1
>  #define BME_FLAG_AUTO   (1U << 1)
> +#define BME_FLAG_EXTRA_DATA_COMPATIBLE   (1U << 1)
>  
>  /* bits [1, 8] U [56, 63] are reserved */
>  #define BME_TABLE_ENTRY_RESERVED_MASK 0xff000000000001fe
> @@ -333,6 +334,13 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs, QCow2Bitmap *bm,
>          return NULL;
>      }
>  
> +    if (!(bmh->flags & BME_FLAG_EXTRA_DATA_COMPATIBLE) &&
> +            bmh->extra_data_size != 0) {
> +        error_setg(errp, "Uncompatible extra data found for bitmap '%s'",

s/Uncompatible/Incompatible/

> +                   bm->name);
> +        return NULL;
> +    }
> +
>      bitmap_table = g_try_malloc(bmh->bitmap_table_size * sizeof(uint64_t));
>      if (bitmap_table == NULL) {
>          error_setg_errno(errp, -ENOMEM,
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH 06/22] hbitmap: load/store

[John Snow]

On 03/15/2016 04:04 PM, Vladimir Sementsov-Ogievskiy wrote:
> Add functions for load/store HBitmap to BDS, using clusters table:
> Last level of the bitmap is splitted into chunks of 'cluster_size'
> size. Each cell of the table contains offset in bds, to load/store
> corresponding chunk.
> 
> Also,
>     0 in cell means all-zeroes-chunk (should not be saved)
>     1 in cell means all-ones-chunk (should not be saved)
>     hbitmap_prepare_store() fills table with
>       0 for all-zeroes chunks
>       1 for all-ones chunks
>       2 for others
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/dirty-bitmap.c         |  23 +++++
>  include/block/dirty-bitmap.h |  11 +++
>  include/qemu/hbitmap.h       |  12 +++
>  util/hbitmap.c               | 209 +++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 255 insertions(+)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index e68c177..816c6ee 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -396,3 +396,26 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
>  {
>      return hbitmap_count(bitmap->bitmap);
>  }
> +
> +int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
> +                           const uint64_t *table, uint32_t table_size,
> +                           uint32_t cluster_size)
> +{
> +    return hbitmap_load(bitmap->bitmap, bs, table, table_size, cluster_size);
> +}
> +
> +int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
> +                                    uint32_t cluster_size,
> +                                    uint64_t *table,
> +                                    uint32_t *table_size)
> +{
> +    return hbitmap_prepare_store(bitmap->bitmap, cluster_size,
> +                                 table, table_size);
> +}
> +
> +int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
> +                            const uint64_t *table, uint32_t table_size,
> +                            uint32_t cluster_size)
> +{
> +    return hbitmap_store(bitmap->bitmap, bs, table, table_size, cluster_size);
> +}
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 27515af..20cb540 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -43,4 +43,15 @@ void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>  void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
>  
> +int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
> +                           const uint64_t *table, uint32_t table_size,
> +                           uint32_t cluster_size);
> +int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
> +                                    uint32_t cluster_size,
> +                                    uint64_t *table,
> +                                    uint32_t *table_size);
> +int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
> +                            const uint64_t *table, uint32_t table_size,
> +                            uint32_t cluster_size);
> +
>  #endif
> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
> index 6d1da4d..d83bb79 100644
> --- a/include/qemu/hbitmap.h
> +++ b/include/qemu/hbitmap.h
> @@ -241,5 +241,17 @@ static inline size_t hbitmap_iter_next_word(HBitmapIter *hbi, unsigned long *p_c
>      return hbi->pos;
>  }
>  
> +typedef struct BlockDriverState BlockDriverState;
> +
> +int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
> +                 const uint64_t *table, uint32_t table_size,
> +                 uint32_t cluster_size);
> +
> +int hbitmap_prepare_store(const HBitmap *bitmap, uint32_t cluster_size,
> +                          uint64_t *table, uint32_t *table_size);
> +
> +int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
> +                  const uint64_t *table, uint32_t table_size,
> +                  uint32_t cluster_size);
>  
>  #endif
> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index 28595fb..1960e4f 100644
> --- a/util/hbitmap.c
> +++ b/util/hbitmap.c
> @@ -15,6 +15,8 @@
>  #include "qemu/host-utils.h"
>  #include "trace.h"
>  
> +#include "block/block.h"
> +
>  /* HBitmaps provides an array of bits.  The bits are stored as usual in an
>   * array of unsigned longs, but HBitmap is also optimized to provide fast
>   * iteration over set bits; going from one bit to the next is O(logB n)
> @@ -499,3 +501,210 @@ char *hbitmap_md5(const HBitmap *bitmap)
>      const guchar *data = (const guchar *)bitmap->levels[HBITMAP_LEVELS - 1];
>      return g_compute_checksum_for_data(G_CHECKSUM_MD5, data, size);
>  }
> +
> +/* hb_restore_levels()
> + * Using the last level restore all other levels
> + */
> +static void hb_restore_levels(HBitmap *bitmap)
> +{
> +    int64_t i, size, prev_size;
> +    int lev;
> +
> +    /* restore levels starting from penultimate to zero level, assuming
> +     * that the last level is ok */
> +    size = MAX((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
> +    for (lev = HBITMAP_LEVELS - 1; lev-- > 0; ) {
> +        prev_size = size;
> +        size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
> +        memset(bitmap->levels[lev], 0, size * sizeof(unsigned long));
> +
> +        for (i = 0; i < prev_size; ++i) {
> +            if (bitmap->levels[lev + 1][i]) {
> +                bitmap->levels[lev][i >> BITS_PER_LEVEL] |=
> +                    1UL << (i & (BITS_PER_LONG - 1));
> +            }
> +        }
> +    }
> +
> +    bitmap->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
> +}
> +
> +/* load_bitmap()
> + * Load dirty bitmap from file, using table with cluster offsets.
> + * Table entries are assumed to be in little endian format.
> + */
> +int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
> +                 const uint64_t *table, uint32_t table_size,
> +                 uint32_t cluster_size)
> +{
> +    uint32_t i;
> +    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
> +    uint8_t *end = cur + ((bitmap->size + 7) >> 3);
> +
> +    hbitmap_reset_all(bitmap);
> +
> +    for (i = 0; i < table_size && cur < end; ++i) {
> +        uint64_t offset = table[i];
> +        uint64_t count = MIN(cluster_size, end - cur);
> +
> +        /* Zero offset means zero region, offset = 1 means filled region.
> +         * Cluster is not allocated in both cases. */
> +        if (offset == 1) {
> +            memset(cur, 0xff, count);
> +        } else if (offset) {
> +            int ret = bdrv_pread(bs, offset, cur, count);
> +            if (ret < 0) {
> +                return ret;
> +            }
> +        }
> +
> +        cur += cluster_size;
> +    }
> +
> +    cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
> +    while (cur < end) {
> +        if (BITS_PER_LONG == 32) {
> +            le32_to_cpus((uint32_t *)cur);
> +        } else {
> +            le64_to_cpus((uint64_t *)cur);
> +        }
> +
> +        cur += sizeof(unsigned long);
> +    }
> +
> +    hb_restore_levels(bitmap);
> +
> +    return 0;
> +}
> +
> +static bool buffer_is_all_ones(void *buf, size_t len)
> +{
> +    /* FIXME */
> +    return false;
> +}
> +
> +/* hbitmap_prepare_store()
> + * Devide bitmap data into clusters, and then,
> + * for zero cluster: table[i] = 0
> + * for all-ones cluster: table[i] = 1
> + * for other clusters: table[i] = 2
> + */
> +int hbitmap_prepare_store(const HBitmap *bitmap,
> +                          uint32_t cluster_size,
> +                          uint64_t *table,
> +                          uint32_t *table_size)
> +{
> +    HBitmapIter hbi;
> +    hbitmap_iter_init(&hbi, bitmap, 0);
> +    uint64_t nb_bits_in_cl = (uint64_t)cluster_size << 3;
> +    uint32_t need_table_size =
> +            (bitmap->size + nb_bits_in_cl - 1) / nb_bits_in_cl;
> +
> +    if (table == NULL && *table_size == 0) {
> +        *table_size = need_table_size;
> +        return 0;
> +    }
> +
> +    if (*table_size < need_table_size) {
> +        return -ENOMEM;
> +    }
> +
> +    memset(table, 0, *table_size * sizeof(table[0]));
> +
> +    for (;;) {
> +        unsigned long cur;
> +        size_t pos = hbitmap_iter_next_word(&hbi, &cur);
> +        size_t byte = pos * sizeof(unsigned long);
> +        uint64_t bit = byte << 3;
> +        uint64_t nbits = MIN(cluster_size << 3, bitmap->size - bit), next_bit;
> +        size_t i = byte / cluster_size;
> +
> +        if (pos == -1) {
> +            break;
> +        }
> +
> +        if (pos % cluster_size != 0) {
> +            table[i] = 2;
> +        } else if (buffer_is_all_ones(&bitmap->levels[HBITMAP_LEVELS - 1][pos],
> +                               nbits >> 3)) {
> +            table[i] = 1;
> +            if (nbits & 7) {
> +                uint8_t last_byte =
> +                        *(((uint8_t *)&bitmap->levels[HBITMAP_LEVELS - 1][pos])
> +                                + (nbits >> 3));
> +                if (last_byte != ((1 << (nbits & 7)) - 1)) {
> +                    table[i] = 2;
> +                }
> +            }
> +        } else {
> +            table[i] = 2;
> +        }
> +
> +        next_bit = (i + 1) * cluster_size << 3;
> +
> +        if (next_bit >= bitmap->size) {
> +            break;
> +        }
> +
> +        hbitmap_iter_init(&hbi, bitmap, next_bit << bitmap->granularity);
> +    }
> +
> +    return 0;
> +}
> +
> +static void longs_to_le(unsigned long *longs, size_t count)
> +{
> +    unsigned long *end = longs + count;
> +    while (longs < end) {
> +        if (BITS_PER_LONG == 32) {
> +            cpu_to_le32s((uint32_t *)longs);
> +        } else {
> +            cpu_to_le64s((uint64_t *)longs);
> +        }
> +
> +        longs++;
> +    }
> +}
> +
> +/* store_bitmap()
> + * update bitmap table by storing bitmap to it.
> + * bitmap table entries are assumed to be in big endian format
> + * On the error, the resulting bitmap table is valid for clearing, but
> + * may contain invalid bitmap */
> +int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
> +                  const uint64_t *table, uint32_t table_size,
> +                  uint32_t cluster_size)
> +{
> +    int i;
> +    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
> +    uint8_t *end = cur +
> +        ((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL) * (sizeof(long));
> +
> +    for (i = 0; i < table_size && cur < end; ++i) {
> +        uint64_t offset = table[i];
> +        uint64_t count = MIN(cluster_size, end - cur);
> +
> +        /* Zero offset means zero region, offset = 1 means filled region.
> +         * Cluster is not allocated in both cases. */
> +        if (offset > 1) {
> +            int ret;
> +            if (cpu_to_le16(1) == 1) {
> +                ret = bdrv_pwrite(bs, offset, cur, count);

I suspect that the reason you're using bdrv_pwrite down in here is to
avoid a buffered copy where hbitmap prepares a buffer and then the file
format layer decides what to do with it, opting instead to allow the
hbitmap layer itself to do a direct write.

I don't think this is going to work, though -- hbitmaps are a generic
utility and shouldn't have block layer access.

 I think the standard, naive design is the right one:

(1) qcow2-bitmap.c calls
bdrv_dirty_bitmap_serialize(bdrvdirtybitmap *bitmap, void *out);
and serialization primitives for hbitmap are used to construct a buffer
placed in *out.

(2) qcow2-bitmap.c writes this buffer itself where it needs it, using
bdrv_pwrite.

If this proves to be too slow, we can optimize it later.

> +            } else {
> +                void *tmp = g_memdup(cur, count);
> +                longs_to_le((unsigned long *)cur,
> +                            count / sizeof(unsigned long));
> +                ret = bdrv_pwrite(bs, offset, tmp, count);
> +                g_free(tmp);
> +            }
> +
> +            if (ret < 0) {
> +                return ret;
> +            }
> +        }
> +
> +        cur += cluster_size;
> +    }
> +
> +    return 0;
> +}
> 

Re: [Qemu-devel] [PATCH 06/22] hbitmap: load/store

[Vladimir Sementsov-Ogievskiy]

On 23.03.2016 00:49, John Snow wrote:
>
> On 03/15/2016 04:04 PM, Vladimir Sementsov-Ogievskiy wrote:
>> Add functions for load/store HBitmap to BDS, using clusters table:
>> Last level of the bitmap is splitted into chunks of 'cluster_size'
>> size. Each cell of the table contains offset in bds, to load/store
>> corresponding chunk.
>>
>> Also,
>>      0 in cell means all-zeroes-chunk (should not be saved)
>>      1 in cell means all-ones-chunk (should not be saved)
>>      hbitmap_prepare_store() fills table with
>>        0 for all-zeroes chunks
>>        1 for all-ones chunks
>>        2 for others
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   block/dirty-bitmap.c         |  23 +++++
>>   include/block/dirty-bitmap.h |  11 +++
>>   include/qemu/hbitmap.h       |  12 +++
>>   util/hbitmap.c               | 209 +++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 255 insertions(+)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index e68c177..816c6ee 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -396,3 +396,26 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
>>   {
>>       return hbitmap_count(bitmap->bitmap);
>>   }
>> +
>> +int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
>> +                           const uint64_t *table, uint32_t table_size,
>> +                           uint32_t cluster_size)
>> +{
>> +    return hbitmap_load(bitmap->bitmap, bs, table, table_size, cluster_size);
>> +}
>> +
>> +int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
>> +                                    uint32_t cluster_size,
>> +                                    uint64_t *table,
>> +                                    uint32_t *table_size)
>> +{
>> +    return hbitmap_prepare_store(bitmap->bitmap, cluster_size,
>> +                                 table, table_size);
>> +}
>> +
>> +int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
>> +                            const uint64_t *table, uint32_t table_size,
>> +                            uint32_t cluster_size)
>> +{
>> +    return hbitmap_store(bitmap->bitmap, bs, table, table_size, cluster_size);
>> +}
>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>> index 27515af..20cb540 100644
>> --- a/include/block/dirty-bitmap.h
>> +++ b/include/block/dirty-bitmap.h
>> @@ -43,4 +43,15 @@ void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset);
>>   int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>>   void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
>>   
>> +int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
>> +                           const uint64_t *table, uint32_t table_size,
>> +                           uint32_t cluster_size);
>> +int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
>> +                                    uint32_t cluster_size,
>> +                                    uint64_t *table,
>> +                                    uint32_t *table_size);
>> +int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap, BlockDriverState *bs,
>> +                            const uint64_t *table, uint32_t table_size,
>> +                            uint32_t cluster_size);
>> +
>>   #endif
>> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
>> index 6d1da4d..d83bb79 100644
>> --- a/include/qemu/hbitmap.h
>> +++ b/include/qemu/hbitmap.h
>> @@ -241,5 +241,17 @@ static inline size_t hbitmap_iter_next_word(HBitmapIter *hbi, unsigned long *p_c
>>       return hbi->pos;
>>   }
>>   
>> +typedef struct BlockDriverState BlockDriverState;
>> +
>> +int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
>> +                 const uint64_t *table, uint32_t table_size,
>> +                 uint32_t cluster_size);
>> +
>> +int hbitmap_prepare_store(const HBitmap *bitmap, uint32_t cluster_size,
>> +                          uint64_t *table, uint32_t *table_size);
>> +
>> +int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
>> +                  const uint64_t *table, uint32_t table_size,
>> +                  uint32_t cluster_size);
>>   
>>   #endif
>> diff --git a/util/hbitmap.c b/util/hbitmap.c
>> index 28595fb..1960e4f 100644
>> --- a/util/hbitmap.c
>> +++ b/util/hbitmap.c
>> @@ -15,6 +15,8 @@
>>   #include "qemu/host-utils.h"
>>   #include "trace.h"
>>   
>> +#include "block/block.h"
>> +
>>   /* HBitmaps provides an array of bits.  The bits are stored as usual in an
>>    * array of unsigned longs, but HBitmap is also optimized to provide fast
>>    * iteration over set bits; going from one bit to the next is O(logB n)
>> @@ -499,3 +501,210 @@ char *hbitmap_md5(const HBitmap *bitmap)
>>       const guchar *data = (const guchar *)bitmap->levels[HBITMAP_LEVELS - 1];
>>       return g_compute_checksum_for_data(G_CHECKSUM_MD5, data, size);
>>   }
>> +
>> +/* hb_restore_levels()
>> + * Using the last level restore all other levels
>> + */
>> +static void hb_restore_levels(HBitmap *bitmap)
>> +{
>> +    int64_t i, size, prev_size;
>> +    int lev;
>> +
>> +    /* restore levels starting from penultimate to zero level, assuming
>> +     * that the last level is ok */
>> +    size = MAX((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
>> +    for (lev = HBITMAP_LEVELS - 1; lev-- > 0; ) {
>> +        prev_size = size;
>> +        size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
>> +        memset(bitmap->levels[lev], 0, size * sizeof(unsigned long));
>> +
>> +        for (i = 0; i < prev_size; ++i) {
>> +            if (bitmap->levels[lev + 1][i]) {
>> +                bitmap->levels[lev][i >> BITS_PER_LEVEL] |=
>> +                    1UL << (i & (BITS_PER_LONG - 1));
>> +            }
>> +        }
>> +    }
>> +
>> +    bitmap->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
>> +}
>> +
>> +/* load_bitmap()
>> + * Load dirty bitmap from file, using table with cluster offsets.
>> + * Table entries are assumed to be in little endian format.
>> + */
>> +int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
>> +                 const uint64_t *table, uint32_t table_size,
>> +                 uint32_t cluster_size)
>> +{
>> +    uint32_t i;
>> +    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
>> +    uint8_t *end = cur + ((bitmap->size + 7) >> 3);
>> +
>> +    hbitmap_reset_all(bitmap);
>> +
>> +    for (i = 0; i < table_size && cur < end; ++i) {
>> +        uint64_t offset = table[i];
>> +        uint64_t count = MIN(cluster_size, end - cur);
>> +
>> +        /* Zero offset means zero region, offset = 1 means filled region.
>> +         * Cluster is not allocated in both cases. */
>> +        if (offset == 1) {
>> +            memset(cur, 0xff, count);
>> +        } else if (offset) {
>> +            int ret = bdrv_pread(bs, offset, cur, count);
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
>> +        }
>> +
>> +        cur += cluster_size;
>> +    }
>> +
>> +    cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
>> +    while (cur < end) {
>> +        if (BITS_PER_LONG == 32) {
>> +            le32_to_cpus((uint32_t *)cur);
>> +        } else {
>> +            le64_to_cpus((uint64_t *)cur);
>> +        }
>> +
>> +        cur += sizeof(unsigned long);
>> +    }
>> +
>> +    hb_restore_levels(bitmap);
>> +
>> +    return 0;
>> +}
>> +
>> +static bool buffer_is_all_ones(void *buf, size_t len)
>> +{
>> +    /* FIXME */
>> +    return false;
>> +}
>> +
>> +/* hbitmap_prepare_store()
>> + * Devide bitmap data into clusters, and then,
>> + * for zero cluster: table[i] = 0
>> + * for all-ones cluster: table[i] = 1
>> + * for other clusters: table[i] = 2
>> + */
>> +int hbitmap_prepare_store(const HBitmap *bitmap,
>> +                          uint32_t cluster_size,
>> +                          uint64_t *table,
>> +                          uint32_t *table_size)
>> +{
>> +    HBitmapIter hbi;
>> +    hbitmap_iter_init(&hbi, bitmap, 0);
>> +    uint64_t nb_bits_in_cl = (uint64_t)cluster_size << 3;
>> +    uint32_t need_table_size =
>> +            (bitmap->size + nb_bits_in_cl - 1) / nb_bits_in_cl;
>> +
>> +    if (table == NULL && *table_size == 0) {
>> +        *table_size = need_table_size;
>> +        return 0;
>> +    }
>> +
>> +    if (*table_size < need_table_size) {
>> +        return -ENOMEM;
>> +    }
>> +
>> +    memset(table, 0, *table_size * sizeof(table[0]));
>> +
>> +    for (;;) {
>> +        unsigned long cur;
>> +        size_t pos = hbitmap_iter_next_word(&hbi, &cur);
>> +        size_t byte = pos * sizeof(unsigned long);
>> +        uint64_t bit = byte << 3;
>> +        uint64_t nbits = MIN(cluster_size << 3, bitmap->size - bit), next_bit;
>> +        size_t i = byte / cluster_size;
>> +
>> +        if (pos == -1) {
>> +            break;
>> +        }
>> +
>> +        if (pos % cluster_size != 0) {
>> +            table[i] = 2;
>> +        } else if (buffer_is_all_ones(&bitmap->levels[HBITMAP_LEVELS - 1][pos],
>> +                               nbits >> 3)) {
>> +            table[i] = 1;
>> +            if (nbits & 7) {
>> +                uint8_t last_byte =
>> +                        *(((uint8_t *)&bitmap->levels[HBITMAP_LEVELS - 1][pos])
>> +                                + (nbits >> 3));
>> +                if (last_byte != ((1 << (nbits & 7)) - 1)) {
>> +                    table[i] = 2;
>> +                }
>> +            }
>> +        } else {
>> +            table[i] = 2;
>> +        }
>> +
>> +        next_bit = (i + 1) * cluster_size << 3;
>> +
>> +        if (next_bit >= bitmap->size) {
>> +            break;
>> +        }
>> +
>> +        hbitmap_iter_init(&hbi, bitmap, next_bit << bitmap->granularity);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void longs_to_le(unsigned long *longs, size_t count)
>> +{
>> +    unsigned long *end = longs + count;
>> +    while (longs < end) {
>> +        if (BITS_PER_LONG == 32) {
>> +            cpu_to_le32s((uint32_t *)longs);
>> +        } else {
>> +            cpu_to_le64s((uint64_t *)longs);
>> +        }
>> +
>> +        longs++;
>> +    }
>> +}
>> +
>> +/* store_bitmap()
>> + * update bitmap table by storing bitmap to it.
>> + * bitmap table entries are assumed to be in big endian format
>> + * On the error, the resulting bitmap table is valid for clearing, but
>> + * may contain invalid bitmap */
>> +int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
>> +                  const uint64_t *table, uint32_t table_size,
>> +                  uint32_t cluster_size)
>> +{
>> +    int i;
>> +    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
>> +    uint8_t *end = cur +
>> +        ((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL) * (sizeof(long));
>> +
>> +    for (i = 0; i < table_size && cur < end; ++i) {
>> +        uint64_t offset = table[i];
>> +        uint64_t count = MIN(cluster_size, end - cur);
>> +
>> +        /* Zero offset means zero region, offset = 1 means filled region.
>> +         * Cluster is not allocated in both cases. */
>> +        if (offset > 1) {
>> +            int ret;
>> +            if (cpu_to_le16(1) == 1) {
>> +                ret = bdrv_pwrite(bs, offset, cur, count);
> I suspect that the reason you're using bdrv_pwrite down in here is to
> avoid a buffered copy where hbitmap prepares a buffer and then the file
> format layer decides what to do with it, opting instead to allow the
> hbitmap layer itself to do a direct write.
>
> I don't think this is going to work, though -- hbitmaps are a generic
> utility and shouldn't have block layer access.
>
>   I think the standard, naive design is the right one:
>
> (1) qcow2-bitmap.c calls
> bdrv_dirty_bitmap_serialize(bdrvdirtybitmap *bitmap, void *out);
> and serialization primitives for hbitmap are used to construct a buffer
> placed in *out.
>
> (2) qcow2-bitmap.c writes this buffer itself where it needs it, using
> bdrv_pwrite.
>
> If this proves to be too slow, we can optimize it later.

May be just pass writing function to hbitmap_store, like int 
(*pwrite_func)(void *opaque, int64_t offset, const void *buf, int bytes) 
? Serialization is superfluous here. It is needed for migration, where 
we forced to work with data chunks protocol layer. But why to split here 
the bitmap, dance with granularity and block layer wrappers, and make an 
additional copy of the data?

>
>> +            } else {
>> +                void *tmp = g_memdup(cur, count);
>> +                longs_to_le((unsigned long *)cur,
>> +                            count / sizeof(unsigned long));
>> +                ret = bdrv_pwrite(bs, offset, tmp, count);
>> +                g_free(tmp);
>> +            }
>> +
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
>> +        }
>> +
>> +        cur += cluster_size;
>> +    }
>> +
>> +    return 0;
>> +}
>>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 10/22] qcow2-dirty-bitmap: add qcow2_bitmap_store()

[Vladimir Sementsov-Ogievskiy]

On 22.03.2016 21:49, Eric Blake wrote:
> On 03/15/2016 02:04 PM, Vladimir Sementsov-Ogievskiy wrote:
>> This function stores block dirty bitmap to qcow2. If the bitmap with
>> the same name, size and granularity already exists, it will be
>> rewritten, if the bitmap with the same name exists but granularity or
>> size does not match, an error will be genrated.
> s/genrated/generated/
>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> +
>> +/* if no id is provided, a new one is constructed */
>> +static int qcow2_bitmap_create(BlockDriverState *bs, const char *name,
>> +                               uint64_t size, int granularity)
>> +{
>> +    int ret;
>> +    BDRVQcow2State *s = bs->opaque;
>> +
>> +    if (s->nb_bitmaps >= QCOW_MAX_DIRTY_BITMAPS) {
>> +        return -EFBIG;
>> +    }
>> +
>> +    /* Check that the name is unique */
>> +    if (find_bitmap_by_name(bs, name) != NULL) {
>> +        return -EEXIST;
>> +    }
>> +
> Is the comment about constructing a name stale or misplaced?
>

It's a mistake, thanks. Don't remember, where I stole it)

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 20/22] iotests: test internal persistent dirty bitmap

[Vladimir Sementsov-Ogievskiy]

On 22.03.2016 21:27, Eric Blake wrote:
> On 03/15/2016 02:04 PM, Vladimir Sementsov-Ogievskiy wrote:
>> Add simple test cases for testing persistent dirty bitmaps.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/160        | 112 ++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/160.out    |  21 ++++++++
>>   tests/qemu-iotests/group      |   1 +
>>   tests/qemu-iotests/iotests.py |   6 +++
>>   4 files changed, 140 insertions(+)
>>   create mode 100755 tests/qemu-iotests/160
>>   create mode 100644 tests/qemu-iotests/160.out
>>
>> diff --git a/tests/qemu-iotests/160 b/tests/qemu-iotests/160
>> new file mode 100755
>> index 0000000..f9843da
>> --- /dev/null
>> +++ b/tests/qemu-iotests/160
>> @@ -0,0 +1,112 @@
>> +#!/usr/bin/env python
>> +#
>> +# Tests for persistent dirty bitmaps.
>> +#
>> +# Copyright: Vladimir Sementsov-Ogievskiy 2015
> Do you want to also claim 2016?
>
>> +++ b/tests/qemu-iotests/group
>> @@ -150,3 +150,4 @@
>>   145 auto quick
>>   146 auto quick
>>   148 rw auto quick
>> +160 rw auto quick
> Do we really have that many pending patch series with reserved test ids?

I'm just sick of renaming it on rebases. When the series will be closer 
to accepting I'll be able to rename it appropriately

>
>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 06/22] hbitmap: load/store

[John Snow]

On 03/23/2016 04:22 AM, Vladimir Sementsov-Ogievskiy wrote:
> On 23.03.2016 00:49, John Snow wrote:
>>
>> On 03/15/2016 04:04 PM, Vladimir Sementsov-Ogievskiy wrote:
>>> Add functions for load/store HBitmap to BDS, using clusters table:
>>> Last level of the bitmap is splitted into chunks of 'cluster_size'
>>> size. Each cell of the table contains offset in bds, to load/store
>>> corresponding chunk.
>>>
>>> Also,
>>>      0 in cell means all-zeroes-chunk (should not be saved)
>>>      1 in cell means all-ones-chunk (should not be saved)
>>>      hbitmap_prepare_store() fills table with
>>>        0 for all-zeroes chunks
>>>        1 for all-ones chunks
>>>        2 for others
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>>   block/dirty-bitmap.c         |  23 +++++
>>>   include/block/dirty-bitmap.h |  11 +++
>>>   include/qemu/hbitmap.h       |  12 +++
>>>   util/hbitmap.c               | 209
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>   4 files changed, 255 insertions(+)
>>>
>>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>>> index e68c177..816c6ee 100644
>>> --- a/block/dirty-bitmap.c
>>> +++ b/block/dirty-bitmap.c
>>> @@ -396,3 +396,26 @@ int64_t bdrv_get_dirty_count(BdrvDirtyBitmap
>>> *bitmap)
>>>   {
>>>       return hbitmap_count(bitmap->bitmap);
>>>   }
>>> +
>>> +int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap, BlockDriverState
>>> *bs,
>>> +                           const uint64_t *table, uint32_t table_size,
>>> +                           uint32_t cluster_size)
>>> +{
>>> +    return hbitmap_load(bitmap->bitmap, bs, table, table_size,
>>> cluster_size);
>>> +}
>>> +
>>> +int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
>>> +                                    uint32_t cluster_size,
>>> +                                    uint64_t *table,
>>> +                                    uint32_t *table_size)
>>> +{
>>> +    return hbitmap_prepare_store(bitmap->bitmap, cluster_size,
>>> +                                 table, table_size);
>>> +}
>>> +
>>> +int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap,
>>> BlockDriverState *bs,
>>> +                            const uint64_t *table, uint32_t table_size,
>>> +                            uint32_t cluster_size)
>>> +{
>>> +    return hbitmap_store(bitmap->bitmap, bs, table, table_size,
>>> cluster_size);
>>> +}
>>> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
>>> index 27515af..20cb540 100644
>>> --- a/include/block/dirty-bitmap.h
>>> +++ b/include/block/dirty-bitmap.h
>>> @@ -43,4 +43,15 @@ void bdrv_set_dirty_iter(struct HBitmapIter *hbi,
>>> int64_t offset);
>>>   int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
>>>   void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
>>>   +int bdrv_dirty_bitmap_load(BdrvDirtyBitmap *bitmap,
>>> BlockDriverState *bs,
>>> +                           const uint64_t *table, uint32_t table_size,
>>> +                           uint32_t cluster_size);
>>> +int bdrv_dirty_bitmap_prepare_store(const BdrvDirtyBitmap *bitmap,
>>> +                                    uint32_t cluster_size,
>>> +                                    uint64_t *table,
>>> +                                    uint32_t *table_size);
>>> +int bdrv_dirty_bitmap_store(const BdrvDirtyBitmap *bitmap,
>>> BlockDriverState *bs,
>>> +                            const uint64_t *table, uint32_t table_size,
>>> +                            uint32_t cluster_size);
>>> +
>>>   #endif
>>> diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
>>> index 6d1da4d..d83bb79 100644
>>> --- a/include/qemu/hbitmap.h
>>> +++ b/include/qemu/hbitmap.h
>>> @@ -241,5 +241,17 @@ static inline size_t
>>> hbitmap_iter_next_word(HBitmapIter *hbi, unsigned long *p_c
>>>       return hbi->pos;
>>>   }
>>>   +typedef struct BlockDriverState BlockDriverState;
>>> +
>>> +int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
>>> +                 const uint64_t *table, uint32_t table_size,
>>> +                 uint32_t cluster_size);
>>> +
>>> +int hbitmap_prepare_store(const HBitmap *bitmap, uint32_t cluster_size,
>>> +                          uint64_t *table, uint32_t *table_size);
>>> +
>>> +int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
>>> +                  const uint64_t *table, uint32_t table_size,
>>> +                  uint32_t cluster_size);
>>>     #endif
>>> diff --git a/util/hbitmap.c b/util/hbitmap.c
>>> index 28595fb..1960e4f 100644
>>> --- a/util/hbitmap.c
>>> +++ b/util/hbitmap.c
>>> @@ -15,6 +15,8 @@
>>>   #include "qemu/host-utils.h"
>>>   #include "trace.h"
>>>   +#include "block/block.h"
>>> +
>>>   /* HBitmaps provides an array of bits.  The bits are stored as
>>> usual in an
>>>    * array of unsigned longs, but HBitmap is also optimized to
>>> provide fast
>>>    * iteration over set bits; going from one bit to the next is
>>> O(logB n)
>>> @@ -499,3 +501,210 @@ char *hbitmap_md5(const HBitmap *bitmap)
>>>       const guchar *data = (const guchar
>>> *)bitmap->levels[HBITMAP_LEVELS - 1];
>>>       return g_compute_checksum_for_data(G_CHECKSUM_MD5, data, size);
>>>   }
>>> +
>>> +/* hb_restore_levels()
>>> + * Using the last level restore all other levels
>>> + */
>>> +static void hb_restore_levels(HBitmap *bitmap)
>>> +{
>>> +    int64_t i, size, prev_size;
>>> +    int lev;
>>> +
>>> +    /* restore levels starting from penultimate to zero level, assuming
>>> +     * that the last level is ok */
>>> +    size = MAX((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL,
>>> 1);
>>> +    for (lev = HBITMAP_LEVELS - 1; lev-- > 0; ) {
>>> +        prev_size = size;
>>> +        size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
>>> +        memset(bitmap->levels[lev], 0, size * sizeof(unsigned long));
>>> +
>>> +        for (i = 0; i < prev_size; ++i) {
>>> +            if (bitmap->levels[lev + 1][i]) {
>>> +                bitmap->levels[lev][i >> BITS_PER_LEVEL] |=
>>> +                    1UL << (i & (BITS_PER_LONG - 1));
>>> +            }
>>> +        }
>>> +    }
>>> +
>>> +    bitmap->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
>>> +}
>>> +
>>> +/* load_bitmap()
>>> + * Load dirty bitmap from file, using table with cluster offsets.
>>> + * Table entries are assumed to be in little endian format.
>>> + */
>>> +int hbitmap_load(HBitmap *bitmap, BlockDriverState *bs,
>>> +                 const uint64_t *table, uint32_t table_size,
>>> +                 uint32_t cluster_size)
>>> +{
>>> +    uint32_t i;
>>> +    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
>>> +    uint8_t *end = cur + ((bitmap->size + 7) >> 3);
>>> +
>>> +    hbitmap_reset_all(bitmap);
>>> +
>>> +    for (i = 0; i < table_size && cur < end; ++i) {
>>> +        uint64_t offset = table[i];
>>> +        uint64_t count = MIN(cluster_size, end - cur);
>>> +
>>> +        /* Zero offset means zero region, offset = 1 means filled
>>> region.
>>> +         * Cluster is not allocated in both cases. */
>>> +        if (offset == 1) {
>>> +            memset(cur, 0xff, count);
>>> +        } else if (offset) {
>>> +            int ret = bdrv_pread(bs, offset, cur, count);
>>> +            if (ret < 0) {
>>> +                return ret;
>>> +            }
>>> +        }
>>> +
>>> +        cur += cluster_size;
>>> +    }
>>> +
>>> +    cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
>>> +    while (cur < end) {
>>> +        if (BITS_PER_LONG == 32) {
>>> +            le32_to_cpus((uint32_t *)cur);
>>> +        } else {
>>> +            le64_to_cpus((uint64_t *)cur);
>>> +        }
>>> +
>>> +        cur += sizeof(unsigned long);
>>> +    }
>>> +
>>> +    hb_restore_levels(bitmap);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static bool buffer_is_all_ones(void *buf, size_t len)
>>> +{
>>> +    /* FIXME */
>>> +    return false;
>>> +}
>>> +
>>> +/* hbitmap_prepare_store()
>>> + * Devide bitmap data into clusters, and then,
>>> + * for zero cluster: table[i] = 0
>>> + * for all-ones cluster: table[i] = 1
>>> + * for other clusters: table[i] = 2
>>> + */
>>> +int hbitmap_prepare_store(const HBitmap *bitmap,
>>> +                          uint32_t cluster_size,
>>> +                          uint64_t *table,
>>> +                          uint32_t *table_size)
>>> +{
>>> +    HBitmapIter hbi;
>>> +    hbitmap_iter_init(&hbi, bitmap, 0);
>>> +    uint64_t nb_bits_in_cl = (uint64_t)cluster_size << 3;
>>> +    uint32_t need_table_size =
>>> +            (bitmap->size + nb_bits_in_cl - 1) / nb_bits_in_cl;
>>> +
>>> +    if (table == NULL && *table_size == 0) {
>>> +        *table_size = need_table_size;
>>> +        return 0;
>>> +    }
>>> +
>>> +    if (*table_size < need_table_size) {
>>> +        return -ENOMEM;
>>> +    }
>>> +
>>> +    memset(table, 0, *table_size * sizeof(table[0]));
>>> +
>>> +    for (;;) {
>>> +        unsigned long cur;
>>> +        size_t pos = hbitmap_iter_next_word(&hbi, &cur);
>>> +        size_t byte = pos * sizeof(unsigned long);
>>> +        uint64_t bit = byte << 3;
>>> +        uint64_t nbits = MIN(cluster_size << 3, bitmap->size - bit),
>>> next_bit;
>>> +        size_t i = byte / cluster_size;
>>> +
>>> +        if (pos == -1) {
>>> +            break;
>>> +        }
>>> +
>>> +        if (pos % cluster_size != 0) {
>>> +            table[i] = 2;
>>> +        } else if (buffer_is_all_ones(&bitmap->levels[HBITMAP_LEVELS
>>> - 1][pos],
>>> +                               nbits >> 3)) {
>>> +            table[i] = 1;
>>> +            if (nbits & 7) {
>>> +                uint8_t last_byte =
>>> +                        *(((uint8_t *)&bitmap->levels[HBITMAP_LEVELS
>>> - 1][pos])
>>> +                                + (nbits >> 3));
>>> +                if (last_byte != ((1 << (nbits & 7)) - 1)) {
>>> +                    table[i] = 2;
>>> +                }
>>> +            }
>>> +        } else {
>>> +            table[i] = 2;
>>> +        }
>>> +
>>> +        next_bit = (i + 1) * cluster_size << 3;
>>> +
>>> +        if (next_bit >= bitmap->size) {
>>> +            break;
>>> +        }
>>> +
>>> +        hbitmap_iter_init(&hbi, bitmap, next_bit <<
>>> bitmap->granularity);
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void longs_to_le(unsigned long *longs, size_t count)
>>> +{
>>> +    unsigned long *end = longs + count;
>>> +    while (longs < end) {
>>> +        if (BITS_PER_LONG == 32) {
>>> +            cpu_to_le32s((uint32_t *)longs);
>>> +        } else {
>>> +            cpu_to_le64s((uint64_t *)longs);
>>> +        }
>>> +
>>> +        longs++;
>>> +    }
>>> +}
>>> +
>>> +/* store_bitmap()
>>> + * update bitmap table by storing bitmap to it.
>>> + * bitmap table entries are assumed to be in big endian format
>>> + * On the error, the resulting bitmap table is valid for clearing, but
>>> + * may contain invalid bitmap */
>>> +int hbitmap_store(HBitmap *bitmap, BlockDriverState *bs,
>>> +                  const uint64_t *table, uint32_t table_size,
>>> +                  uint32_t cluster_size)
>>> +{
>>> +    int i;
>>> +    uint8_t *cur = (uint8_t *)bitmap->levels[HBITMAP_LEVELS - 1];
>>> +    uint8_t *end = cur +
>>> +        ((bitmap->size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL) *
>>> (sizeof(long));
>>> +
>>> +    for (i = 0; i < table_size && cur < end; ++i) {
>>> +        uint64_t offset = table[i];
>>> +        uint64_t count = MIN(cluster_size, end - cur);
>>> +
>>> +        /* Zero offset means zero region, offset = 1 means filled
>>> region.
>>> +         * Cluster is not allocated in both cases. */
>>> +        if (offset > 1) {
>>> +            int ret;
>>> +            if (cpu_to_le16(1) == 1) {
>>> +                ret = bdrv_pwrite(bs, offset, cur, count);
>> I suspect that the reason you're using bdrv_pwrite down in here is to
>> avoid a buffered copy where hbitmap prepares a buffer and then the file
>> format layer decides what to do with it, opting instead to allow the
>> hbitmap layer itself to do a direct write.
>>
>> I don't think this is going to work, though -- hbitmaps are a generic
>> utility and shouldn't have block layer access.
>>
>>   I think the standard, naive design is the right one:
>>
>> (1) qcow2-bitmap.c calls
>> bdrv_dirty_bitmap_serialize(bdrvdirtybitmap *bitmap, void *out);
>> and serialization primitives for hbitmap are used to construct a buffer
>> placed in *out.
>>
>> (2) qcow2-bitmap.c writes this buffer itself where it needs it, using
>> bdrv_pwrite.
>>
>> If this proves to be too slow, we can optimize it later.
> 
> May be just pass writing function to hbitmap_store, like int
> (*pwrite_func)(void *opaque, int64_t offset, const void *buf, int bytes)
> ? Serialization is superfluous here. It is needed for migration, where
> we forced to work with data chunks protocol layer. But why to split here
> the bitmap, dance with granularity and block layer wrappers, and make an
> additional copy of the data?
> 

I hate to say it, but I think this is premature optimization. If you can
prove there is a meaningful difference between a buffered serialization
and a zero-copy, I'll go along with it -- but I think until we are
bench-marking quite large bitmaps, we won't see much of a difference.

This should be easy enough to improve later if we need to.

>>
>>> +            } else {
>>> +                void *tmp = g_memdup(cur, count);
>>> +                longs_to_le((unsigned long *)cur,
>>> +                            count / sizeof(unsigned long));
>>> +                ret = bdrv_pwrite(bs, offset, tmp, count);
>>> +                g_free(tmp);
>>> +            }
>>> +
>>> +            if (ret < 0) {
>>> +                return ret;
>>> +            }
>>> +        }
>>> +
>>> +        cur += cluster_size;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>>
> 
>