[Qemu-devel] [PATCH 0/9] QMP: Introduce incremental drive-backup with in-memory dirty bitmap

[Qemu-devel] [PATCH 0/9] QMP: Introduce incremental drive-backup with in-memory dirty bitmap

[Fam Zheng]

This implements incremental backup.

A few new QMP commands related to dirty bitmap are added:

    dirty-bitmap-add *
    dirty-bitmap-disable *
    dirty-bitmap-remove

    (*: also supported as transactions)

As their name implies, they manipulate a block device's dirty bitmap. This
doesn't interfere with dirty bitmap used for migration, backup, mirror, etc,
which don't have a name and are invisible to user. Only named bitmaps (created
by dirty-bitmap-add) can be disabled/removed by user.

They are added to support "user controlled write tracking", so as to determine
the range of date for incremental backup.

A new sync mode for drive-backup is introduced:

    drive-backup device=.. mode=.. sync=dirty-bitmap bitmap=bitmap0

Which will scan dirty bitmap "bitmap0" and only copy all dirty sectors to
target.

Now, let's see the usage with a simple example:

    # Start the guest
    vm = VM()
    vm.start()

    # Fake some guest writes with "qemu-io", this is before creating dirty
    # bitmap, so it won't be copied
    vm.hmp('qemu-io ide0-hd0 "write -P 0xa 512k 1M"')

    # Create a dirty bitmap to track writes
    vm.qmp("dirty-bitmap-add", device="ide0-hd0", name="dirty-0")

    # Fake some more guest writes with "qemu-io", this will be copied
    vm.hmp('qemu-io ide0-hd0 "write -P 0xa 512M 1M"')

    # Now "disable" the first dirty bitmap, do the backup according to it,
    # at meantime continue to track dirty with a new dirty bitmap
    vm.qmp("transaction", actions=[
        {
            'type': 'dirty-bitmap-disable', 'data': {
                'device': 'ide0-hd0',
                'name': 'dirty-0'
            }
        }, {
            'type': 'dirty-bitmap-add', 'data': {
                'device': 'ide0-hd0',
                'name': 'dirty-1'
                }
        }, {
            'type': 'drive-backup', 'data': {
                'device': 'ide0-hd0',
                'target': '/tmp/incre.qcow2',
                'bitmap': 'dirty-0',
                'sync': 'dirty-bitmap'
                }
            }
        ])

    # Once backup job started, the dirty bitmap can be removed (actually only
    # hidden from user since it is still referenced by block job
    vm.qmp("dirty-bitmap-remove", device="ide0-hd0", name="dirty-0")

Wait the block job to complete, then let's check the target image to see what
data is copied:

    ./qemu-img map /tmp/incre.qcow2
    Offset          Length          Mapped to       File
    0x20000000      0x100000        0x160000        /tmp/incre.qcow2

Yes, only the data written after "dirty0" creation is copied.

If this interface looks good, test cases will be included in the next revision.

Also, it's quite easy to use an rbd server or other protocols to do the remote
backup over network.

P.S. Persistent dirty bitmap could be built on top of this series, but is not
yet implemented, because the storage format and integrity measures are not
quite clear for now. The discussion is still open and any questions, ideas, use
cases and concerns are all welcome!

Fam


Fam Zheng (9):
  block: Introduce reference count for dirty bitmaps
  qapi: Add optional field "name" to block dirty bitmap
  block: Add bdrv_dirty_bitmap_make_anon
  qmp: Add dirty-bitmap-add and dirty-bitmap-remove
  block: Handle error of bdrv_getlength in bdrv_create_dirty_bitmap
  block: Introduce bdrv_dirty_bitmap_granularity()
  block: Add support of "dirty-bitmap" sync mode
  qmp: Add dirty-bitmap-disable command
  qapi: Add transaction support to dirty-bitmap-{add,disable}

 block-migration.c         |   3 +-
 block.c                   |  79 +++++++++++++++++++++--
 block/backup.c            |  34 +++++++++-
 block/mirror.c            |   6 +-
 blockdev.c                | 156 +++++++++++++++++++++++++++++++++++++++++++++-
 hmp.c                     |   3 +-
 include/block/block.h     |  13 +++-
 include/block/block_int.h |   2 +
 qapi-schema.json          |  79 +++++++++++++++++++++--
 qmp-commands.hx           |  61 +++++++++++++++++-
 10 files changed, 416 insertions(+), 20 deletions(-)

-- 
1.8.5.2

[Qemu-devel] [PATCH 1/9] block: Introduce reference count for dirty bitmaps

[Fam Zheng]

A dirty bitmap may be created by user via QMP, then reference by other
QMP commands, such as backup. We need reference count machanism to
manage the lifecycle of dirty bitmap.

This adds bdrv_release_dirty_bitmap and changes
bdrv_release_dirty_bitmap to only free the structure when refcnt goes to
0.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               | 17 ++++++++++++++---
 include/block/block.h |  1 +
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 64e7d22..6ad0368 100644
--- a/block.c
+++ b/block.c
@@ -51,6 +51,7 @@
 
 struct BdrvDirtyBitmap {
     HBitmap *bitmap;
+    int refcnt;
     QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
@@ -4543,6 +4544,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity)
     bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS);
     bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
     bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+    bitmap->refcnt = 1;
     QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
     return bitmap;
 }
@@ -4552,14 +4554,23 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
     BdrvDirtyBitmap *bm, *next;
     QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) {
         if (bm == bitmap) {
-            QLIST_REMOVE(bitmap, list);
-            hbitmap_free(bitmap->bitmap);
-            g_free(bitmap);
+            assert(bitmap->refcnt > 0);
+            bitmap->refcnt--;
+            if (bitmap->refcnt == 0) {
+                QLIST_REMOVE(bitmap, list);
+                hbitmap_free(bitmap->bitmap);
+                g_free(bitmap);
+            }
             return;
         }
     }
 }
 
+void bdrv_reference_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+    bitmap->refcnt++;
+}
+
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 {
     BdrvDirtyBitmap *bm;
diff --git a/include/block/block.h b/include/block/block.h
index 36efaea..0c776e3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -426,6 +426,7 @@ struct HBitmapIter;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+void bdrv_reference_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
 void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
-- 
1.8.5.2

[Qemu-devel] [PATCH 2/9] qapi: Add optional field "name" to block dirty bitmap

[Fam Zheng]

This field will be set for user created dirty bitmap. Also pass in an
error pointer to bdrv_create_dirty_bitmap, so when a name is already
taken on this BDS, it can report an error message. This is not global
check, two BDSes can have dirty bitmap with a common name.

Implemented bdrv_find_dirty_bitmap to find a dirty bitmap by name, will
be used later when other QMP commands want to reference dirty bitmap by
name.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block-migration.c     |  3 ++-
 block.c               | 29 ++++++++++++++++++++++++++++-
 block/mirror.c        |  2 +-
 include/block/block.h |  7 ++++++-
 qapi-schema.json      |  4 +++-
 5 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 897fdba..e6e016a 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -315,7 +315,8 @@ static void set_dirty_tracking(void)
     BlkMigDevState *bmds;
 
     QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
-        bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE);
+        bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE,
+                                                      NULL, NULL);
     }
 }
 
diff --git a/block.c b/block.c
index 6ad0368..16ef61b 100644
--- a/block.c
+++ b/block.c
@@ -52,6 +52,7 @@
 struct BdrvDirtyBitmap {
     HBitmap *bitmap;
     int refcnt;
+    char name[1024];
     QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
@@ -4532,19 +4533,43 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
     return true;
 }
 
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity)
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
+                                        const char *name)
+{
+    BdrvDirtyBitmap *bm;
+    QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
+        if (!strcmp(name, bm->name)) {
+            return bm;
+        }
+    }
+    return NULL;
+}
+
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+                                          int granularity,
+                                          const char *name,
+                                          Error **errp)
 {
     int64_t bitmap_size;
     BdrvDirtyBitmap *bitmap;
 
     assert((granularity & (granularity - 1)) == 0);
 
+    if (name && bdrv_find_dirty_bitmap(bs, name)) {
+        if (errp) {
+            error_setg(errp, "Bitmap already exists: %s", name);
+        }
+        return NULL;
+    }
     granularity >>= BDRV_SECTOR_BITS;
     assert(granularity);
     bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS);
     bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
     bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
     bitmap->refcnt = 1;
+    if (name) {
+        pstrcpy(bitmap->name, sizeof(bitmap->name), name);
+    }
     QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
     return bitmap;
 }
@@ -4583,6 +4608,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
         info->count = bdrv_get_dirty_count(bs, bm);
         info->granularity =
             ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+        info->has_name = bm->name[0] != '\0';
+        info->name = g_strdup(bm->name);
         entry->value = info;
         *plist = entry;
         plist = &entry->next;
diff --git a/block/mirror.c b/block/mirror.c
index 2932bab..cc0665b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -598,7 +598,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
     s->granularity = granularity;
     s->buf_size = MAX(buf_size, granularity);
 
-    s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity);
+    s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
     bdrv_set_enable_write_cache(s->target, true);
     bdrv_set_on_error(s->target, on_target_error, on_target_error);
     bdrv_iostatus_enable(s->target);
diff --git a/include/block/block.h b/include/block/block.h
index 0c776e3..8dafa42 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -424,7 +424,12 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
 
 struct HBitmapIter;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity);
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+                                          int granularity,
+                                          const char *name,
+                                          Error **errp);
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
+                                        const char *name);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 void bdrv_reference_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
diff --git a/qapi-schema.json b/qapi-schema.json
index b9b051c..e91143a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -920,6 +920,8 @@
 #
 # Block dirty bitmap information.
 #
+# @name: the name of dirty bitmap
+#
 # @count: number of dirty bytes according to the dirty bitmap
 #
 # @granularity: granularity of the dirty bitmap in bytes (since 1.4)
@@ -927,7 +929,7 @@
 # Since: 1.3
 ##
 { 'type': 'BlockDirtyInfo',
-  'data': {'count': 'int', 'granularity': 'int'} }
+  'data': {'*name': 'str', 'count': 'int', 'granularity': 'int'} }
 
 ##
 # @BlockInfo:
-- 
1.8.5.2

[Qemu-devel] [PATCH 3/9] block: Add bdrv_dirty_bitmap_make_anon

[Fam Zheng]

This will unset the name of dirty bitmap.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               | 5 +++++
 include/block/block.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/block.c b/block.c
index 16ef61b..f7e6851 100644
--- a/block.c
+++ b/block.c
@@ -4545,6 +4545,11 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
     return NULL;
 }
 
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+    bitmap->name[0] = '\0';
+}
+
 BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
                                           int granularity,
                                           const char *name,
diff --git a/include/block/block.h b/include/block/block.h
index 8dafa42..6c88f7a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -430,6 +430,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
                                           Error **errp);
 BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
                                         const char *name);
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 void bdrv_reference_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
-- 
1.8.5.2

[Qemu-devel] [PATCH 4/9] qmp: Add dirty-bitmap-add and dirty-bitmap-remove

[Fam Zheng]

The new command pair is added to manage user created dirty bitmap. The
dirty bitmap's name is mandatory and must be unique for the same device,
but different devices can have bitmaps with the same names.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockdev.c       | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json | 45 ++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx  | 49 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 154 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 2c3242b..dd0f2ac 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1633,6 +1633,66 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
     }
 }
 
+void qmp_dirty_bitmap_add(const char *device, const char *name,
+                          bool has_granularity, int64_t granularity,
+                          Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    if (!name || name[0] == '\0') {
+        error_setg(errp, "Bitmap name cannot be empty");
+        return;
+    }
+    if (has_granularity) {
+        if (granularity & (granularity - 1)) {
+            error_setg(errp, "Granularity must be power of 2");
+            return;
+        }
+    } else {
+        granularity = 65536;
+    }
+
+    bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
+    if (!bitmap) {
+        return;
+    }
+}
+
+void qmp_dirty_bitmap_remove(const char *device, const char *name,
+                             Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    if (!name || name[0] == '\0') {
+        error_setg(errp, "Bitmap name cannot be empty");
+        return;
+    }
+    bitmap = bdrv_find_dirty_bitmap(bs, name);
+    if (!bitmap) {
+        error_setg(errp, "Dirty bitmap not found: %s", name);
+        return;
+    }
+
+    /* Make it invisible to user in case the following
+     * bdrv_release_dirty_bitmap doens't free it because of refcnt */
+    bdrv_dirty_bitmap_make_anon(bs, bitmap);
+    bdrv_release_dirty_bitmap(bs, bitmap);
+}
+
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     const char *id = qdict_get_str(qdict, "id");
diff --git a/qapi-schema.json b/qapi-schema.json
index e91143a..095d6c0 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2062,6 +2062,51 @@
             '*on-target-error': 'BlockdevOnError' } }
 
 ##
+# @DirtyBitmap
+#
+# @device: name of device which the bitmap is tracking
+#
+# @name: name of the dirty bitmap
+#
+# @granularity: #optional the bitmap granularity, default is 64k for
+#               dirty-bitmap-add
+#
+# Since 2.0
+##
+{ 'type': 'DirtyBitmap',
+  'data': { 'device': 'str', 'name': 'str', '*granularity': 'int' } }
+
+##
+# @dirty-bitmap-add
+#
+# Create a dirty bitmap with a name on the device
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If @name is already taken, GenericError with an explaining message
+#
+# Since 2.0
+##
+{'command': 'dirty-bitmap-add',
+  'data': 'DirtyBitmap' }
+
+##
+# @dirty-bitmap-remove
+#
+# Remove a dirty bitmap on the device
+#
+# Setting granularity has no effect here.
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If @name is not found, GenericError with an explaining message
+#
+# Since 2.0
+##
+{'command': 'dirty-bitmap-remove',
+  'data': { 'device': 'str', 'name': 'str' } }
+
+##
 # @migrate_cancel
 #
 # Cancel the current executing migration process.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fba15cd..fbbf29c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1064,6 +1064,55 @@ Example:
 EQMP
 
     {
+        .name       = "dirty-bitmap-add",
+        .args_type  = "device:B,name:s,granularity:i?",
+        .mhandler.cmd_new = qmp_marshal_input_dirty_bitmap_add,
+    },
+    {
+        .name       = "dirty-bitmap-remove",
+        .args_type  = "device:B,name:s",
+        .mhandler.cmd_new = qmp_marshal_input_dirty_bitmap_remove,
+    },
+
+SQMP
+
+dirty-bitmap-add
+----------------
+
+Create a dirty bitmap with a name on the device, and start tracking the writes.
+
+Arguments:
+
+- "device": device name to create dirty bitmap (json-string)
+- "name": name of the new dirty bitmap (json-string)
+- "granularity": granularity to track writes with. (int)
+
+Example:
+
+-> { "execute": "dirty-bitmap-add", "arguments": { "device": "drive0",
+                                                   "name": "bitmap0" } }
+<- { "return": {} }
+
+dirty-bitmap-remove
+----------------
+
+Stop write tracking and remove the dirty bitmap that was created with
+dirty-bitmap-add.
+
+Arguments:
+
+- "device": device name to remove dirty bitmap (json-string)
+- "name": name of the dirty bitmap to remove (json-string)
+
+Example:
+
+-> { "execute": "dirty-bitmap-remove", "arguments": { "device": "drive0",
+                                                      "name": "bitmap0" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "blockdev-snapshot-sync",
         .args_type  = "device:B,snapshot-file:s,format:s?,mode:s?",
         .mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_sync,
-- 
1.8.5.2

[Qemu-devel] [PATCH 5/9] block: Handle error of bdrv_getlength in bdrv_create_dirty_bitmap

[Fam Zheng]

bdrv_getlength could fail, check the return value before using it.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index f7e6851..cc9c530 100644
--- a/block.c
+++ b/block.c
@@ -4568,7 +4568,12 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
     }
     granularity >>= BDRV_SECTOR_BITS;
     assert(granularity);
-    bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS);
+    bitmap_size = bdrv_getlength(bs);
+    if (bitmap_size < 0) {
+        error_setg(errp, "could not get length of device");
+        return NULL;
+    }
+    bitmap_size >>= BDRV_SECTOR_BITS;
     bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
     bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
     bitmap->refcnt = 1;
-- 
1.8.5.2

[Qemu-devel] [PATCH 6/9] block: Introduce bdrv_dirty_bitmap_granularity()

[Fam Zheng]

This returns the granularity (in sectors) of dirty bitmap.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               | 6 ++++++
 include/block/block.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/block.c b/block.c
index cc9c530..0e59a9a 100644
--- a/block.c
+++ b/block.c
@@ -4637,6 +4637,12 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector
     }
 }
 
+int bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
+                                  BdrvDirtyBitmap *bitmap)
+{
+    return hbitmap_granularity(bitmap->bitmap);
+}
+
 void bdrv_dirty_iter_init(BlockDriverState *bs,
                           BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
 {
diff --git a/include/block/block.h b/include/block/block.h
index 6c88f7a..858baad 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -434,6 +434,8 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 void bdrv_reference_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
+int bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
+                                  BdrvDirtyBitmap *bitmap);
 int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
 void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
 void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
-- 
1.8.5.2

[Qemu-devel] [PATCH 7/9] block: Add support of "dirty-bitmap" sync mode

[Fam Zheng]

For "dirty-bitmap" sync mode, the block job will iterate through the
given dirty bitmap to decide if a sector needs backup (backup all the
dirty clusters and skip clean ones), just as allocation conditions of
"top" sync mode.

If the dirty bitmap is updated (because of guest writes) while the block
job is scanning it, an extra time of copy of written sectors could
happen because of overlapping of write interception and dirty bitmap.

User should build a transaction to:
 - Deactive the dirty bitmap.
 - Start a backup job with the dirty bitmap.
 - Create another dirty bitmap for continuity of incremental tracking.

With coming dirty bitmap transaction patches.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/backup.c            | 34 +++++++++++++++++++++++++++++++++-
 block/mirror.c            |  4 ++++
 blockdev.c                |  6 +++++-
 hmp.c                     |  3 ++-
 include/block/block_int.h |  2 ++
 qapi-schema.json          | 10 ++++++----
 qmp-commands.hx           |  7 ++++---
 7 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 0198514..78ca9b1 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -37,6 +37,8 @@ typedef struct CowRequest {
 typedef struct BackupBlockJob {
     BlockJob common;
     BlockDriverState *target;
+    BdrvDirtyBitmap *sync_bitmap;
+    int sync_bitmap_gran;
     MirrorSyncMode sync_mode;
     RateLimit limit;
     BlockdevOnError on_source_error;
@@ -258,7 +260,7 @@ static void coroutine_fn backup_run(void *opaque)
             job->common.busy = true;
         }
     } else {
-        /* Both FULL and TOP SYNC_MODE's require copying.. */
+        /* FULL, TOP and DIRTY_BITMAP SYNC_MODE's require copying.. */
         for (; start < end; start++) {
             bool error_is_read;
 
@@ -312,7 +314,21 @@ static void coroutine_fn backup_run(void *opaque)
                 if (alloced == 0) {
                     continue;
                 }
+            } else if (job->sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+                int i, dirty = 0;
+                for (i = 0; i < BACKUP_SECTORS_PER_CLUSTER;
+                     i += job->sync_bitmap_gran) {
+                    if (bdrv_get_dirty(bs, job->sync_bitmap,
+                            start * BACKUP_SECTORS_PER_CLUSTER + i)) {
+                        dirty = 1;
+                        break;
+                    }
+                }
+                if (!dirty) {
+                    continue;
+                }
             }
+
             /* FULL sync mode we copy the whole drive. */
             ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER,
                     BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
@@ -336,6 +352,9 @@ static void coroutine_fn backup_run(void *opaque)
     qemu_co_rwlock_wrlock(&job->flush_rwlock);
     qemu_co_rwlock_unlock(&job->flush_rwlock);
 
+    if (job->sync_bitmap) {
+        bdrv_release_dirty_bitmap(bs, job->sync_bitmap);
+    }
     hbitmap_free(job->bitmap);
 
     bdrv_iostatus_disable(target);
@@ -346,6 +365,7 @@ static void coroutine_fn backup_run(void *opaque)
 
 void backup_start(BlockDriverState *bs, BlockDriverState *target,
                   int64_t speed, MirrorSyncMode sync_mode,
+                  BdrvDirtyBitmap *sync_bitmap,
                   BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
                   BlockDriverCompletionFunc *cb, void *opaque,
@@ -364,6 +384,16 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
         return;
     }
 
+    if (sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP && !sync_bitmap) {
+        error_setg(errp, "must provide a valid bitmap name for \"dirty-bitmap\""
+                         "sync mode");
+        return;
+    }
+
+    if (sync_bitmap) {
+        bdrv_reference_dirty_bitmap(bs, sync_bitmap);
+    }
+
     len = bdrv_getlength(bs);
     if (len < 0) {
         error_setg_errno(errp, -len, "unable to get length for '%s'",
@@ -381,6 +411,8 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
     job->on_target_error = on_target_error;
     job->target = target;
     job->sync_mode = sync_mode;
+    job->sync_bitmap = sync_bitmap;
+    job->sync_bitmap_gran = bdrv_dirty_bitmap_granularity(bs, job->sync_bitmap);
     job->common.len = len;
     job->common.co = qemu_coroutine_create(backup_run);
     qemu_coroutine_enter(job->common.co, job);
diff --git a/block/mirror.c b/block/mirror.c
index cc0665b..ccab15a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -617,6 +617,10 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
     bool is_none_mode;
     BlockDriverState *base;
 
+    if (mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+        error_setg(errp, "Sync mode 'dirty-bitmap' not supported");
+        return;
+    }
     is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
     base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
     mirror_start_job(bs, target, speed, granularity, buf_size,
diff --git a/blockdev.c b/blockdev.c
index dd0f2ac..ac00562 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1309,6 +1309,7 @@ static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
                      backup->sync,
                      backup->has_mode, backup->mode,
                      backup->has_speed, backup->speed,
+                     backup->has_bitmap, backup->bitmap,
                      backup->has_on_source_error, backup->on_source_error,
                      backup->has_on_target_error, backup->on_target_error,
                      &local_err);
@@ -1898,6 +1899,7 @@ void qmp_drive_backup(const char *device, const char *target,
                       enum MirrorSyncMode sync,
                       bool has_mode, enum NewImageMode mode,
                       bool has_speed, int64_t speed,
+                      bool has_bitmap, const char *bitmap,
                       bool has_on_source_error, BlockdevOnError on_source_error,
                       bool has_on_target_error, BlockdevOnError on_target_error,
                       Error **errp)
@@ -1996,7 +1998,9 @@ void qmp_drive_backup(const char *device, const char *target,
         return;
     }
 
-    backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
+    backup_start(bs, target_bs, speed, sync,
+                 has_bitmap ? bdrv_find_dirty_bitmap(bs, bitmap) : NULL,
+                 on_source_error, on_target_error,
                  block_job_cb, bs, &local_err);
     if (local_err != NULL) {
         bdrv_unref(target_bs);
diff --git a/hmp.c b/hmp.c
index 32ee285..f3b16eb 100644
--- a/hmp.c
+++ b/hmp.c
@@ -949,7 +949,8 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
 
     qmp_drive_backup(device, filename, !!format, format,
                      full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
-                     true, mode, false, 0, false, 0, false, 0, &errp);
+                     true, mode, false, 0, false, NULL,
+                     false, 0, false, 0, &errp);
     hmp_handle_error(mon, &errp);
 }
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2772f2f..9dea8f5 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -456,6 +456,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
  * @target: Block device to write to.
  * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
  * @sync_mode: What parts of the disk image should be copied to the destination.
+ * @sync_bitmap: The dirty bitmap if sync_mode is MIRROR_SYNC_MODE_DIRTY_BITMAP.
  * @on_source_error: The action to take upon error reading from the source.
  * @on_target_error: The action to take upon error writing to the target.
  * @cb: Completion function for the job.
@@ -466,6 +467,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
  */
 void backup_start(BlockDriverState *bs, BlockDriverState *target,
                   int64_t speed, MirrorSyncMode sync_mode,
+                  BdrvDirtyBitmap *sync_bitmap,
                   BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
                   BlockDriverCompletionFunc *cb, void *opaque,
diff --git a/qapi-schema.json b/qapi-schema.json
index 095d6c0..eef894c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1421,7 +1421,7 @@
 # Since: 1.3
 ##
 { 'enum': 'MirrorSyncMode',
-  'data': ['top', 'full', 'none'] }
+  'data': ['top', 'full', 'none', 'dirty-bitmap'] }
 
 ##
 # @BlockJobType:
@@ -1791,14 +1791,16 @@
 #          probe if @mode is 'existing', else the format of the source
 #
 # @sync: what parts of the disk image should be copied to the destination
-#        (all the disk, only the sectors allocated in the topmost image, or
-#        only new I/O).
+#        (all the disk, only the sectors allocated in the topmost image, from a
+#        dirty bitmap, or only new I/O).
 #
 # @mode: #optional whether and how QEMU should create a new image, default is
 #        'absolute-paths'.
 #
 # @speed: #optional the maximum speed, in bytes per second
 #
+# @bitmap: #optional the name of dirty bitmap if sync is "dirty-bitmap"
+#
 # @on-source-error: #optional the action to take on an error on the source,
 #                   default 'report'.  'stop' and 'enospc' can only be used
 #                   if the block device supports io-status (see BlockInfo).
@@ -1816,7 +1818,7 @@
 { 'type': 'DriveBackup',
   'data': { 'device': 'str', 'target': 'str', '*format': 'str',
             'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
-            '*speed': 'int',
+            '*speed': 'int', '*bitmap': 'str',
             '*on-source-error': 'BlockdevOnError',
             '*on-target-error': 'BlockdevOnError' } }
 
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fbbf29c..eaae70e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -917,7 +917,7 @@ EQMP
     {
         .name       = "drive-backup",
         .args_type  = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
-                      "on-source-error:s?,on-target-error:s?",
+                      "bitmap:s?,on-source-error:s?,on-target-error:s?",
         .mhandler.cmd_new = qmp_marshal_input_drive_backup,
     },
 
@@ -944,8 +944,9 @@ Arguments:
             (json-string, optional)
 - "sync": what parts of the disk image should be copied to the destination;
   possibilities include "full" for all the disk, "top" for only the sectors
-  allocated in the topmost image, or "none" to only replicate new I/O
-  (MirrorSyncMode).
+  allocated in the topmost image, "dirty-bitmap" for only sync the dirty
+  sectors in the bitmap, or "none" to only replicate new I/O (MirrorSyncMode).
+- "bitmap": bitmap name to use for sync==dirty-bitmap
 - "mode": whether and how QEMU should create a new image
           (NewImageMode, optional, default 'absolute-paths')
 - "speed": the maximum speed, in bytes per second (json-int, optional)
-- 
1.8.5.2

[Qemu-devel] [PATCH 8/9] qmp: Add dirty-bitmap-disable command

[Fam Zheng]

This will put the dirty bitmap into a disabled state and no more
writes will be tracked.

It will be used before backup or writing to persistent file.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               | 15 +++++++++++++++
 blockdev.c            | 22 ++++++++++++++++++++++
 include/block/block.h |  2 ++
 qapi-schema.json      | 16 ++++++++++++++++
 qmp-commands.hx       |  5 +++++
 5 files changed, 60 insertions(+)

diff --git a/block.c b/block.c
index 0e59a9a..62efb93 100644
--- a/block.c
+++ b/block.c
@@ -53,6 +53,7 @@ struct BdrvDirtyBitmap {
     HBitmap *bitmap;
     int refcnt;
     char name[1024];
+    bool enabled;
     QLIST_ENTRY(BdrvDirtyBitmap) list;
 };
 
@@ -4580,6 +4581,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
     if (name) {
         pstrcpy(bitmap->name, sizeof(bitmap->name), name);
     }
+    bitmap->enabled = true;
     QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
     return bitmap;
 }
@@ -4606,6 +4608,16 @@ void bdrv_reference_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
     bitmap->refcnt++;
 }
 
+void bdrv_disable_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+    bitmap->enabled = false;
+}
+
+void bdrv_enable_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+    bitmap->enabled = true;
+}
+
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 {
     BdrvDirtyBitmap *bm;
@@ -4654,6 +4666,9 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
 {
     BdrvDirtyBitmap *bitmap;
     QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+        if (!bitmap->enabled) {
+            continue;
+        }
         hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
     }
 }
diff --git a/blockdev.c b/blockdev.c
index ac00562..090a681 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1694,6 +1694,28 @@ void qmp_dirty_bitmap_remove(const char *device, const char *name,
     bdrv_release_dirty_bitmap(bs, bitmap);
 }
 
+void qmp_dirty_bitmap_disable(const char *device, const char *name,
+                              bool has_granularity, int64_t granularity,
+                              Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    bitmap = bdrv_find_dirty_bitmap(bs, name);
+    if (!bitmap) {
+        error_setg(errp, "Dirty bitmap not found: %s", name);
+        return;
+    }
+
+    bdrv_disable_dirty_bitmap(bs, bitmap);
+}
+
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     const char *id = qdict_get_str(qdict, "id");
diff --git a/include/block/block.h b/include/block/block.h
index 858baad..8b9b142 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -433,6 +433,8 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
 void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 void bdrv_reference_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+void bdrv_disable_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+void bdrv_enable_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
 int bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
                                   BdrvDirtyBitmap *bitmap);
diff --git a/qapi-schema.json b/qapi-schema.json
index eef894c..8a81026 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2109,6 +2109,22 @@
   'data': { 'device': 'str', 'name': 'str' } }
 
 ##
+# @dirty-bitmap-disable
+#
+# Disable a dirty bitmap on the device
+#
+# Setting granularity has no effect here.
+#
+# Returns: nothing on success
+#          If @device is not a valid block device, DeviceNotFound
+#          If @name is not found, GenericError with an explaining message
+#
+# Since 2.0
+##
+{'command': 'dirty-bitmap-disable',
+  'data': 'DirtyBitmap' }
+
+##
 # @migrate_cancel
 #
 # Cancel the current executing migration process.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index eaae70e..88b5f58 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1074,6 +1074,11 @@ EQMP
         .args_type  = "device:B,name:s",
         .mhandler.cmd_new = qmp_marshal_input_dirty_bitmap_remove,
     },
+    {
+        .name       = "dirty-bitmap-disable",
+        .args_type  = "device:B,name:s",
+        .mhandler.cmd_new = qmp_marshal_input_dirty_bitmap_disable,
+    },
 
 SQMP
 
-- 
1.8.5.2

[Qemu-devel] [PATCH 9/9] qapi: Add transaction support to dirty-bitmap-{add, disable}

[Fam Zheng]

This adds dirty-bitmap-add and dirty-bitmap-disable to transactions.
With this, user can stop a dirty bitmap, start backup of it, and start
another dirty bitmap atomically, so that the dirty bitmap is tracked
incrementally and we don't miss any write.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockdev.c       | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |  4 +++-
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 090a681..3a8fbf2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1335,6 +1335,64 @@ static void drive_backup_abort(BlkTransactionState *common)
     }
 }
 
+static void dirty_bitmap_add_prepare(BlkTransactionState *common, Error **errp)
+{
+    DirtyBitmap *action;
+    Error *local_err = NULL;
+
+    action = common->action->dirty_bitmap_add;
+    qmp_dirty_bitmap_add(action->device, action->name, false, 0, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
+}
+
+static void dirty_bitmap_add_abort(BlkTransactionState *common)
+{
+    DirtyBitmap *action;
+    BdrvDirtyBitmap *bm;
+    BlockDriverState *bs;
+
+    action = common->action->dirty_bitmap_add;
+    bs = bdrv_find(action->device);
+    if (bs) {
+        bm = bdrv_find_dirty_bitmap(bs, action->name);
+        if (bm) {
+            bdrv_release_dirty_bitmap(bs, bm);
+        }
+    }
+}
+
+static void dirty_bitmap_disable_prepare(BlkTransactionState *common,
+                                         Error **errp)
+{
+    DirtyBitmap *action;
+    Error *local_err = NULL;
+
+    action = common->action->dirty_bitmap_disable;
+    qmp_dirty_bitmap_disable(action->device, action->name,
+                             false, 0, &local_err);
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
+}
+
+static void dirty_bitmap_disable_abort(BlkTransactionState *common)
+{
+    DirtyBitmap *action;
+    BdrvDirtyBitmap *bitmap;
+    BlockDriverState *bs;
+
+    action = common->action->dirty_bitmap_disable;
+    bs = bdrv_find(action->device);
+    if (bs) {
+        bitmap = bdrv_find_dirty_bitmap(bs, action->name);
+        if (bitmap) {
+            bdrv_enable_dirty_bitmap(bs, bitmap);
+        }
+    }
+}
+
 static void abort_prepare(BlkTransactionState *common, Error **errp)
 {
     error_setg(errp, "Transaction aborted using Abort action");
@@ -1367,6 +1425,16 @@ static const BdrvActionOps actions[] = {
         .prepare  = internal_snapshot_prepare,
         .abort = internal_snapshot_abort,
     },
+    [TRANSACTION_ACTION_KIND_DIRTY_BITMAP_ADD] = {
+        .instance_size = sizeof(BlkTransactionState),
+        .prepare = dirty_bitmap_add_prepare,
+        .abort = dirty_bitmap_add_abort,
+    },
+    [TRANSACTION_ACTION_KIND_DIRTY_BITMAP_DISABLE] = {
+        .instance_size = sizeof(BlkTransactionState),
+        .prepare = dirty_bitmap_disable_prepare,
+        .abort = dirty_bitmap_disable_abort,
+    },
 };
 
 /*
diff --git a/qapi-schema.json b/qapi-schema.json
index 8a81026..22cf010 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1843,7 +1843,9 @@
        'blockdev-snapshot-sync': 'BlockdevSnapshot',
        'drive-backup': 'DriveBackup',
        'abort': 'Abort',
-       'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
+       'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
+       'dirty-bitmap-add': 'DirtyBitmap',
+       'dirty-bitmap-disable': 'DirtyBitmap'
    } }
 
 ##
-- 
1.8.5.2

Re: [Qemu-devel] [PATCH 0/9] QMP: Introduce incremental drive-backup with in-memory dirty bitmap

[Stefan Hajnoczi]

On Mon, Jan 13, 2014 at 06:39:39PM +0800, Fam Zheng wrote:
> This implements incremental backup.
> 
> A few new QMP commands related to dirty bitmap are added:
> 
>     dirty-bitmap-add *
>     dirty-bitmap-disable *
>     dirty-bitmap-remove
> 
>     (*: also supported as transactions)
> 
> As their name implies, they manipulate a block device's dirty bitmap. This
> doesn't interfere with dirty bitmap used for migration, backup, mirror, etc,
> which don't have a name and are invisible to user. Only named bitmaps (created
> by dirty-bitmap-add) can be disabled/removed by user.
> 
> They are added to support "user controlled write tracking", so as to determine
> the range of date for incremental backup.
> 
> A new sync mode for drive-backup is introduced:
> 
>     drive-backup device=.. mode=.. sync=dirty-bitmap bitmap=bitmap0
> 
> Which will scan dirty bitmap "bitmap0" and only copy all dirty sectors to
> target.
> 
> Now, let's see the usage with a simple example:
> 
>     # Start the guest
>     vm = VM()
>     vm.start()
> 
>     # Fake some guest writes with "qemu-io", this is before creating dirty
>     # bitmap, so it won't be copied
>     vm.hmp('qemu-io ide0-hd0 "write -P 0xa 512k 1M"')
> 
>     # Create a dirty bitmap to track writes
>     vm.qmp("dirty-bitmap-add", device="ide0-hd0", name="dirty-0")
> 
>     # Fake some more guest writes with "qemu-io", this will be copied
>     vm.hmp('qemu-io ide0-hd0 "write -P 0xa 512M 1M"')
> 
>     # Now "disable" the first dirty bitmap, do the backup according to it,
>     # at meantime continue to track dirty with a new dirty bitmap
>     vm.qmp("transaction", actions=[
>         {
>             'type': 'dirty-bitmap-disable', 'data': {
>                 'device': 'ide0-hd0',
>                 'name': 'dirty-0'
>             }
>         }, {
>             'type': 'dirty-bitmap-add', 'data': {
>                 'device': 'ide0-hd0',
>                 'name': 'dirty-1'
>                 }
>         }, {
>             'type': 'drive-backup', 'data': {
>                 'device': 'ide0-hd0',
>                 'target': '/tmp/incre.qcow2',
>                 'bitmap': 'dirty-0',
>                 'sync': 'dirty-bitmap'
>                 }
>             }
>         ])
> 
>     # Once backup job started, the dirty bitmap can be removed (actually only
>     # hidden from user since it is still referenced by block job
>     vm.qmp("dirty-bitmap-remove", device="ide0-hd0", name="dirty-0")

I'm interested in the lifecycle of a dirty bitmap (but haven't reviewed
the patches yet).  In particular, what happens if a bitmap is added to
more than one drive?  Is there a more elegant way to handle the disable,
drive-backup, remove step (we only need to explicitly disable because we
still need the bitmap name for the drive-backup command)?  Also what
happens if we add the bitmap again after disabling?

No need to answer all these questions, but it suggests the interface
exposes a bit of complexity.  Maybe it's possible to make it simpler and
easier to use?

> P.S. Persistent dirty bitmap could be built on top of this series, but is not
> yet implemented, because the storage format and integrity measures are not
> quite clear for now. The discussion is still open and any questions, ideas, use
> cases and concerns are all welcome!

It's desirable to keep dirty bitmaps in separate files.  That way they
can be used with any image format (raw, qcow2, etc).

For performance, the dirty bitmap is maintained in memory.  Keeping the
persistent dirty bitmap consistent would be very expensive since it
requires an fdatasync(2) before each write request.

I think it's reasonable to only write out the in-memory bitmap on
shutdown or live migration.  If the QEMU process crashes it is not safe
to trust the dirty bitmap; a full backup must be performed.

The persistent bitmap file must contain:
1. Bitmap granularity (e.g. 64 KB)
2. The actual bitmap (1 TB disk @ 64 KB granularity = 2 MB bitmap)
3. Flags including a "clean" bit

When QEMU activates the persistent dirty bitmap, it clears the "clean"
flag.  When QEMU deactivates and finishes writing out the dirty bitmap,
it sets the "clean" flag.

The "clean" flag is used to tell whether the persistent bitmap file is
safe to use again.

The file contains no information about the VM or disk image.  It's up to
the user or management tool to keep track of files.

Re: [Qemu-devel] [PATCH 2/9] qapi: Add optional field "name" to block dirty bitmap

[Stefan Hajnoczi]

On Mon, Jan 13, 2014 at 06:39:41PM +0800, Fam Zheng wrote:
> diff --git a/block.c b/block.c
> index 6ad0368..16ef61b 100644
> --- a/block.c
> +++ b/block.c
> @@ -52,6 +52,7 @@
>  struct BdrvDirtyBitmap {
>      HBitmap *bitmap;
>      int refcnt;
> +    char name[1024];

Is there a reason for a fixed-size buffer?  g_strdup() is nicer than
arbitrary limits, especially since the lifecycle of BdrvDirtyBitmap is
well-defined and you just need to add a g_free().

> +BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
> +                                          int granularity,
> +                                          const char *name,
> +                                          Error **errp)
>  {
>      int64_t bitmap_size;
>      BdrvDirtyBitmap *bitmap;
>  
>      assert((granularity & (granularity - 1)) == 0);
>  
> +    if (name && bdrv_find_dirty_bitmap(bs, name)) {
> +        if (errp) {
> +            error_setg(errp, "Bitmap already exists: %s", name);
> +        }

error_setg() already checks that errp != NULL, you can drop the if
statement.

> diff --git a/block/mirror.c b/block/mirror.c
> index 2932bab..cc0665b 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -598,7 +598,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
>      s->granularity = granularity;
>      s->buf_size = MAX(buf_size, granularity);
>  
> -    s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity);
> +    s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);

No error return means the coroutine will be started even if creating the
dirty bitmap fails.  I didn't check what happens but this definitely
makes the reader wonder if the error will be handled cleanly.

> diff --git a/qapi-schema.json b/qapi-schema.json
> index b9b051c..e91143a 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -920,6 +920,8 @@
>  #
>  # Block dirty bitmap information.
>  #
> +# @name: the name of dirty bitmap

Since 2.0

Re: [Qemu-devel] [PATCH 0/9] QMP: Introduce incremental drive-backup with in-memory dirty bitmap

[Fam Zheng]

On Fri, 01/17 17:25, Stefan Hajnoczi wrote:
> On Mon, Jan 13, 2014 at 06:39:39PM +0800, Fam Zheng wrote:
> > This implements incremental backup.
> > 
> > A few new QMP commands related to dirty bitmap are added:
> > 
> >     dirty-bitmap-add *
> >     dirty-bitmap-disable *
> >     dirty-bitmap-remove
> > 
> >     (*: also supported as transactions)
> > 
> > As their name implies, they manipulate a block device's dirty bitmap. This
> > doesn't interfere with dirty bitmap used for migration, backup, mirror, etc,
> > which don't have a name and are invisible to user. Only named bitmaps (created
> > by dirty-bitmap-add) can be disabled/removed by user.
> > 
> > They are added to support "user controlled write tracking", so as to determine
> > the range of date for incremental backup.
> > 
> > A new sync mode for drive-backup is introduced:
> > 
> >     drive-backup device=.. mode=.. sync=dirty-bitmap bitmap=bitmap0
> > 
> > Which will scan dirty bitmap "bitmap0" and only copy all dirty sectors to
> > target.
> > 
> > Now, let's see the usage with a simple example:
> > 
> >     # Start the guest
> >     vm = VM()
> >     vm.start()
> > 
> >     # Fake some guest writes with "qemu-io", this is before creating dirty
> >     # bitmap, so it won't be copied
> >     vm.hmp('qemu-io ide0-hd0 "write -P 0xa 512k 1M"')
> > 
> >     # Create a dirty bitmap to track writes
> >     vm.qmp("dirty-bitmap-add", device="ide0-hd0", name="dirty-0")
> > 
> >     # Fake some more guest writes with "qemu-io", this will be copied
> >     vm.hmp('qemu-io ide0-hd0 "write -P 0xa 512M 1M"')
> > 
> >     # Now "disable" the first dirty bitmap, do the backup according to it,
> >     # at meantime continue to track dirty with a new dirty bitmap
> >     vm.qmp("transaction", actions=[
> >         {
> >             'type': 'dirty-bitmap-disable', 'data': {
> >                 'device': 'ide0-hd0',
> >                 'name': 'dirty-0'
> >             }
> >         }, {
> >             'type': 'dirty-bitmap-add', 'data': {
> >                 'device': 'ide0-hd0',
> >                 'name': 'dirty-1'
> >                 }
> >         }, {
> >             'type': 'drive-backup', 'data': {
> >                 'device': 'ide0-hd0',
> >                 'target': '/tmp/incre.qcow2',
> >                 'bitmap': 'dirty-0',
> >                 'sync': 'dirty-bitmap'
> >                 }
> >             }
> >         ])
> > 
> >     # Once backup job started, the dirty bitmap can be removed (actually only
> >     # hidden from user since it is still referenced by block job
> >     vm.qmp("dirty-bitmap-remove", device="ide0-hd0", name="dirty-0")
> 
> I'm interested in the lifecycle of a dirty bitmap (but haven't reviewed
> the patches yet).  In particular, what happens if a bitmap is added to
> more than one drive?  Is there a more elegant way to handle the disable,
> drive-backup, remove step (we only need to explicitly disable because we
> still need the bitmap name for the drive-backup command)?  Also what
> happens if we add the bitmap again after disabling?

A same name on that device can't be used again unless it's removed. A bitmap is
associated to (and only) one device, it can't be shared.

> 
> No need to answer all these questions, but it suggests the interface
> exposes a bit of complexity.  Maybe it's possible to make it simpler and
> easier to use?
> 

At least the user has to explicitly start tracking, that's the dirty-bitmap-add
step. Alternatively, we can have "disable, drive-backup, remove" step simplified as:

    drive-backup sync=dirty-bitmap bitmap=dirty0 reset-bitmap=true

where backup job copy out the dirty bitmap (and clears it, as reset-bitmap is
true), and backup with it atomically.

Of course it doesn't have to actually copy the whole bitmap: it just makes the
old one anonymous, create a new empty one and give it the same name. When
backup is done, the old bitmap is removed.

What do you think?

Fam

> > P.S. Persistent dirty bitmap could be built on top of this series, but is not
> > yet implemented, because the storage format and integrity measures are not
> > quite clear for now. The discussion is still open and any questions, ideas, use
> > cases and concerns are all welcome!
> 
> It's desirable to keep dirty bitmaps in separate files.  That way they
> can be used with any image format (raw, qcow2, etc).
> 
> For performance, the dirty bitmap is maintained in memory.  Keeping the
> persistent dirty bitmap consistent would be very expensive since it
> requires an fdatasync(2) before each write request.
> 
> I think it's reasonable to only write out the in-memory bitmap on
> shutdown or live migration.  If the QEMU process crashes it is not safe
> to trust the dirty bitmap; a full backup must be performed.
> 
> The persistent bitmap file must contain:
> 1. Bitmap granularity (e.g. 64 KB)
> 2. The actual bitmap (1 TB disk @ 64 KB granularity = 2 MB bitmap)
> 3. Flags including a "clean" bit
> 
> When QEMU activates the persistent dirty bitmap, it clears the "clean"
> flag.  When QEMU deactivates and finishes writing out the dirty bitmap,
> it sets the "clean" flag.
> 
> The "clean" flag is used to tell whether the persistent bitmap file is
> safe to use again.
> 
> The file contains no information about the VM or disk image.  It's up to
> the user or management tool to keep track of files.

Re: [Qemu-devel] [PATCH 0/9] QMP: Introduce incremental drive-backup with in-memory dirty bitmap

[Stefan Hajnoczi]

On Mon, Jan 20, 2014 at 10:55:58AM +0800, Fam Zheng wrote:
> On Fri, 01/17 17:25, Stefan Hajnoczi wrote:
> > On Mon, Jan 13, 2014 at 06:39:39PM +0800, Fam Zheng wrote:
> > > This implements incremental backup.
> > > 
> > > A few new QMP commands related to dirty bitmap are added:
> > > 
> > >     dirty-bitmap-add *
> > >     dirty-bitmap-disable *
> > >     dirty-bitmap-remove
> > > 
> > >     (*: also supported as transactions)
> > > 
> > > As their name implies, they manipulate a block device's dirty bitmap. This
> > > doesn't interfere with dirty bitmap used for migration, backup, mirror, etc,
> > > which don't have a name and are invisible to user. Only named bitmaps (created
> > > by dirty-bitmap-add) can be disabled/removed by user.
> > > 
> > > They are added to support "user controlled write tracking", so as to determine
> > > the range of date for incremental backup.
> > > 
> > > A new sync mode for drive-backup is introduced:
> > > 
> > >     drive-backup device=.. mode=.. sync=dirty-bitmap bitmap=bitmap0
> > > 
> > > Which will scan dirty bitmap "bitmap0" and only copy all dirty sectors to
> > > target.
> > > 
> > > Now, let's see the usage with a simple example:
> > > 
> > >     # Start the guest
> > >     vm = VM()
> > >     vm.start()
> > > 
> > >     # Fake some guest writes with "qemu-io", this is before creating dirty
> > >     # bitmap, so it won't be copied
> > >     vm.hmp('qemu-io ide0-hd0 "write -P 0xa 512k 1M"')
> > > 
> > >     # Create a dirty bitmap to track writes
> > >     vm.qmp("dirty-bitmap-add", device="ide0-hd0", name="dirty-0")
> > > 
> > >     # Fake some more guest writes with "qemu-io", this will be copied
> > >     vm.hmp('qemu-io ide0-hd0 "write -P 0xa 512M 1M"')
> > > 
> > >     # Now "disable" the first dirty bitmap, do the backup according to it,
> > >     # at meantime continue to track dirty with a new dirty bitmap
> > >     vm.qmp("transaction", actions=[
> > >         {
> > >             'type': 'dirty-bitmap-disable', 'data': {
> > >                 'device': 'ide0-hd0',
> > >                 'name': 'dirty-0'
> > >             }
> > >         }, {
> > >             'type': 'dirty-bitmap-add', 'data': {
> > >                 'device': 'ide0-hd0',
> > >                 'name': 'dirty-1'
> > >                 }
> > >         }, {
> > >             'type': 'drive-backup', 'data': {
> > >                 'device': 'ide0-hd0',
> > >                 'target': '/tmp/incre.qcow2',
> > >                 'bitmap': 'dirty-0',
> > >                 'sync': 'dirty-bitmap'
> > >                 }
> > >             }
> > >         ])
> > > 
> > >     # Once backup job started, the dirty bitmap can be removed (actually only
> > >     # hidden from user since it is still referenced by block job
> > >     vm.qmp("dirty-bitmap-remove", device="ide0-hd0", name="dirty-0")
> > 
> > I'm interested in the lifecycle of a dirty bitmap (but haven't reviewed
> > the patches yet).  In particular, what happens if a bitmap is added to
> > more than one drive?  Is there a more elegant way to handle the disable,
> > drive-backup, remove step (we only need to explicitly disable because we
> > still need the bitmap name for the drive-backup command)?  Also what
> > happens if we add the bitmap again after disabling?
> 
> A same name on that device can't be used again unless it's removed. A bitmap is
> associated to (and only) one device, it can't be shared.
> 
> > 
> > No need to answer all these questions, but it suggests the interface
> > exposes a bit of complexity.  Maybe it's possible to make it simpler and
> > easier to use?
> > 
> 
> At least the user has to explicitly start tracking, that's the dirty-bitmap-add
> step. Alternatively, we can have "disable, drive-backup, remove" step simplified as:
> 
>     drive-backup sync=dirty-bitmap bitmap=dirty0 reset-bitmap=true
> 
> where backup job copy out the dirty bitmap (and clears it, as reset-bitmap is
> true), and backup with it atomically.
> 
> Of course it doesn't have to actually copy the whole bitmap: it just makes the
> old one anonymous, create a new empty one and give it the same name. When
> backup is done, the old bitmap is removed.
> 
> What do you think?

That simplifies usage.  I can think of two options:
1. We want to continue incremental backup so drive-backup should
   atomically swap the bitmap with a new one of the same name (as you
   described).
2. We want to end incremental backup so the drive-backup command should
   consume the bitmap and not create a new one.

Then users just need to add a new bitmap and later use drive-backup.
They don't need to manually manage the bitmap's lifecycle.

Stefan