[Qemu-devel] [PATCH v2 0/2] bitmaps: merge bitmaps from different nodes

[Qemu-devel] [PATCH v2 0/2] bitmaps: merge bitmaps from different nodes

[Vladimir Sementsov-Ogievskiy]

Hi all!

We need to copy bitmaps to new top node on external snapshot, to
not break incremental backup chain.

The only thing to do is to allow block-dirty-bitmap-merge to work
with different nodes, here it is.

v2: use 'alternate' type in qapi for specifying source bitmap
    instead of adding new field external-bitmaps [Eric]

Vladimir Sementsov-Ogievskiy (2):
  qapi: support external bitmaps in block-dirty-bitmap-merge
  iotests: test external snapshot with bitmap copying

 qapi/block-core.json       | 22 +++++++++++++---
 block/dirty-bitmap.c       |  9 ++++---
 blockdev.c                 | 50 +++++++++++++++++++++++-------------
 tests/qemu-iotests/254     | 52 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/254.out | 52 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 6 files changed, 163 insertions(+), 23 deletions(-)
 create mode 100755 tests/qemu-iotests/254
 create mode 100644 tests/qemu-iotests/254.out

-- 
2.18.0

[Qemu-devel] [PATCH v2 1/2] qapi: support external bitmaps in block-dirty-bitmap-merge

[Vladimir Sementsov-Ogievskiy]

Add new optional parameter making possible to merge bitmaps from
different nodes. It is needed to maintain external snapshots during
incremental backup chain history.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qapi/block-core.json | 22 ++++++++++++++++---
 block/dirty-bitmap.c |  9 +++++---
 blockdev.c           | 50 +++++++++++++++++++++++++++++---------------
 3 files changed, 58 insertions(+), 23 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7ccbfff9d0..dcc935d655 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2003,19 +2003,35 @@
   'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
             '*persistent': 'bool', '*autoload': 'bool', '*disabled': 'bool' } }
 
+##
+# @BlockDirtyBitmapMergeSource:
+#
+# @local: name of the bitmap, attached to the same node as target bitmap.
+#
+# @external: bitmap with specified node
+#
+# Since: 4.1
+##
+{ 'alternate': 'BlockDirtyBitmapMergeSource',
+  'data': { 'local': 'str',
+            'external': 'BlockDirtyBitmap' } }
+
 ##
 # @BlockDirtyBitmapMerge:
 #
-# @node: name of device/node which the bitmap is tracking
+# @node: name of device/node which the @target bitmap is tracking
 #
 # @target: name of the destination dirty bitmap
 #
-# @bitmaps: name(s) of the source dirty bitmap(s)
+# @bitmaps: name(s) of the source dirty bitmap(s) at @node and/or fully
+#           specifed BlockDirtyBitmap elements. The latter are supported
+#           since 4.1.
 #
 # Since: 4.0
 ##
 { 'struct': 'BlockDirtyBitmapMerge',
-  'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } }
+  'data': { 'node': 'str', 'target': 'str',
+            'bitmaps': ['BlockDirtyBitmapMergeSource'] } }
 
 ##
 # @block-dirty-bitmap-add:
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 59e6ebb861..49646a30e6 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -816,10 +816,10 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
 {
     bool ret;
 
-    /* only bitmaps from one bds are supported */
-    assert(dest->mutex == src->mutex);
-
     qemu_mutex_lock(dest->mutex);
+    if (src->mutex != dest->mutex) {
+        qemu_mutex_lock(src->mutex);
+    }
 
     if (bdrv_dirty_bitmap_check(dest, BDRV_BITMAP_DEFAULT, errp)) {
         goto out;
@@ -845,4 +845,7 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
 
 out:
     qemu_mutex_unlock(dest->mutex);
+    if (src->mutex != dest->mutex) {
+        qemu_mutex_unlock(src->mutex);
+    }
 }
diff --git a/blockdev.c b/blockdev.c
index 79fbac8450..64ccef735b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2112,11 +2112,10 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common)
     }
 }
 
-static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
-                                                    const char *target,
-                                                    strList *bitmaps,
-                                                    HBitmap **backup,
-                                                    Error **errp);
+static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(
+        const char *node, const char *target,
+        BlockDirtyBitmapMergeSourceList *bitmaps,
+        HBitmap **backup, Error **errp);
 
 static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
                                              Error **errp)
@@ -2965,15 +2964,14 @@ void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
     bdrv_disable_dirty_bitmap(bitmap);
 }
 
-static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
-                                                    const char *target,
-                                                    strList *bitmaps,
-                                                    HBitmap **backup,
-                                                    Error **errp)
+static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(
+        const char *node, const char *target,
+        BlockDirtyBitmapMergeSourceList *bitmaps,
+        HBitmap **backup, Error **errp)
 {
     BlockDriverState *bs;
     BdrvDirtyBitmap *dst, *src, *anon;
-    strList *lst;
+    BlockDirtyBitmapMergeSourceList *lst;
     Error *local_err = NULL;
 
     dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
@@ -2988,11 +2986,28 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
     }
 
     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);
-            dst = NULL;
-            goto out;
+        switch (lst->value->type) {
+            const char *name, *node;
+        case QTYPE_QSTRING:
+            name = lst->value->u.local;
+            src = bdrv_find_dirty_bitmap(bs, name);
+            if (!src) {
+                error_setg(errp, "Dirty bitmap '%s' not found", name);
+                dst = NULL;
+                goto out;
+            }
+            break;
+        case QTYPE_QDICT:
+            node = lst->value->u.external.node;
+            name = lst->value->u.external.name;
+            src = block_dirty_bitmap_lookup(node, name, NULL, errp);
+            if (!src) {
+                dst = NULL;
+                goto out;
+            }
+            break;
+        default:
+            abort();
         }
 
         bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err);
@@ -3012,7 +3027,8 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
 }
 
 void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
-                                  strList *bitmaps, Error **errp)
+                                  BlockDirtyBitmapMergeSourceList *bitmaps,
+                                  Error **errp)
 {
     do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
 }
-- 
2.18.0

[Qemu-devel] [PATCH v2 2/2] iotests: test external snapshot with bitmap copying

[Vladimir Sementsov-Ogievskiy]

This test shows that external snapshots and incremental backups are
friends.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/254     | 52 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/254.out | 52 ++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 105 insertions(+)
 create mode 100755 tests/qemu-iotests/254
 create mode 100644 tests/qemu-iotests/254.out

diff --git a/tests/qemu-iotests/254 b/tests/qemu-iotests/254
new file mode 100755
index 0000000000..33cb80a512
--- /dev/null
+++ b/tests/qemu-iotests/254
@@ -0,0 +1,52 @@
+#!/usr/bin/env python
+#
+# Test external snapshot with bitmap copying.
+#
+# Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import iotests
+from iotests import qemu_img_create, file_path, log
+
+disk, top = file_path('disk', 'top')
+size = 1024 * 1024
+
+qemu_img_create('-f', iotests.imgfmt, disk, str(size))
+
+vm = iotests.VM().add_drive(disk, opts='node-name=base')
+vm.launch()
+
+vm.qmp_log('block-dirty-bitmap-add', node='drive0', name='bitmap0')
+
+vm.hmp_qemu_io('drive0', 'write 0 512K')
+
+vm.qmp_log('transaction', indent=2, actions=[
+    {'type': 'blockdev-snapshot-sync',
+     'data': {'device': 'drive0', 'snapshot-file': top,
+              'snapshot-node-name': 'snap'}},
+    {'type': 'block-dirty-bitmap-add',
+     'data': {'node': 'snap', 'name': 'bitmap0'}},
+    {'type': 'block-dirty-bitmap-merge',
+     'data': {'node': 'snap', 'target': 'bitmap0',
+              'bitmaps': [{'node': 'base', 'name': 'bitmap0'}]}}
+], filters=[iotests.filter_qmp_testfiles])
+
+result = vm.qmp('query-block')['return'][0]
+log("query-block: device = {}, node-name = {}, dirty-bitmaps:".format(
+    result['device'], result['inserted']['node-name']))
+log(result['dirty-bitmaps'], indent=2)
+
+vm.shutdown()
diff --git a/tests/qemu-iotests/254.out b/tests/qemu-iotests/254.out
new file mode 100644
index 0000000000..d7394cf002
--- /dev/null
+++ b/tests/qemu-iotests/254.out
@@ -0,0 +1,52 @@
+{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "node": "drive0"}}
+{"return": {}}
+{
+  "execute": "transaction",
+  "arguments": {
+    "actions": [
+      {
+        "data": {
+          "device": "drive0",
+          "snapshot-file": "TEST_DIR/PID-top",
+          "snapshot-node-name": "snap"
+        },
+        "type": "blockdev-snapshot-sync"
+      },
+      {
+        "data": {
+          "name": "bitmap0",
+          "node": "snap"
+        },
+        "type": "block-dirty-bitmap-add"
+      },
+      {
+        "data": {
+          "bitmaps": [
+            {
+              "name": "bitmap0",
+              "node": "base"
+            }
+          ],
+          "node": "snap",
+          "target": "bitmap0"
+        },
+        "type": "block-dirty-bitmap-merge"
+      }
+    ]
+  }
+}
+{
+  "return": {}
+}
+query-block: device = drive0, node-name = snap, dirty-bitmaps:
+[
+  {
+    "busy": false,
+    "count": 524288,
+    "granularity": 65536,
+    "name": "bitmap0",
+    "persistent": false,
+    "recording": true,
+    "status": "active"
+  }
+]
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 00e474ab0a..5552d0153c 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -250,3 +250,4 @@
 248 rw auto quick
 249 rw auto quick
 252 rw auto backing quick
+254 rw auto backing quick
-- 
2.18.0

Re: [Qemu-devel] [PATCH v2 1/2] qapi: support external bitmaps in block-dirty-bitmap-merge

[John Snow]

On 5/17/19 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add new optional parameter making possible to merge bitmaps from
> different nodes. It is needed to maintain external snapshots during
> incremental backup chain history.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  qapi/block-core.json | 22 ++++++++++++++++---
>  block/dirty-bitmap.c |  9 +++++---
>  blockdev.c           | 50 +++++++++++++++++++++++++++++---------------
>  3 files changed, 58 insertions(+), 23 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 7ccbfff9d0..dcc935d655 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2003,19 +2003,35 @@
>    'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
>              '*persistent': 'bool', '*autoload': 'bool', '*disabled': 'bool' } }
>  
> +##
> +# @BlockDirtyBitmapMergeSource:
> +#
> +# @local: name of the bitmap, attached to the same node as target bitmap.
> +#
> +# @external: bitmap with specified node
> +#
> +# Since: 4.1
> +##
> +{ 'alternate': 'BlockDirtyBitmapMergeSource',
> +  'data': { 'local': 'str',
> +            'external': 'BlockDirtyBitmap' } }
> +

We might be able to use something more generic to name this type of
thing, but I think such changes are wire compatible, so we can rename it
to be more generic if we decide to use this for something else in the
future, so this is good.

>  ##
>  # @BlockDirtyBitmapMerge:
>  #
> -# @node: name of device/node which the bitmap is tracking
> +# @node: name of device/node which the @target bitmap is tracking
>  #
>  # @target: name of the destination dirty bitmap
>  #
> -# @bitmaps: name(s) of the source dirty bitmap(s)
> +# @bitmaps: name(s) of the source dirty bitmap(s) at @node and/or fully
> +#           specifed BlockDirtyBitmap elements. The latter are supported
> +#           since 4.1.
>  #
>  # Since: 4.0
>  ##
>  { 'struct': 'BlockDirtyBitmapMerge',
> -  'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } }
> +  'data': { 'node': 'str', 'target': 'str',
> +            'bitmaps': ['BlockDirtyBitmapMergeSource'] } }
>  
>  ##
>  # @block-dirty-bitmap-add:
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 59e6ebb861..49646a30e6 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -816,10 +816,10 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
>  {
>      bool ret;
>  
> -    /* only bitmaps from one bds are supported */
> -    assert(dest->mutex == src->mutex);
> -
>      qemu_mutex_lock(dest->mutex);
> +    if (src->mutex != dest->mutex) {
> +        qemu_mutex_lock(src->mutex);
> +    }
>  
>      if (bdrv_dirty_bitmap_check(dest, BDRV_BITMAP_DEFAULT, errp)) {
>          goto out;
> @@ -845,4 +845,7 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
>  
>  out:
>      qemu_mutex_unlock(dest->mutex);
> +    if (src->mutex != dest->mutex) {
> +        qemu_mutex_unlock(src->mutex);
> +    }
>  }
> diff --git a/blockdev.c b/blockdev.c
> index 79fbac8450..64ccef735b 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2112,11 +2112,10 @@ static void block_dirty_bitmap_disable_abort(BlkActionState *common)
>      }
>  }
>  
> -static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
> -                                                    const char *target,
> -                                                    strList *bitmaps,
> -                                                    HBitmap **backup,
> -                                                    Error **errp);
> +static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(
> +        const char *node, const char *target,
> +        BlockDirtyBitmapMergeSourceList *bitmaps,
> +        HBitmap **backup, Error **errp);
>  
>  static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
>                                               Error **errp)
> @@ -2965,15 +2964,14 @@ void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
>      bdrv_disable_dirty_bitmap(bitmap);
>  }
>  
> -static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
> -                                                    const char *target,
> -                                                    strList *bitmaps,
> -                                                    HBitmap **backup,
> -                                                    Error **errp)
> +static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(
> +        const char *node, const char *target,
> +        BlockDirtyBitmapMergeSourceList *bitmaps,
> +        HBitmap **backup, Error **errp)
>  {
>      BlockDriverState *bs;
>      BdrvDirtyBitmap *dst, *src, *anon;
> -    strList *lst;
> +    BlockDirtyBitmapMergeSourceList *lst;
>      Error *local_err = NULL;
>  
>      dst = block_dirty_bitmap_lookup(node, target, &bs, errp);
> @@ -2988,11 +2986,28 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
>      }
>  
>      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);
> -            dst = NULL;
> -            goto out;
> +        switch (lst->value->type) {
> +            const char *name, *node;
> +        case QTYPE_QSTRING:
> +            name = lst->value->u.local;
> +            src = bdrv_find_dirty_bitmap(bs, name);
> +            if (!src) {
> +                error_setg(errp, "Dirty bitmap '%s' not found", name);
> +                dst = NULL;
> +                goto out;
> +            }
> +            break;
> +        case QTYPE_QDICT:
> +            node = lst->value->u.external.node;
> +            name = lst->value->u.external.name;
> +            src = block_dirty_bitmap_lookup(node, name, NULL, errp);
> +            if (!src) {
> +                dst = NULL;
> +                goto out;
> +            }
> +            break;
> +        default:
> +            abort();
>          }
>  
>          bdrv_merge_dirty_bitmap(anon, src, NULL, &local_err);
> @@ -3012,7 +3027,8 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
>  }
>  
>  void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
> -                                  strList *bitmaps, Error **errp)
> +                                  BlockDirtyBitmapMergeSourceList *bitmaps,
> +                                  Error **errp)
>  {
>      do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
>  }
> 

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

Re: [Qemu-devel] [PATCH v2 2/2] iotests: test external snapshot with bitmap copying

[John Snow]

On 5/17/19 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
> This test shows that external snapshots and incremental backups are
> friends.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  tests/qemu-iotests/254     | 52 ++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/254.out | 52 ++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 105 insertions(+)
>  create mode 100755 tests/qemu-iotests/254
>  create mode 100644 tests/qemu-iotests/254.out
> 
> diff --git a/tests/qemu-iotests/254 b/tests/qemu-iotests/254
> new file mode 100755
> index 0000000000..33cb80a512
> --- /dev/null
> +++ b/tests/qemu-iotests/254
> @@ -0,0 +1,52 @@
> +#!/usr/bin/env python
> +#
> +# Test external snapshot with bitmap copying.
> +#
> +# Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import iotests
> +from iotests import qemu_img_create, file_path, log
> +
> +disk, top = file_path('disk', 'top')
> +size = 1024 * 1024
> +
> +qemu_img_create('-f', iotests.imgfmt, disk, str(size))
> +
> +vm = iotests.VM().add_drive(disk, opts='node-name=base')
> +vm.launch()
> +
> +vm.qmp_log('block-dirty-bitmap-add', node='drive0', name='bitmap0')
> +
> +vm.hmp_qemu_io('drive0', 'write 0 512K')
> +
> +vm.qmp_log('transaction', indent=2, actions=[
> +    {'type': 'blockdev-snapshot-sync',
> +     'data': {'device': 'drive0', 'snapshot-file': top,
> +              'snapshot-node-name': 'snap'}},
> +    {'type': 'block-dirty-bitmap-add',
> +     'data': {'node': 'snap', 'name': 'bitmap0'}},
> +    {'type': 'block-dirty-bitmap-merge',
> +     'data': {'node': 'snap', 'target': 'bitmap0',
> +              'bitmaps': [{'node': 'base', 'name': 'bitmap0'}]}}
> +], filters=[iotests.filter_qmp_testfiles])
> +
> +result = vm.qmp('query-block')['return'][0]
> +log("query-block: device = {}, node-name = {}, dirty-bitmaps:".format(
> +    result['device'], result['inserted']['node-name']))
> +log(result['dirty-bitmaps'], indent=2)
> +
> +vm.shutdown()
> diff --git a/tests/qemu-iotests/254.out b/tests/qemu-iotests/254.out
> new file mode 100644
> index 0000000000..d7394cf002
> --- /dev/null
> +++ b/tests/qemu-iotests/254.out
> @@ -0,0 +1,52 @@
> +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "node": "drive0"}}
> +{"return": {}}
> +{
> +  "execute": "transaction",
> +  "arguments": {
> +    "actions": [
> +      {
> +        "data": {
> +          "device": "drive0",
> +          "snapshot-file": "TEST_DIR/PID-top",
> +          "snapshot-node-name": "snap"
> +        },
> +        "type": "blockdev-snapshot-sync"
> +      },
> +      {
> +        "data": {
> +          "name": "bitmap0",
> +          "node": "snap"
> +        },
> +        "type": "block-dirty-bitmap-add"
> +      },
> +      {
> +        "data": {
> +          "bitmaps": [
> +            {
> +              "name": "bitmap0",
> +              "node": "base"
> +            }
> +          ],
> +          "node": "snap",
> +          "target": "bitmap0"
> +        },
> +        "type": "block-dirty-bitmap-merge"
> +      }
> +    ]
> +  }
> +}
> +{
> +  "return": {}
> +}
> +query-block: device = drive0, node-name = snap, dirty-bitmaps:
> +[
> +  {
> +    "busy": false,
> +    "count": 524288,
> +    "granularity": 65536,
> +    "name": "bitmap0",
> +    "persistent": false,
> +    "recording": true,
> +    "status": "active"
> +  }
> +]
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index 00e474ab0a..5552d0153c 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -250,3 +250,4 @@
>  248 rw auto quick
>  249 rw auto quick
>  252 rw auto backing quick
> +254 rw auto backing quick
> 

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

We will eventually want something more rigorous in terms of a full
workflow, but for the purposes of testing cross-node merge I think this
demonstrates it fine.

I'll stage these shortly.

--js

Re: [Qemu-devel] [PATCH v2 2/2] iotests: test external snapshot with bitmap copying

[Vladimir Sementsov-Ogievskiy]

18.05.2019 4:31, John Snow wrote:
> 
> 
> On 5/17/19 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
>> This test shows that external snapshots and incremental backups are
>> friends.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/254     | 52 ++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/254.out | 52 ++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/group   |  1 +
>>   3 files changed, 105 insertions(+)
>>   create mode 100755 tests/qemu-iotests/254
>>   create mode 100644 tests/qemu-iotests/254.out
>>
>> diff --git a/tests/qemu-iotests/254 b/tests/qemu-iotests/254
>> new file mode 100755
>> index 0000000000..33cb80a512
>> --- /dev/null
>> +++ b/tests/qemu-iotests/254
>> @@ -0,0 +1,52 @@
>> +#!/usr/bin/env python
>> +#
>> +# Test external snapshot with bitmap copying.
>> +#
>> +# Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +#
>> +
>> +import iotests
>> +from iotests import qemu_img_create, file_path, log
>> +
>> +disk, top = file_path('disk', 'top')
>> +size = 1024 * 1024
>> +
>> +qemu_img_create('-f', iotests.imgfmt, disk, str(size))
>> +
>> +vm = iotests.VM().add_drive(disk, opts='node-name=base')
>> +vm.launch()
>> +
>> +vm.qmp_log('block-dirty-bitmap-add', node='drive0', name='bitmap0')
>> +
>> +vm.hmp_qemu_io('drive0', 'write 0 512K')
>> +
>> +vm.qmp_log('transaction', indent=2, actions=[
>> +    {'type': 'blockdev-snapshot-sync',
>> +     'data': {'device': 'drive0', 'snapshot-file': top,
>> +              'snapshot-node-name': 'snap'}},
>> +    {'type': 'block-dirty-bitmap-add',
>> +     'data': {'node': 'snap', 'name': 'bitmap0'}},
>> +    {'type': 'block-dirty-bitmap-merge',
>> +     'data': {'node': 'snap', 'target': 'bitmap0',
>> +              'bitmaps': [{'node': 'base', 'name': 'bitmap0'}]}}
>> +], filters=[iotests.filter_qmp_testfiles])
>> +
>> +result = vm.qmp('query-block')['return'][0]
>> +log("query-block: device = {}, node-name = {}, dirty-bitmaps:".format(
>> +    result['device'], result['inserted']['node-name']))
>> +log(result['dirty-bitmaps'], indent=2)
>> +
>> +vm.shutdown()
>> diff --git a/tests/qemu-iotests/254.out b/tests/qemu-iotests/254.out
>> new file mode 100644
>> index 0000000000..d7394cf002
>> --- /dev/null
>> +++ b/tests/qemu-iotests/254.out
>> @@ -0,0 +1,52 @@
>> +{"execute": "block-dirty-bitmap-add", "arguments": {"name": "bitmap0", "node": "drive0"}}
>> +{"return": {}}
>> +{
>> +  "execute": "transaction",
>> +  "arguments": {
>> +    "actions": [
>> +      {
>> +        "data": {
>> +          "device": "drive0",
>> +          "snapshot-file": "TEST_DIR/PID-top",
>> +          "snapshot-node-name": "snap"
>> +        },
>> +        "type": "blockdev-snapshot-sync"
>> +      },
>> +      {
>> +        "data": {
>> +          "name": "bitmap0",
>> +          "node": "snap"
>> +        },
>> +        "type": "block-dirty-bitmap-add"
>> +      },
>> +      {
>> +        "data": {
>> +          "bitmaps": [
>> +            {
>> +              "name": "bitmap0",
>> +              "node": "base"
>> +            }
>> +          ],
>> +          "node": "snap",
>> +          "target": "bitmap0"
>> +        },
>> +        "type": "block-dirty-bitmap-merge"
>> +      }
>> +    ]
>> +  }
>> +}
>> +{
>> +  "return": {}
>> +}
>> +query-block: device = drive0, node-name = snap, dirty-bitmaps:
>> +[
>> +  {
>> +    "busy": false,
>> +    "count": 524288,
>> +    "granularity": 65536,
>> +    "name": "bitmap0",
>> +    "persistent": false,
>> +    "recording": true,
>> +    "status": "active"
>> +  }
>> +]
>> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
>> index 00e474ab0a..5552d0153c 100644
>> --- a/tests/qemu-iotests/group
>> +++ b/tests/qemu-iotests/group
>> @@ -250,3 +250,4 @@
>>   248 rw auto quick
>>   249 rw auto quick
>>   252 rw auto backing quick
>> +254 rw auto backing quick
>>
> 
> Reviewed-by: John Snow <jsnow@redhat.com>
> 
> We will eventually want something more rigorous in terms of a full
> workflow, but for the purposes of testing cross-node merge I think this
> demonstrates it fine.
> 
> I'll stage these shortly.
> 

Thank you!


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v2 1/2] qapi: support external bitmaps in block-dirty-bitmap-merge

[Eric Blake]

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

On 5/17/19 5:45 PM, John Snow wrote:
> 
> 
> On 5/17/19 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Add new optional parameter making possible to merge bitmaps from
>> different nodes. It is needed to maintain external snapshots during
>> incremental backup chain history.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---

>> +# Since: 4.1
>> +##
>> +{ 'alternate': 'BlockDirtyBitmapMergeSource',
>> +  'data': { 'local': 'str',
>> +            'external': 'BlockDirtyBitmap' } }
>> +
> 
> We might be able to use something more generic to name this type of
> thing, but I think such changes are wire compatible, so we can rename it
> to be more generic if we decide to use this for something else in the
> future, so this is good.

Correct - type names are not API, so we can rename them later if it
makes sense.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH v2 2/2] iotests: test external snapshot with bitmap copying

[John Snow]

On 5/20/19 3:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> 18.05.2019 4:31, John Snow wrote:
>>
>>
>> On 5/17/19 11:21 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> This test shows that external snapshots and incremental backups are
>>> friends.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

...

>> Reviewed-by: John Snow <jsnow@redhat.com>
>>
>> We will eventually want something more rigorous in terms of a full
>> workflow, but for the purposes of testing cross-node merge I think this
>> demonstrates it fine.
>>
>> I'll stage these shortly.
>>
> 
> Thank you!
> 
> 

Staged: https://github.com/jnsnow/qemu/tree/bitmaps

Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/2] iotests: test external snapshot with bitmap copying

[Max Reitz]

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

On 17.05.19 17:21, Vladimir Sementsov-Ogievskiy wrote:
> This test shows that external snapshots and incremental backups are
> friends.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  tests/qemu-iotests/254     | 52 ++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/254.out | 52 ++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 105 insertions(+)
>  create mode 100755 tests/qemu-iotests/254
>  create mode 100644 tests/qemu-iotests/254.out

This test fails for me with qcow (v1).

Max


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

Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/2] iotests: test external snapshot with bitmap copying

[Vladimir Sementsov-Ogievskiy]

05.06.2019 17:51, Max Reitz wrote:
> On 17.05.19 17:21, Vladimir Sementsov-Ogievskiy wrote:
>> This test shows that external snapshots and incremental backups are
>> friends.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/254     | 52 ++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/254.out | 52 ++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/group   |  1 +
>>   3 files changed, 105 insertions(+)
>>   create mode 100755 tests/qemu-iotests/254
>>   create mode 100644 tests/qemu-iotests/254.out
> 
> This test fails for me with qcow (v1).
> 
> Max
> 

Thanks, will fix somehow soon.

-- 
Best regards,
Vladimir