[Qemu-devel] [PATCH 0/1] blockdev: implement n-ary bitmap merge

[Qemu-devel] [PATCH 0/1] blockdev: implement n-ary bitmap merge

[John Snow]

requires: 20180606182449.1607-1-jsnow@redhat.com

See patch for details; this is somewhat an RFC that I suspect
will be useful for libvirt in some situations, but maybe it's
actually overkill.

John Snow (1):
  blockdev: n-ary bitmap merge

 blockdev.c           | 40 ++++++++++++++++++++++++++++++----------
 qapi/block-core.json | 10 +++++-----
 2 files changed, 35 insertions(+), 15 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 1/1] blockdev: n-ary bitmap merge

[John Snow]

It might be nice to have an all-or-nothing merge command that either
succeeds in merging all bitmaps or fails for all of them. This way,
when assembling bitmaps that represent arbitrary points in time from
component bitmaps, we always know the state of the target bitmap even
in cases of failure.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c           | 40 ++++++++++++++++++++++++++++++----------
 qapi/block-core.json | 10 +++++-----
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 041f5d594f..4d60d0a03c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3054,34 +3054,54 @@ 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)
+void qmp_x_block_dirty_bitmap_merge(const char *node, const char *target,
+                                    strList *bitmaps, Error **errp)
 {
     BlockDriverState *bs;
-    BdrvDirtyBitmap *dst, *src;
+    BdrvDirtyBitmap *dst, *src, *anon;
+    strList *lst;
+    Error *local_err = NULL;
 
-    dst = block_dirty_bitmap_lookup(node, dst_name, &bs, errp);
+    dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
     if (!dst) {
         return;
     }
 
     if (bdrv_dirty_bitmap_frozen(dst)) {
         error_setg(errp, "Bitmap '%s' is frozen and cannot be modified",
-                   dst_name);
+                   target);
         return;
     } else if (bdrv_dirty_bitmap_readonly(dst)) {
         error_setg(errp, "Bitmap '%s' is readonly and cannot be modified",
-                   dst_name);
+                   target);
         return;
     }
 
-    src = bdrv_find_dirty_bitmap(bs, src_name);
-    if (!src) {
-        error_setg(errp, "Dirty bitmap '%s' not found", src_name);
+    anon = bdrv_create_dirty_bitmap(bs, bdrv_dirty_bitmap_granularity(dst),
+                                    NULL, errp);
+    if (!anon) {
         return;
     }
 
-    bdrv_merge_dirty_bitmap(dst, src, errp);
+    /* Aggregate bitmaps to anonymous temp bitmap */
+    for (lst = bitmaps; lst; lst = lst->next) {
+      src = bdrv_find_dirty_bitmap(bs, lst->value);
+      if (!src) {
+          error_setg(errp, "Dirty bitmap '%s' not found", lst->value);
+          goto out;
+      }
+      bdrv_merge_dirty_bitmap(anon, src, &local_err);
+      if (local_err) {
+          error_propagate(errp, local_err);
+          goto out;
+      }
+    }
+
+    /* Merge into dst; dst is unchanged on failure */
+    bdrv_merge_dirty_bitmap(dst, anon, errp);
+
+out:
+    bdrv_release_dirty_bitmap(bs, anon);
 }
 
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index fff23fc82b..83402da7e7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1749,14 +1749,14 @@
 #
 # @node: name of device/node which the bitmap is tracking
 #
-# @dst_name: name of the destination dirty bitmap
+# @target: name of the destination dirty bitmap
 #
-# @src_name: name of the source dirty bitmap
+# @bitmaps: name(s) of the source dirty bitmap(s)
 #
 # Since: 3.0
 ##
 { 'struct': 'BlockDirtyBitmapMerge',
-  'data': { 'node': 'str', 'dst_name': 'str', 'src_name': 'str' } }
+  'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } }
 
 ##
 # @block-dirty-bitmap-add:
@@ -1884,8 +1884,8 @@
 # Example:
 #
 # -> { "execute": "x-block-dirty-bitmap-merge",
-#      "arguments": { "node": "drive0", "dst_name": "bitmap0",
-#                     "src_name": "bitmap1" } }
+#      "arguments": { "node": "drive0", "target": "bitmap0",
+#                     "bitmaps": ["bitmap1"] } }
 # <- { "return": {} }
 #
 ##
-- 
2.14.3

Re: [Qemu-devel] [PATCH 0/1] blockdev: implement n-ary bitmap merge

[John Snow]

I mis-typed Vladimir's email, I have corrected it here.

On 06/11/2018 06:43 PM, John Snow wrote:
> requires: 20180606182449.1607-1-jsnow@redhat.com
> 
> See patch for details; this is somewhat an RFC that I suspect
> will be useful for libvirt in some situations, but maybe it's
> actually overkill.
> 
> John Snow (1):
>   blockdev: n-ary bitmap merge
> 
>  blockdev.c           | 40 ++++++++++++++++++++++++++++++----------
>  qapi/block-core.json | 10 +++++-----
>  2 files changed, 35 insertions(+), 15 deletions(-)
> 

Re: [Qemu-devel] [PATCH 0/1] blockdev: implement n-ary bitmap merge

[John Snow]

On 06/11/2018 06:43 PM, John Snow wrote:
> requires: 20180606182449.1607-1-jsnow@redhat.com

No longer requires any prerequisites.

--js

> 
> See patch for details; this is somewhat an RFC that I suspect
> will be useful for libvirt in some situations, but maybe it's
> actually overkill.
> 
> John Snow (1):
>   blockdev: n-ary bitmap merge
> 
>  blockdev.c           | 40 ++++++++++++++++++++++++++++++----------
>  qapi/block-core.json | 10 +++++-----
>  2 files changed, 35 insertions(+), 15 deletions(-)
> 

Re: [Qemu-devel] [PATCH 1/1] blockdev: n-ary bitmap merge

[Vladimir Sementsov-Ogievskiy]

add Nikolay

12.06.2018 01:43, John Snow wrote:
> It might be nice to have an all-or-nothing merge command that either
> succeeds in merging all bitmaps or fails for all of them. This way,
> when assembling bitmaps that represent arbitrary points in time from
> component bitmaps, we always know the state of the target bitmap even
> in cases of failure.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   blockdev.c           | 40 ++++++++++++++++++++++++++++++----------
>   qapi/block-core.json | 10 +++++-----
>   2 files changed, 35 insertions(+), 15 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 041f5d594f..4d60d0a03c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3054,34 +3054,54 @@ 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)
> +void qmp_x_block_dirty_bitmap_merge(const char *node, const char *target,
> +                                    strList *bitmaps, Error **errp)
>   {
>       BlockDriverState *bs;
> -    BdrvDirtyBitmap *dst, *src;
> +    BdrvDirtyBitmap *dst, *src, *anon;
> +    strList *lst;
> +    Error *local_err = NULL;
>   
> -    dst = block_dirty_bitmap_lookup(node, dst_name, &bs, errp);
> +    dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
>       if (!dst) {
>           return;
>       }
>   
>       if (bdrv_dirty_bitmap_frozen(dst)) {
>           error_setg(errp, "Bitmap '%s' is frozen and cannot be modified",
> -                   dst_name);
> +                   target);
>           return;
>       } else if (bdrv_dirty_bitmap_readonly(dst)) {
>           error_setg(errp, "Bitmap '%s' is readonly and cannot be modified",
> -                   dst_name);
> +                   target);
>           return;
>       }
>   
> -    src = bdrv_find_dirty_bitmap(bs, src_name);
> -    if (!src) {
> -        error_setg(errp, "Dirty bitmap '%s' not found", src_name);
> +    anon = bdrv_create_dirty_bitmap(bs, bdrv_dirty_bitmap_granularity(dst),
> +                                    NULL, errp);
> +    if (!anon) {
>           return;
>       }
>   
> -    bdrv_merge_dirty_bitmap(dst, src, errp);
> +    /* Aggregate bitmaps to anonymous temp bitmap */
> +    for (lst = bitmaps; lst; lst = lst->next) {
> +      src = bdrv_find_dirty_bitmap(bs, lst->value);
> +      if (!src) {
> +          error_setg(errp, "Dirty bitmap '%s' not found", lst->value);
> +          goto out;
> +      }
> +      bdrv_merge_dirty_bitmap(anon, src, &local_err);
> +      if (local_err) {
> +          error_propagate(errp, local_err);
> +          goto out;
> +      }
> +    }
> +
> +    /* Merge into dst; dst is unchanged on failure */
> +    bdrv_merge_dirty_bitmap(dst, anon, errp);
> +
> +out:
> +    bdrv_release_dirty_bitmap(bs, anon);
>   }
>   
>   BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index fff23fc82b..83402da7e7 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1749,14 +1749,14 @@
>   #
>   # @node: name of device/node which the bitmap is tracking
>   #
> -# @dst_name: name of the destination dirty bitmap
> +# @target: name of the destination dirty bitmap
>   #
> -# @src_name: name of the source dirty bitmap
> +# @bitmaps: name(s) of the source dirty bitmap(s)
>   #
>   # Since: 3.0
>   ##
>   { 'struct': 'BlockDirtyBitmapMerge',
> -  'data': { 'node': 'str', 'dst_name': 'str', 'src_name': 'str' } }
> +  'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } }
>   
>   ##
>   # @block-dirty-bitmap-add:
> @@ -1884,8 +1884,8 @@
>   # Example:
>   #
>   # -> { "execute": "x-block-dirty-bitmap-merge",
> -#      "arguments": { "node": "drive0", "dst_name": "bitmap0",
> -#                     "src_name": "bitmap1" } }
> +#      "arguments": { "node": "drive0", "target": "bitmap0",
> +#                     "bitmaps": ["bitmap1"] } }
>   # <- { "return": {} }
>   #
>   ##


-- 
Best regards,
Vladimir