[Qemu-devel] [PATCH v2 0/5] block dirty bitmaps: support libvirt API

[Qemu-devel] [PATCH v2 0/5] block dirty bitmaps: support libvirt API

[John Snow]

This is largely the same series that Vladimir sent in January, but at
the time I was unsure of if we'd want these commands or not in QEMU.

After discussing with Virtuozzo their plans for a checkpoint-like API
implemented primarily in libvirt, I agree that these commands are at
least tentatively useful.

Eric Blake is currently writing a counter-proposal and demo API to show
to Virtuozzo on libvirt's development list. Check in these QMP commands
with the experimental prefix 'x-' for now so that it can be used for
prototyping.

Once the design for the libvirt API looks reasonably final I will
remove the 'x-' prefixes, or, if we wind up not using these particular
commands I will delete them entirely.

v4:
 - Reinstated Vladimir's authorship credit on all patches
 - Switched 2.12 to 3.0
 - Edited doc to clarify failure mode for merge (Jeff)
 - Removed !bs check where applicable (Jeff)

v3:
 - Drop patch one (already merged)
 - Minor phrasing adjustments to documentation
 - Removed &state->bs argument to bitmap lookup for enable/disable
 - Added x- prefix to all three commands and to add's new argument.

Vladimir's original cover letter is below:

There are three qmp commands, needed to implement external backup API.

Using these three commands, client may do all needed bitmap management by
hand:

on backup start we need to do a transaction:
 {disable old bitmap, create new bitmap}

on backup success:
 drop old bitmap

on backup fail:
 enable old bitmap
 merge new bitmap to old bitmap
 drop new bitmap

Vladimir Sementsov-Ogievskiy (5):
  block/dirty-bitmap: add lock to bdrv_enable/disable_dirty_bitmap
  qapi: add x-block-dirty-bitmap-enable/disable
  qmp: transaction support for x-block-dirty-bitmap-enable/disable
  qapi: add x-block-dirty-bitmap-merge
  qapi: add disabled parameter to block-dirty-bitmap-add

 block/dirty-bitmap.c         |  24 ++++++-
 blockdev.c                   | 163 ++++++++++++++++++++++++++++++++++++++++++-
 include/block/dirty-bitmap.h |   3 +-
 qapi/block-core.json         |  86 ++++++++++++++++++++++-
 qapi/transaction.json        |   4 ++
 5 files changed, 275 insertions(+), 5 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v2 1/5] block/dirty-bitmap: add lock to bdrv_enable/disable_dirty_bitmap

[John Snow]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Add locks and remove comments about BQL accordingly to
dirty_bitmap_mutex definition in block_int.h.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
---
 block/dirty-bitmap.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 967159479d..56234257f4 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -442,18 +442,20 @@ void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs,
     }
 }
 
-/* Called with BQL taken.  */
 void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
+    bdrv_dirty_bitmap_lock(bitmap);
     assert(!bdrv_dirty_bitmap_frozen(bitmap));
     bitmap->disabled = true;
+    bdrv_dirty_bitmap_unlock(bitmap);
 }
 
-/* Called with BQL taken.  */
 void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
 {
+    bdrv_dirty_bitmap_lock(bitmap);
     assert(!bdrv_dirty_bitmap_frozen(bitmap));
     bitmap->disabled = false;
+    bdrv_dirty_bitmap_unlock(bitmap);
 }
 
 BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
-- 
2.14.3

[Qemu-devel] [PATCH v2 2/5] qapi: add x-block-dirty-bitmap-enable/disable

[John Snow]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Expose the ability to turn bitmaps "on" or "off". This is experimental
and principally for the sake of the Libvirt Checkpoints API, and it may
or may not be committed for 3.0.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c           | 42 ++++++++++++++++++++++++++++++++++++++++++
 qapi/block-core.json | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 8de95be8f4..fb402edc94 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2923,6 +2923,48 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
     bdrv_clear_dirty_bitmap(bitmap, NULL);
 }
 
+void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
+                                   Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+
+    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
+    if (!bitmap) {
+        return;
+    }
+
+    if (bdrv_dirty_bitmap_frozen(bitmap)) {
+        error_setg(errp,
+                   "Bitmap '%s' is currently frozen and cannot be enabled",
+                   name);
+        return;
+    }
+
+    bdrv_enable_dirty_bitmap(bitmap);
+}
+
+void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
+                                    Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+
+    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
+    if (!bitmap) {
+        return;
+    }
+
+    if (bdrv_dirty_bitmap_frozen(bitmap)) {
+        error_setg(errp,
+                   "Bitmap '%s' is currently frozen and cannot be disabled",
+                   name);
+        return;
+    }
+
+    bdrv_disable_dirty_bitmap(bitmap);
+}
+
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
                                                               const char *name,
                                                               Error **errp)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4b1de474a9..02de674f5f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1808,6 +1808,48 @@
 { 'command': 'block-dirty-bitmap-clear',
   'data': 'BlockDirtyBitmap' }
 
+##
+# @x-block-dirty-bitmap-enable:
+#
+# Enables a dirty bitmap so that it will begin tracking disk changes.
+#
+# Returns: nothing on success
+#          If @node is not a valid block device, DeviceNotFound
+#          If @name is not found, GenericError with an explanation
+#
+# Since: 3.0
+#
+# Example:
+#
+# -> { "execute": "x-block-dirty-bitmap-enable",
+#      "arguments": { "node": "drive0", "name": "bitmap0" } }
+# <- { "return": {} }
+#
+##
+  { 'command': 'x-block-dirty-bitmap-enable',
+    'data': 'BlockDirtyBitmap' }
+
+##
+# @x-block-dirty-bitmap-disable:
+#
+# Disables a dirty bitmap so that it will stop tracking disk changes.
+#
+# Returns: nothing on success
+#          If @node is not a valid block device, DeviceNotFound
+#          If @name is not found, GenericError with an explanation
+#
+# Since: 3.0
+#
+# Example:
+#
+# -> { "execute": "x-block-dirty-bitmap-disable",
+#      "arguments": { "node": "drive0", "name": "bitmap0" } }
+# <- { "return": {} }
+#
+##
+    { 'command': 'x-block-dirty-bitmap-disable',
+      'data': 'BlockDirtyBitmap' }
+
 ##
 # @BlockDirtyBitmapSha256:
 #
-- 
2.14.3

[Qemu-devel] [PATCH v2 3/5] qmp: transaction support for x-block-dirty-bitmap-enable/disable

[John Snow]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
[Added x- prefix. --js]
Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c            | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 qapi/transaction.json |  4 +++
 2 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index fb402edc94..af705738cd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2052,6 +2052,7 @@ typedef struct BlockDirtyBitmapState {
     BlockDriverState *bs;
     HBitmap *backup;
     bool prepared;
+    bool was_enabled;
 } BlockDirtyBitmapState;
 
 static void block_dirty_bitmap_add_prepare(BlkActionState *common,
@@ -2151,6 +2152,74 @@ static void block_dirty_bitmap_clear_commit(BlkActionState *common)
     hbitmap_free(state->backup);
 }
 
+static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
+                                              Error **errp)
+{
+    BlockDirtyBitmap *action;
+    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+                                             common, common);
+
+    if (action_check_completion_mode(common, errp) < 0) {
+        return;
+    }
+
+    action = common->action->u.x_block_dirty_bitmap_enable.data;
+    state->bitmap = block_dirty_bitmap_lookup(action->node,
+                                              action->name,
+                                              NULL,
+                                              errp);
+    if (!state->bitmap) {
+        return;
+    }
+
+    state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
+    bdrv_enable_dirty_bitmap(state->bitmap);
+}
+
+static void block_dirty_bitmap_enable_abort(BlkActionState *common)
+{
+    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+                                             common, common);
+
+    if (!state->was_enabled) {
+        bdrv_disable_dirty_bitmap(state->bitmap);
+    }
+}
+
+static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
+                                               Error **errp)
+{
+    BlockDirtyBitmap *action;
+    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+                                             common, common);
+
+    if (action_check_completion_mode(common, errp) < 0) {
+        return;
+    }
+
+    action = common->action->u.x_block_dirty_bitmap_disable.data;
+    state->bitmap = block_dirty_bitmap_lookup(action->node,
+                                              action->name,
+                                              NULL,
+                                              errp);
+    if (!state->bitmap) {
+        return;
+    }
+
+    state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
+    bdrv_disable_dirty_bitmap(state->bitmap);
+}
+
+static void block_dirty_bitmap_disable_abort(BlkActionState *common)
+{
+    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+                                             common, common);
+
+    if (state->was_enabled) {
+        bdrv_enable_dirty_bitmap(state->bitmap);
+    }
+}
+
 static void abort_prepare(BlkActionState *common, Error **errp)
 {
     error_setg(errp, "Transaction aborted using Abort action");
@@ -2211,7 +2280,17 @@ static const BlkActionOps actions[] = {
         .prepare = block_dirty_bitmap_clear_prepare,
         .commit = block_dirty_bitmap_clear_commit,
         .abort = block_dirty_bitmap_clear_abort,
-    }
+    },
+    [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_ENABLE] = {
+        .instance_size = sizeof(BlockDirtyBitmapState),
+        .prepare = block_dirty_bitmap_enable_prepare,
+        .abort = block_dirty_bitmap_enable_abort,
+    },
+    [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_DISABLE] = {
+        .instance_size = sizeof(BlockDirtyBitmapState),
+        .prepare = block_dirty_bitmap_disable_prepare,
+        .abort = block_dirty_bitmap_disable_abort,
+     }
 };
 
 /**
diff --git a/qapi/transaction.json b/qapi/transaction.json
index bd312792da..d7e4274550 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -46,6 +46,8 @@
 # - @abort: since 1.6
 # - @block-dirty-bitmap-add: since 2.5
 # - @block-dirty-bitmap-clear: since 2.5
+# - @x-block-dirty-bitmap-enable: since 3.0
+# - @x-block-dirty-bitmap-disable: since 3.0
 # - @blockdev-backup: since 2.3
 # - @blockdev-snapshot: since 2.5
 # - @blockdev-snapshot-internal-sync: since 1.7
@@ -59,6 +61,8 @@
        'abort': 'Abort',
        'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
        'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
+       'x-block-dirty-bitmap-enable': 'BlockDirtyBitmap',
+       'x-block-dirty-bitmap-disable': 'BlockDirtyBitmap',
        'blockdev-backup': 'BlockdevBackup',
        'blockdev-snapshot': 'BlockdevSnapshot',
        'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
-- 
2.14.3

[Qemu-devel] [PATCH v2 4/5] qapi: add x-block-dirty-bitmap-merge

[John Snow]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/dirty-bitmap.c         | 18 ++++++++++++++++++
 blockdev.c                   | 30 ++++++++++++++++++++++++++++++
 include/block/dirty-bitmap.h |  3 ++-
 qapi/block-core.json         | 38 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 56234257f4..4159d3929e 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -757,3 +757,21 @@ int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t offset)
 {
     return hbitmap_next_zero(bitmap->bitmap, offset);
 }
+
+void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
+                             Error **errp)
+{
+    /* only bitmaps from one bds are supported */
+    assert(dest->mutex == src->mutex);
+
+    qemu_mutex_lock(dest->mutex);
+
+    assert(bdrv_dirty_bitmap_enabled(dest));
+    assert(!bdrv_dirty_bitmap_readonly(dest));
+
+    if (!hbitmap_merge(dest->bitmap, src->bitmap)) {
+        error_setg(errp, "Bitmaps are incompatible and can't be merged");
+    }
+
+    qemu_mutex_unlock(dest->mutex);
+}
diff --git a/blockdev.c b/blockdev.c
index af705738cd..dc40a358b0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3044,6 +3044,36 @@ void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
     bdrv_disable_dirty_bitmap(bitmap);
 }
 
+void qmp_x_block_dirty_bitmap_merge(const char *node, const char *dst_name,
+                                    const char *src_name, Error **errp)
+{
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *dst, *src;
+
+    dst = block_dirty_bitmap_lookup(node, dst_name, &bs, errp);
+    if (!dst) {
+        return;
+    }
+
+    if (bdrv_dirty_bitmap_frozen(dst)) {
+        error_setg(errp, "Bitmap '%s' is frozen and cannot be modified",
+                   dst_name);
+        return;
+    } else if (bdrv_dirty_bitmap_readonly(dst)) {
+        error_setg(errp, "Bitmap '%s' is readonly and cannot be modified",
+                   dst_name);
+        return;
+    }
+
+    src = bdrv_find_dirty_bitmap(bs, src_name);
+    if (!src) {
+        error_setg(errp, "Dirty bitmap '%s' not found", src_name);
+        return;
+    }
+
+    bdrv_merge_dirty_bitmap(dst, src, errp);
+}
+
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
                                                               const char *name,
                                                               Error **errp)
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 1ff8949b1b..1e14743032 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -70,7 +70,8 @@ void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);
 void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
                                        bool persistent);
 void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked);
-
+void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
+                             Error **errp);
 
 /* Functions that require manual locking.  */
 void bdrv_dirty_bitmap_lock(BdrvDirtyBitmap *bitmap);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 02de674f5f..9d4ab93190 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1740,6 +1740,20 @@
   'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
             '*persistent': 'bool', '*autoload': 'bool' } }
 
+##
+# @BlockDirtyBitmapMerge:
+#
+# @node: name of device/node which the bitmap is tracking
+#
+# @dst_name: name of the destination dirty bitmap
+#
+# @src_name: name of the source dirty bitmap
+#
+# Since: 3.0
+##
+{ 'struct': 'BlockDirtyBitmapMerge',
+  'data': { 'node': 'str', 'dst_name': 'str', 'src_name': 'str' } }
+
 ##
 # @block-dirty-bitmap-add:
 #
@@ -1850,6 +1864,30 @@
     { 'command': 'x-block-dirty-bitmap-disable',
       'data': 'BlockDirtyBitmap' }
 
+##
+# @x-block-dirty-bitmap-merge:
+#
+# Merge @src_name dirty bitmap to @dst_name dirty bitmap. @src_name dirty
+# bitmap is unchanged. On error, @dst_name is unchanged.
+#
+# Returns: nothing on success
+#          If @node is not a valid block device, DeviceNotFound
+#          If @dst_name or @src_name is not found, GenericError
+#          If bitmaps has different sizes or granularities, GenericError
+#
+# Since: 3.0
+#
+# Example:
+#
+# -> { "execute": "x-block-dirty-bitmap-merge",
+#      "arguments": { "node": "drive0", "dst_name": "bitmap0",
+#                     "src_name": "bitmap1" } }
+# <- { "return": {} }
+#
+##
+      { 'command': 'x-block-dirty-bitmap-merge',
+        'data': 'BlockDirtyBitmapMerge' }
+
 ##
 # @BlockDirtyBitmapSha256:
 #
-- 
2.14.3

[Qemu-devel] [PATCH v2 5/5] qapi: add disabled parameter to block-dirty-bitmap-add

[John Snow]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

This is needed, for example, to create a new bitmap and merge several
disabled bitmaps into a new one. Without this flag we will have to
put block-dirty-bitmap-add and block-dirty-bitmap-disable into one
transaction.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
---
 blockdev.c           | 10 ++++++++++
 qapi/block-core.json |  6 +++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index dc40a358b0..041f5d594f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2073,6 +2073,7 @@ static void block_dirty_bitmap_add_prepare(BlkActionState *common,
                                action->has_granularity, action->granularity,
                                action->has_persistent, action->persistent,
                                action->has_autoload, action->autoload,
+                               action->has_x_disabled, action->x_disabled,
                                &local_err);
 
     if (!local_err) {
@@ -2880,6 +2881,7 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
                                 bool has_granularity, uint32_t granularity,
                                 bool has_persistent, bool persistent,
                                 bool has_autoload, bool autoload,
+                                bool has_disabled, bool disabled,
                                 Error **errp)
 {
     BlockDriverState *bs;
@@ -2914,6 +2916,10 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
         warn_report("Autoload option is deprecated and its value is ignored");
     }
 
+    if (!has_disabled) {
+        disabled = false;
+    }
+
     if (persistent &&
         !bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
     {
@@ -2925,6 +2931,10 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
         return;
     }
 
+    if (disabled) {
+        bdrv_disable_dirty_bitmap(bitmap);
+    }
+
     bdrv_dirty_bitmap_set_persistance(bitmap, persistent);
 }
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9d4ab93190..fff23fc82b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1734,11 +1734,15 @@
 #            Currently, all dirty tracking bitmaps are loaded from Qcow2 on
 #            open.
 #
+# @x-disabled: the bitmap is created in the disabled state, which means that
+#              it will not track drive changes. The bitmap may be enabled with
+#              x-block-dirty-bitmap-enable. Default is false. (Since: 3.0)
+#
 # Since: 2.4
 ##
 { 'struct': 'BlockDirtyBitmapAdd',
   'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
-            '*persistent': 'bool', '*autoload': 'bool' } }
+            '*persistent': 'bool', '*autoload': 'bool', '*x-disabled': 'bool' } }
 
 ##
 # @BlockDirtyBitmapMerge:
-- 
2.14.3

Re: [Qemu-devel] [Qemu-block] [PATCH v2 4/5] qapi: add x-block-dirty-bitmap-merge

[Jeff Cody]

On Wed, Jun 06, 2018 at 02:24:48PM -0400, John Snow wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Jeff Cody <jcody@redhat.com>

> ---
>  block/dirty-bitmap.c         | 18 ++++++++++++++++++
>  blockdev.c                   | 30 ++++++++++++++++++++++++++++++
>  include/block/dirty-bitmap.h |  3 ++-
>  qapi/block-core.json         | 38 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 88 insertions(+), 1 deletion(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 56234257f4..4159d3929e 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -757,3 +757,21 @@ int64_t bdrv_dirty_bitmap_next_zero(BdrvDirtyBitmap *bitmap, uint64_t offset)
>  {
>      return hbitmap_next_zero(bitmap->bitmap, offset);
>  }
> +
> +void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
> +                             Error **errp)
> +{
> +    /* only bitmaps from one bds are supported */
> +    assert(dest->mutex == src->mutex);
> +
> +    qemu_mutex_lock(dest->mutex);
> +
> +    assert(bdrv_dirty_bitmap_enabled(dest));
> +    assert(!bdrv_dirty_bitmap_readonly(dest));
> +
> +    if (!hbitmap_merge(dest->bitmap, src->bitmap)) {
> +        error_setg(errp, "Bitmaps are incompatible and can't be merged");
> +    }
> +
> +    qemu_mutex_unlock(dest->mutex);
> +}
> diff --git a/blockdev.c b/blockdev.c
> index af705738cd..dc40a358b0 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3044,6 +3044,36 @@ void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
>      bdrv_disable_dirty_bitmap(bitmap);
>  }
>  
> +void qmp_x_block_dirty_bitmap_merge(const char *node, const char *dst_name,
> +                                    const char *src_name, Error **errp)
> +{
> +    BlockDriverState *bs;
> +    BdrvDirtyBitmap *dst, *src;
> +
> +    dst = block_dirty_bitmap_lookup(node, dst_name, &bs, errp);
> +    if (!dst) {
> +        return;
> +    }
> +
> +    if (bdrv_dirty_bitmap_frozen(dst)) {
> +        error_setg(errp, "Bitmap '%s' is frozen and cannot be modified",
> +                   dst_name);
> +        return;
> +    } else if (bdrv_dirty_bitmap_readonly(dst)) {
> +        error_setg(errp, "Bitmap '%s' is readonly and cannot be modified",
> +                   dst_name);
> +        return;
> +    }
> +
> +    src = bdrv_find_dirty_bitmap(bs, src_name);
> +    if (!src) {
> +        error_setg(errp, "Dirty bitmap '%s' not found", src_name);
> +        return;
> +    }
> +
> +    bdrv_merge_dirty_bitmap(dst, src, errp);
> +}
> +
>  BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
>                                                                const char *name,
>                                                                Error **errp)
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 1ff8949b1b..1e14743032 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -70,7 +70,8 @@ void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);
>  void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
>                                         bool persistent);
>  void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked);
> -
> +void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
> +                             Error **errp);
>  
>  /* Functions that require manual locking.  */
>  void bdrv_dirty_bitmap_lock(BdrvDirtyBitmap *bitmap);
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 02de674f5f..9d4ab93190 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1740,6 +1740,20 @@
>    'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
>              '*persistent': 'bool', '*autoload': 'bool' } }
>  
> +##
> +# @BlockDirtyBitmapMerge:
> +#
> +# @node: name of device/node which the bitmap is tracking
> +#
> +# @dst_name: name of the destination dirty bitmap
> +#
> +# @src_name: name of the source dirty bitmap
> +#
> +# Since: 3.0
> +##
> +{ 'struct': 'BlockDirtyBitmapMerge',
> +  'data': { 'node': 'str', 'dst_name': 'str', 'src_name': 'str' } }
> +
>  ##
>  # @block-dirty-bitmap-add:
>  #
> @@ -1850,6 +1864,30 @@
>      { 'command': 'x-block-dirty-bitmap-disable',
>        'data': 'BlockDirtyBitmap' }
>  
> +##
> +# @x-block-dirty-bitmap-merge:
> +#
> +# Merge @src_name dirty bitmap to @dst_name dirty bitmap. @src_name dirty
> +# bitmap is unchanged. On error, @dst_name is unchanged.
> +#
> +# Returns: nothing on success
> +#          If @node is not a valid block device, DeviceNotFound
> +#          If @dst_name or @src_name is not found, GenericError
> +#          If bitmaps has different sizes or granularities, GenericError
> +#
> +# Since: 3.0
> +#
> +# Example:
> +#
> +# -> { "execute": "x-block-dirty-bitmap-merge",
> +#      "arguments": { "node": "drive0", "dst_name": "bitmap0",
> +#                     "src_name": "bitmap1" } }
> +# <- { "return": {} }
> +#
> +##
> +      { 'command': 'x-block-dirty-bitmap-merge',
> +        'data': 'BlockDirtyBitmapMerge' }
> +
>  ##
>  # @BlockDirtyBitmapSha256:
>  #
> -- 
> 2.14.3
> 
> 

Re: [Qemu-devel] [Qemu-block] [PATCH v2 3/5] qmp: transaction support for x-block-dirty-bitmap-enable/disable

[Jeff Cody]

On Wed, Jun 06, 2018 at 02:24:47PM -0400, John Snow wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> [Added x- prefix. --js]
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Jeff Cody <jcody@redhat.com>

> ---
>  blockdev.c            | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qapi/transaction.json |  4 +++
>  2 files changed, 84 insertions(+), 1 deletion(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index fb402edc94..af705738cd 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2052,6 +2052,7 @@ typedef struct BlockDirtyBitmapState {
>      BlockDriverState *bs;
>      HBitmap *backup;
>      bool prepared;
> +    bool was_enabled;
>  } BlockDirtyBitmapState;
>  
>  static void block_dirty_bitmap_add_prepare(BlkActionState *common,
> @@ -2151,6 +2152,74 @@ static void block_dirty_bitmap_clear_commit(BlkActionState *common)
>      hbitmap_free(state->backup);
>  }
>  
> +static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
> +                                              Error **errp)
> +{
> +    BlockDirtyBitmap *action;
> +    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> +                                             common, common);
> +
> +    if (action_check_completion_mode(common, errp) < 0) {
> +        return;
> +    }
> +
> +    action = common->action->u.x_block_dirty_bitmap_enable.data;
> +    state->bitmap = block_dirty_bitmap_lookup(action->node,
> +                                              action->name,
> +                                              NULL,
> +                                              errp);
> +    if (!state->bitmap) {
> +        return;
> +    }
> +
> +    state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
> +    bdrv_enable_dirty_bitmap(state->bitmap);
> +}
> +
> +static void block_dirty_bitmap_enable_abort(BlkActionState *common)
> +{
> +    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> +                                             common, common);
> +
> +    if (!state->was_enabled) {
> +        bdrv_disable_dirty_bitmap(state->bitmap);
> +    }
> +}
> +
> +static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
> +                                               Error **errp)
> +{
> +    BlockDirtyBitmap *action;
> +    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> +                                             common, common);
> +
> +    if (action_check_completion_mode(common, errp) < 0) {
> +        return;
> +    }
> +
> +    action = common->action->u.x_block_dirty_bitmap_disable.data;
> +    state->bitmap = block_dirty_bitmap_lookup(action->node,
> +                                              action->name,
> +                                              NULL,
> +                                              errp);
> +    if (!state->bitmap) {
> +        return;
> +    }
> +
> +    state->was_enabled = bdrv_dirty_bitmap_enabled(state->bitmap);
> +    bdrv_disable_dirty_bitmap(state->bitmap);
> +}
> +
> +static void block_dirty_bitmap_disable_abort(BlkActionState *common)
> +{
> +    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> +                                             common, common);
> +
> +    if (state->was_enabled) {
> +        bdrv_enable_dirty_bitmap(state->bitmap);
> +    }
> +}
> +
>  static void abort_prepare(BlkActionState *common, Error **errp)
>  {
>      error_setg(errp, "Transaction aborted using Abort action");
> @@ -2211,7 +2280,17 @@ static const BlkActionOps actions[] = {
>          .prepare = block_dirty_bitmap_clear_prepare,
>          .commit = block_dirty_bitmap_clear_commit,
>          .abort = block_dirty_bitmap_clear_abort,
> -    }
> +    },
> +    [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_ENABLE] = {
> +        .instance_size = sizeof(BlockDirtyBitmapState),
> +        .prepare = block_dirty_bitmap_enable_prepare,
> +        .abort = block_dirty_bitmap_enable_abort,
> +    },
> +    [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_DISABLE] = {
> +        .instance_size = sizeof(BlockDirtyBitmapState),
> +        .prepare = block_dirty_bitmap_disable_prepare,
> +        .abort = block_dirty_bitmap_disable_abort,
> +     }
>  };
>  
>  /**
> diff --git a/qapi/transaction.json b/qapi/transaction.json
> index bd312792da..d7e4274550 100644
> --- a/qapi/transaction.json
> +++ b/qapi/transaction.json
> @@ -46,6 +46,8 @@
>  # - @abort: since 1.6
>  # - @block-dirty-bitmap-add: since 2.5
>  # - @block-dirty-bitmap-clear: since 2.5
> +# - @x-block-dirty-bitmap-enable: since 3.0
> +# - @x-block-dirty-bitmap-disable: since 3.0
>  # - @blockdev-backup: since 2.3
>  # - @blockdev-snapshot: since 2.5
>  # - @blockdev-snapshot-internal-sync: since 1.7
> @@ -59,6 +61,8 @@
>         'abort': 'Abort',
>         'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
>         'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
> +       'x-block-dirty-bitmap-enable': 'BlockDirtyBitmap',
> +       'x-block-dirty-bitmap-disable': 'BlockDirtyBitmap',
>         'blockdev-backup': 'BlockdevBackup',
>         'blockdev-snapshot': 'BlockdevSnapshot',
>         'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
> -- 
> 2.14.3
> 
> 

Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/5] qapi: add x-block-dirty-bitmap-enable/disable

[Jeff Cody]

On Wed, Jun 06, 2018 at 02:24:46PM -0400, John Snow wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> Expose the ability to turn bitmaps "on" or "off". This is experimental
> and principally for the sake of the Libvirt Checkpoints API, and it may
> or may not be committed for 3.0.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Jeff Cody <jcody@redhat.com>

> ---
>  blockdev.c           | 42 ++++++++++++++++++++++++++++++++++++++++++
>  qapi/block-core.json | 42 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 84 insertions(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 8de95be8f4..fb402edc94 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2923,6 +2923,48 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
>      bdrv_clear_dirty_bitmap(bitmap, NULL);
>  }
>  
> +void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
> +                                   Error **errp)
> +{
> +    BlockDriverState *bs;
> +    BdrvDirtyBitmap *bitmap;
> +
> +    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
> +    if (!bitmap) {
> +        return;
> +    }
> +
> +    if (bdrv_dirty_bitmap_frozen(bitmap)) {
> +        error_setg(errp,
> +                   "Bitmap '%s' is currently frozen and cannot be enabled",
> +                   name);
> +        return;
> +    }
> +
> +    bdrv_enable_dirty_bitmap(bitmap);
> +}
> +
> +void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
> +                                    Error **errp)
> +{
> +    BlockDriverState *bs;
> +    BdrvDirtyBitmap *bitmap;
> +
> +    bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
> +    if (!bitmap) {
> +        return;
> +    }
> +
> +    if (bdrv_dirty_bitmap_frozen(bitmap)) {
> +        error_setg(errp,
> +                   "Bitmap '%s' is currently frozen and cannot be disabled",
> +                   name);
> +        return;
> +    }
> +
> +    bdrv_disable_dirty_bitmap(bitmap);
> +}
> +
>  BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
>                                                                const char *name,
>                                                                Error **errp)
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 4b1de474a9..02de674f5f 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1808,6 +1808,48 @@
>  { 'command': 'block-dirty-bitmap-clear',
>    'data': 'BlockDirtyBitmap' }
>  
> +##
> +# @x-block-dirty-bitmap-enable:
> +#
> +# Enables a dirty bitmap so that it will begin tracking disk changes.
> +#
> +# Returns: nothing on success
> +#          If @node is not a valid block device, DeviceNotFound
> +#          If @name is not found, GenericError with an explanation
> +#
> +# Since: 3.0
> +#
> +# Example:
> +#
> +# -> { "execute": "x-block-dirty-bitmap-enable",
> +#      "arguments": { "node": "drive0", "name": "bitmap0" } }
> +# <- { "return": {} }
> +#
> +##
> +  { 'command': 'x-block-dirty-bitmap-enable',
> +    'data': 'BlockDirtyBitmap' }
> +
> +##
> +# @x-block-dirty-bitmap-disable:
> +#
> +# Disables a dirty bitmap so that it will stop tracking disk changes.
> +#
> +# Returns: nothing on success
> +#          If @node is not a valid block device, DeviceNotFound
> +#          If @name is not found, GenericError with an explanation
> +#
> +# Since: 3.0
> +#
> +# Example:
> +#
> +# -> { "execute": "x-block-dirty-bitmap-disable",
> +#      "arguments": { "node": "drive0", "name": "bitmap0" } }
> +# <- { "return": {} }
> +#
> +##
> +    { 'command': 'x-block-dirty-bitmap-disable',
> +      'data': 'BlockDirtyBitmap' }
> +
>  ##
>  # @BlockDirtyBitmapSha256:
>  #
> -- 
> 2.14.3
> 
> 

Re: [Qemu-devel] [PATCH v2 0/5] block dirty bitmaps: support libvirt API

[John Snow]

On 06/06/2018 02:24 PM, John Snow wrote:
> This is largely the same series that Vladimir sent in January, but at
> the time I was unsure of if we'd want these commands or not in QEMU.
> 
> After discussing with Virtuozzo their plans for a checkpoint-like API
> implemented primarily in libvirt, I agree that these commands are at
> least tentatively useful.
> 
> Eric Blake is currently writing a counter-proposal and demo API to show
> to Virtuozzo on libvirt's development list. Check in these QMP commands
> with the experimental prefix 'x-' for now so that it can be used for
> prototyping.
> 
> Once the design for the libvirt API looks reasonably final I will
> remove the 'x-' prefixes, or, if we wind up not using these particular
> commands I will delete them entirely.
> 
> v4:
>  - Reinstated Vladimir's authorship credit on all patches
>  - Switched 2.12 to 3.0
>  - Edited doc to clarify failure mode for merge (Jeff)
>  - Removed !bs check where applicable (Jeff)
> 
> v3:
>  - Drop patch one (already merged)
>  - Minor phrasing adjustments to documentation
>  - Removed &state->bs argument to bitmap lookup for enable/disable
>  - Added x- prefix to all three commands and to add's new argument.
> 
> Vladimir's original cover letter is below:
> 
> There are three qmp commands, needed to implement external backup API.
> 
> Using these three commands, client may do all needed bitmap management by
> hand:
> 
> on backup start we need to do a transaction:
>  {disable old bitmap, create new bitmap}
> 
> on backup success:
>  drop old bitmap
> 
> on backup fail:
>  enable old bitmap
>  merge new bitmap to old bitmap
>  drop new bitmap
> 
> Vladimir Sementsov-Ogievskiy (5):
>   block/dirty-bitmap: add lock to bdrv_enable/disable_dirty_bitmap
>   qapi: add x-block-dirty-bitmap-enable/disable
>   qmp: transaction support for x-block-dirty-bitmap-enable/disable
>   qapi: add x-block-dirty-bitmap-merge
>   qapi: add disabled parameter to block-dirty-bitmap-add
> 
>  block/dirty-bitmap.c         |  24 ++++++-
>  blockdev.c                   | 163 ++++++++++++++++++++++++++++++++++++++++++-
>  include/block/dirty-bitmap.h |   3 +-
>  qapi/block-core.json         |  86 ++++++++++++++++++++++-
>  qapi/transaction.json        |   4 ++
>  5 files changed, 275 insertions(+), 5 deletions(-)
> 

Hmm, I sorta-maybe have jurisdiction on this, but it touches more
blockdev.c and qapi/block-core.json than it does block/dirty-bitmap.c.

I'm gonna send a PR for this and if I was wrong about that, Kevin, just
veto the PR thread.

--js