All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/9] block: add differential backup support
@ 2015-06-05  0:20 John Snow
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 1/9] qapi: Rename 'dirty-bitmap' mode to 'incremental' John Snow
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: John Snow @ 2015-06-05  0:20 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, John Snow, jcody, qemu-devel

Requires: 1433454372-16356-1-git-send-email-jsnow@redhat.com
          [0/10] block: incremental backup transactions

It's entirely possible to use the incremental backup primitives to
achieve a differential backup mechanism, but in the interest of
ease of use, I am proposing the explicit addition of the mechanism
because it does not particularly complicate the code, add new edge
cases, or present itself as difficult to test.

This series actually adds two ease of use features:

(1) Add a copy primitive for bitmaps to add flexibility to the
    backup system in case users would like to later run multiple
    backup chains (weekly vs. monthly or perhaps incremental vs.
    differential)

(2) Add a 'differential' backup mode that does what the name says
    on the tin.

==
For convenience, this branch is available at:
https://github.com/jnsnow/qemu.git branch differential-backup
https://github.com/jnsnow/qemu/tree/differential-backup

This version is tagged differential-backup-v1:
https://github.com/jnsnow/qemu/releases/tag/differential-backup-v1
==

John Snow (9):
  qapi: Rename 'dirty-bitmap' mode to 'incremental'
  hbitmap: add hbitmap_copy
  block: add bdrv_copy_dirty_bitmap
  qapi: add Copy data type for bitmaps
  qmp: add qmp cmd block-dirty-bitmap-copy
  qmp: add block-dirty-bitmap-copy transaction
  block: add differential backup mode
  iotests: 124: support differential backups
  iotests: add differential backup test

 block.c                    | 35 +++++++++++++++++-
 block/backup.c             | 19 ++++++----
 block/mirror.c             |  9 ++++-
 blockdev.c                 | 61 +++++++++++++++++++++++++++++++
 docs/bitmaps.md            |  8 ++--
 include/block/block.h      |  5 +++
 include/block/block_int.h  |  2 +-
 include/qemu/hbitmap.h     |  9 +++++
 qapi-schema.json           |  4 +-
 qapi/block-core.json       | 40 ++++++++++++++++++--
 qmp-commands.hx            | 36 ++++++++++++++++--
 tests/qemu-iotests/124     | 91 +++++++++++++++++++++++++++++-----------------
 tests/qemu-iotests/124.out |  4 +-
 util/hbitmap.c             | 17 +++++++++
 14 files changed, 280 insertions(+), 60 deletions(-)

-- 
2.1.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 1/9] qapi: Rename 'dirty-bitmap' mode to 'incremental'
  2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
@ 2015-06-05  0:20 ` John Snow
  2015-06-05  2:34   ` Eric Blake
  2015-06-25 16:16   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 2/9] hbitmap: add hbitmap_copy John Snow
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 18+ messages in thread
From: John Snow @ 2015-06-05  0:20 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, John Snow, jcody, qemu-devel

If we wish to make differential backups a feature that's easy to access,
it might be pertinent to rename the "dirty-bitmap" mode to "incremental"
to make it clear what /type/ of backup the dirty-bitmap is helping us
perform.

This is an API breaking change, but 2.4 has not yet gone live,
so we have this flexibility.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 block/backup.c            | 10 +++++-----
 block/mirror.c            |  4 ++--
 docs/bitmaps.md           |  8 ++++----
 include/block/block_int.h |  2 +-
 qapi/block-core.json      |  8 ++++----
 qmp-commands.hx           |  6 +++---
 tests/qemu-iotests/124    | 10 +++++-----
 7 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index e681f1b..a8f7c43 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -37,7 +37,7 @@ typedef struct CowRequest {
 typedef struct BackupBlockJob {
     BlockJob common;
     BlockDriverState *target;
-    /* bitmap for sync=dirty-bitmap */
+    /* bitmap for sync=incremental */
     BdrvDirtyBitmap *sync_bitmap;
     MirrorSyncMode sync_mode;
     RateLimit limit;
@@ -390,7 +390,7 @@ static void coroutine_fn backup_run(void *opaque)
             qemu_coroutine_yield();
             job->common.busy = true;
         }
-    } else if (job->sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+    } else if (job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
         ret = backup_run_incremental(job);
     } else {
         /* Both FULL and TOP SYNC_MODE's require copying.. */
@@ -510,10 +510,10 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
         return;
     }
 
-    if (sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+    if (sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
         if (!sync_bitmap) {
             error_setg(errp, "must provide a valid bitmap name for "
-                             "\"dirty-bitmap\" sync mode");
+                             "\"incremental\" sync mode");
             return;
         }
 
@@ -548,7 +548,7 @@ 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_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP ?
+    job->sync_bitmap = sync_mode == MIRROR_SYNC_MODE_INCREMENTAL ?
                        sync_bitmap : NULL;
     job->common.len = len;
     job->common.co = qemu_coroutine_create(backup_run);
diff --git a/block/mirror.c b/block/mirror.c
index 58f391a..adf391c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -709,8 +709,8 @@ 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");
+    if (mode == MIRROR_SYNC_MODE_INCREMENTAL) {
+        error_setg(errp, "Sync mode 'incremental' not supported");
         return;
     }
     is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
diff --git a/docs/bitmaps.md b/docs/bitmaps.md
index a60fee1..9fd8ea6 100644
--- a/docs/bitmaps.md
+++ b/docs/bitmaps.md
@@ -206,7 +206,7 @@ full backup as a backing image.
         "bitmap": "bitmap0",
         "target": "incremental.0.img",
         "format": "qcow2",
-        "sync": "dirty-bitmap",
+        "sync": "incremental",
         "mode": "existing"
       }
     }
@@ -231,7 +231,7 @@ full backup as a backing image.
         "bitmap": "bitmap0",
         "target": "incremental.1.img",
         "format": "qcow2",
-        "sync": "dirty-bitmap",
+        "sync": "incremental",
         "mode": "existing"
       }
     }
@@ -271,7 +271,7 @@ full backup as a backing image.
         "bitmap": "bitmap0",
         "target": "incremental.0.img",
         "format": "qcow2",
-        "sync": "dirty-bitmap",
+        "sync": "incremental",
         "mode": "existing"
       }
     }
@@ -304,7 +304,7 @@ full backup as a backing image.
         "bitmap": "bitmap0",
         "target": "incremental.0.img",
         "format": "qcow2",
-        "sync": "dirty-bitmap",
+        "sync": "incremental",
         "mode": "existing"
       }
     }
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 5ca5f15..656abcf 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -613,7 +613,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.
+ * @sync_bitmap: The dirty bitmap if sync_mode is MIRROR_SYNC_MODE_INCREMENTAL.
  * @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.
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 8411d4f..0061713 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -534,12 +534,12 @@
 #
 # @none: only copy data written from now on
 #
-# @dirty-bitmap: only copy data described by the dirty bitmap. Since: 2.4
+# @incremental: only copy data described by the dirty bitmap. Since: 2.4
 #
 # Since: 1.3
 ##
 { 'enum': 'MirrorSyncMode',
-  'data': ['top', 'full', 'none', 'dirty-bitmap'] }
+  'data': ['top', 'full', 'none', 'incremental'] }
 
 ##
 # @BlockJobType:
@@ -722,8 +722,8 @@
 #
 # @speed: #optional the maximum speed, in bytes per second
 #
-# @bitmap: #optional the name of dirty bitmap if sync is "dirty-bitmap".
-#          Must be present if sync is "dirty-bitmap", must NOT be present
+# @bitmap: #optional the name of dirty bitmap if sync is "incremental".
+#          Must be present if sync is "incremental", must NOT be present
 #          otherwise. (Since 2.4)
 #
 # @on-source-error: #optional the action to take on an error on the source,
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a6029a2..1e74188 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1137,10 +1137,10 @@ 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, "dirty-bitmap" for only the dirty sectors in
+  allocated in the topmost image, "incremental" for only the dirty sectors in
   the bitmap, or "none" to only replicate new I/O (MirrorSyncMode).
-- "bitmap": dirty bitmap name for sync==dirty-bitmap. Must be present if sync
-            is "dirty-bitmap", must NOT be present otherwise.
+- "bitmap": dirty bitmap name for sync==incremental. Must be present if sync
+            is "incremental", must NOT be present otherwise.
 - "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)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 772edd4..07b1a47 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -207,7 +207,7 @@ class TestIncrementalBackup(iotests.QMPTestCase):
 
         target = self.prepare_backup(bitmap, parent)
         res = self.do_qmp_backup(device=bitmap.drive['id'],
-                                 sync='dirty-bitmap', bitmap=bitmap.name,
+                                 sync='incremental', bitmap=bitmap.name,
                                  format=bitmap.drive['fmt'], target=target,
                                  mode='existing')
         if not res:
@@ -448,10 +448,10 @@ class TestIncrementalBackup(iotests.QMPTestCase):
         # Ask for a new incremental backup per-each drive,
         # expecting drive1's backup to fail:
         transaction = [
-            transaction_drive_backup(drive0['id'], target0, sync='dirty-bitmap',
+            transaction_drive_backup(drive0['id'], target0, sync='incremental',
                                      format=drive0['fmt'], mode='existing',
                                      bitmap=dr0bm0.name),
-            transaction_drive_backup(drive1['id'], target1, sync='dirty-bitmap',
+            transaction_drive_backup(drive1['id'], target1, sync='incremental',
                                      format=drive1['fmt'], mode='existing',
                                      bitmap=dr1bm0.name),
         ]
@@ -497,7 +497,7 @@ class TestIncrementalBackup(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
         self.files.append(self.err_img)
         result = self.vm.qmp('drive-backup', device=self.drives[0]['id'],
-                             sync='dirty-bitmap', format=self.drives[0]['fmt'],
+                             sync='incremental', format=self.drives[0]['fmt'],
                              target=self.err_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
@@ -506,7 +506,7 @@ class TestIncrementalBackup(iotests.QMPTestCase):
         self.assert_no_active_block_jobs()
         self.files.append(self.err_img)
         result = self.vm.qmp('drive-backup', device=self.drives[0]['id'],
-                             sync='dirty-bitmap', bitmap='unknown',
+                             sync='incremental', bitmap='unknown',
                              format=self.drives[0]['fmt'], target=self.err_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
 
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 2/9] hbitmap: add hbitmap_copy
  2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 1/9] qapi: Rename 'dirty-bitmap' mode to 'incremental' John Snow
@ 2015-06-05  0:20 ` John Snow
  2015-06-05  2:37   ` Eric Blake
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 3/9] block: add bdrv_copy_dirty_bitmap John Snow
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: John Snow @ 2015-06-05  0:20 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, John Snow, jcody, qemu-devel

It would be nice to have the flexibility to decide that
we would like multiple backup chains (perhaps of differing
frequency, or stored at different sites -- who knows.)

If the user didn't have the foresight to add all the requisite
bitmaps before the drive was engaged, the copy function will
allow them to later differentiate an incremental backup chain
into two or more chains at will.

hbitmap_copy here is just the primitive to make copies of the
implementation bitmap.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 include/qemu/hbitmap.h |  9 +++++++++
 util/hbitmap.c         | 17 +++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index f0a85f8..e24fbe7 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -88,6 +88,15 @@ void hbitmap_truncate(HBitmap *hb, uint64_t size);
 bool hbitmap_merge(HBitmap *a, const HBitmap *b);
 
 /**
+ * hbitmap_copy:
+ * @hb: The bitmap to copy.
+ * @return The newly copied bitmap.
+ *
+ * Given a bitmap, create a new one with all the same bits set.
+ */
+HBitmap *hbitmap_copy(const HBitmap *hb);
+
+/**
  * hbitmap_empty:
  * @hb: HBitmap to operate on.
  *
diff --git a/util/hbitmap.c b/util/hbitmap.c
index a10c7ae..544ecd5 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -93,6 +93,9 @@ struct HBitmap {
 
     /* The length of each levels[] array. */
     uint64_t sizes[HBITMAP_LEVELS];
+
+    /* NB: If any pointers are introduced into this structure, take care to
+     * update hbitmap_copy() accordingly. */
 };
 
 /* Advance hbi to the next nonzero word and return it.  hbi->pos
@@ -480,3 +483,17 @@ bool hbitmap_merge(HBitmap *a, const HBitmap *b)
 
     return true;
 }
+
+
+HBitmap *hbitmap_copy(const HBitmap *bitmap)
+{
+    int i;
+    HBitmap *hb = g_memdup(bitmap, sizeof(HBitmap));
+
+    for (i = HBITMAP_LEVELS - 1; i >= 0; i--) {
+        hb->levels[i] = g_memdup(bitmap->levels[i],
+                                 hb->sizes[i] * sizeof(unsigned long));
+    }
+
+    return hb;
+}
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 3/9] block: add bdrv_copy_dirty_bitmap
  2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 1/9] qapi: Rename 'dirty-bitmap' mode to 'incremental' John Snow
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 2/9] hbitmap: add hbitmap_copy John Snow
@ 2015-06-05  0:20 ` John Snow
  2015-06-05  2:42   ` Eric Blake
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 4/9] qapi: add Copy data type for bitmaps John Snow
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: John Snow @ 2015-06-05  0:20 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, John Snow, jcody, qemu-devel

One step up from hbitmap, we need a copy primitive for
the BdrvDirtyBitmap type.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 block.c               | 26 ++++++++++++++++++++++++++
 include/block/block.h |  4 ++++
 2 files changed, 30 insertions(+)

diff --git a/block.c b/block.c
index 1eb81ac..5551f79 100644
--- a/block.c
+++ b/block.c
@@ -3114,6 +3114,32 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
     return bitmap;
 }
 
+BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
+                                        const BdrvDirtyBitmap *bitmap,
+                                        const char *name,
+                                        Error **errp)
+{
+    BdrvDirtyBitmap *new_bitmap;
+
+    if (name && bdrv_find_dirty_bitmap(bs, name)) {
+        error_setg(errp, "Bitmap already exists: %s", name);
+        return NULL;
+    }
+
+    new_bitmap = g_new0(BdrvDirtyBitmap, 1);
+    new_bitmap->bitmap = hbitmap_copy(bitmap->bitmap);
+    new_bitmap->size = bitmap->size;
+    new_bitmap->name = g_strdup(name);
+
+    if (bitmap->successor) {
+        /* NB: 2nd arg is read-only. */
+        hbitmap_merge(new_bitmap->bitmap, bitmap->successor->bitmap);
+    }
+
+    QLIST_INSERT_HEAD(&bs->dirty_bitmaps, new_bitmap, list);
+    return new_bitmap;
+}
+
 bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
 {
     return bitmap->successor;
diff --git a/include/block/block.h b/include/block/block.h
index b7892d2..e88a332 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -456,6 +456,10 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
                                           uint32_t granularity,
                                           const char *name,
                                           Error **errp);
+BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
+                                        const BdrvDirtyBitmap *bitmap,
+                                        const char *name,
+                                        Error **errp);
 int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
                                        BdrvDirtyBitmap *bitmap,
                                        Error **errp);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 4/9] qapi: add Copy data type for bitmaps
  2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
                   ` (2 preceding siblings ...)
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 3/9] block: add bdrv_copy_dirty_bitmap John Snow
@ 2015-06-05  0:20 ` John Snow
  2015-06-05  2:57   ` Eric Blake
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 5/9] qmp: add qmp cmd block-dirty-bitmap-copy John Snow
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: John Snow @ 2015-06-05  0:20 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, John Snow, jcody, qemu-devel

We need both a "source" and a "destination" bitmap name,
so a new type is needed to share with the transactional
system in a later patch.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 qapi/block-core.json | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0061713..6bed205 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1000,6 +1000,20 @@
   'data': { 'node': 'str', 'name': 'str' } }
 
 ##
+# @BlockDirtyBitmapCopy
+#
+# @node: name of device/node which the bitmap is tracking
+#
+# @source: name of the dirty bitmap to be copied from
+#
+# @dest: name of the new dirty bitmap to create
+#
+# Since 2.4
+##
+{ 'struct': 'BlockDirtyBitmapCopy',
+  'data': { 'node': 'str', 'source': 'str', 'dest': 'str' } }
+
+##
 # @BlockDirtyBitmapAdd
 #
 # @node: name of device/node which the bitmap is tracking
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 5/9] qmp: add qmp cmd block-dirty-bitmap-copy
  2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
                   ` (3 preceding siblings ...)
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 4/9] qapi: add Copy data type for bitmaps John Snow
@ 2015-06-05  0:20 ` John Snow
  2015-06-05  3:04   ` Eric Blake
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 6/9] qmp: add block-dirty-bitmap-copy transaction John Snow
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: John Snow @ 2015-06-05  0:20 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, John Snow, jcody, qemu-devel

Add the ability to copy one bitmap to a new bitmap.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c           | 22 ++++++++++++++++++++++
 qapi/block-core.json | 16 ++++++++++++++++
 qmp-commands.hx      | 30 ++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 905bf84..9233bcd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2445,6 +2445,28 @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
     aio_context_release(aio_context);
 }
 
+void qmp_block_dirty_bitmap_copy(const char *node, const char *source,
+                                 const char *dest, Error **errp)
+{
+    AioContext *aio_context;
+    BlockDriverState *bs;
+    BdrvDirtyBitmap *bitmap;
+
+    if (!dest || dest[0] == '\0') {
+        error_setg(errp, "Bitmap name cannot be empty");
+        return;
+    }
+
+    bitmap = block_dirty_bitmap_lookup(node, source, &bs, &aio_context, errp);
+    if (!bitmap || !bs) {
+        return;
+    }
+
+    /* Duplicate name checking is left to bdrv_copy_dirty_bitmap */
+    bdrv_copy_dirty_bitmap(bs, bitmap, dest, errp);
+    aio_context_release(aio_context);
+}
+
 void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
                                    Error **errp)
 {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6bed205..92c9e53 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1043,6 +1043,22 @@
   'data': 'BlockDirtyBitmapAdd' }
 
 ##
+# @block-dirty-bitmap-copy
+#
+# Copy a dirty bitmap into a new dirty bitmap
+#
+# Returns: nothing on success
+#          If @node is not a valid block device or node, DeviceNotFound
+#          If @source is not a valid bitmap, GenericError
+#          if @dest is not a valid bitmap name, GenericError
+#          if @dest is already taken, GenericError
+#
+# Since 2.4
+##
+{ 'command': 'block-dirty-bitmap-copy',
+  'data': 'BlockDirtyBitmapCopy' }
+
+##
 # @block-dirty-bitmap-remove
 #
 # Remove a dirty bitmap on the node
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 1e74188..9818b3f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1347,6 +1347,36 @@ Example:
 EQMP
 
     {
+        .name       = "block-dirty-bitmap-copy",
+        .args_type  = "node:B,source:s,dest:s",
+        .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_copy,
+    },
+
+SQMP
+
+block-dirty-bitmap-copy
+-----------------------
+Since 2.4
+
+Copy a dirty bitmap from 'source' to a new bitmap 'dest', then start tracking
+new writes immediately.
+
+Arguments:
+
+- "node": device/node on which to create dirty bitmap (json-string)
+- "source": name of the dirty bitmap to be copied (json-string)
+- "dest": name of the dirty bitmap to be copied (json-string)
+
+Example:
+
+-> { "execute": "block-dirty-bitmap-copy", "arguments": { "node": "drive0",
+                                                          "source": "bitmap0",
+                                                          "dest": "bitmap1" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "block-dirty-bitmap-remove",
         .args_type  = "node:B,name:s",
         .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_remove,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 6/9] qmp: add block-dirty-bitmap-copy transaction
  2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
                   ` (4 preceding siblings ...)
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 5/9] qmp: add qmp cmd block-dirty-bitmap-copy John Snow
@ 2015-06-05  0:20 ` John Snow
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 7/9] block: add differential backup mode John Snow
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: John Snow @ 2015-06-05  0:20 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, John Snow, jcody, qemu-devel

And then add the transaction that allows us to perform this
operation atomically.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 blockdev.c       | 39 +++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |  4 +++-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index 9233bcd..3f9842a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2012,6 +2012,40 @@ static void block_dirty_bitmap_add_abort(BlkActionState *common)
     }
 }
 
+static void block_dirty_bitmap_copy_prepare(BlkActionState *common,
+                                            Error **errp)
+{
+    Error *local_err = NULL;
+    BlockDirtyBitmapCopy *action;
+    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+                                             common, common);
+
+    action = common->action->block_dirty_bitmap_copy;
+    /* AIO context taken (and released) within qmp_block_dirty_bitmap_copy */
+    qmp_block_dirty_bitmap_copy(action->node, action->source,
+                                action->dest, &local_err);
+
+    if (!local_err) {
+        state->prepared = true;
+    } else {
+        error_propagate(errp, local_err);
+    }
+}
+
+static void block_dirty_bitmap_copy_abort(BlkActionState *common)
+{
+    BlockDirtyBitmapCopy *action;
+    BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+                                             common, common);
+
+    action = common->action->block_dirty_bitmap_copy;
+    if (state->prepared) {
+        qmp_block_dirty_bitmap_remove(action->node,
+                                      action->source,
+                                      &error_abort);
+    }
+}
+
 static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
                                              Error **errp)
 {
@@ -2113,6 +2147,11 @@ static const BlkActionOps actions[] = {
         .prepare = block_dirty_bitmap_add_prepare,
         .abort = block_dirty_bitmap_add_abort,
     },
+    [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_COPY] = {
+        .instance_size = sizeof(BlockDirtyBitmapState),
+        .prepare = block_dirty_bitmap_copy_prepare,
+        .abort = block_dirty_bitmap_copy_abort,
+    },
     [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_CLEAR] = {
         .instance_size = sizeof(BlockDirtyBitmapState),
         .prepare = block_dirty_bitmap_clear_prepare,
diff --git a/qapi-schema.json b/qapi-schema.json
index bbd4b3a..89fdd0f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1472,6 +1472,7 @@
 # blockdev-snapshot-internal-sync since 1.7
 # blockdev-backup since 2.3
 # block-dirty-bitmap-add since 2.4
+# block-dirty-bitmap-copy since 2.4
 # block-dirty-bitmap-clear since 2.4
 ##
 { 'union': 'TransactionAction',
@@ -1482,7 +1483,8 @@
        'abort': 'Abort',
        'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
        'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
-       'block-dirty-bitmap-clear': 'BlockDirtyBitmap'
+       'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
+       'block-dirty-bitmap-copy': 'BlockDirtyBitmapCopy'
    } }
 
 ##
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 7/9] block: add differential backup mode
  2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
                   ` (5 preceding siblings ...)
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 6/9] qmp: add block-dirty-bitmap-copy transaction John Snow
@ 2015-06-05  0:20 ` John Snow
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 8/9] iotests: 124: support differential backups John Snow
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: John Snow @ 2015-06-05  0:20 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, John Snow, jcody, qemu-devel

This is simple: instead of clearing the bitmap, just leave the bitmap
data intact even in case of success.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 block.c               |  9 ++++++++-
 block/backup.c        | 17 ++++++++++-------
 block/mirror.c        |  9 +++++++--
 include/block/block.h |  1 +
 qapi/block-core.json  |  6 ++++--
 5 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/block.c b/block.c
index 5551f79..3e780f9 100644
--- a/block.c
+++ b/block.c
@@ -3166,7 +3166,9 @@ DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
  * Requires that the bitmap is not frozen and has no successor.
  */
 int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
-                                       BdrvDirtyBitmap *bitmap, Error **errp)
+                                       BdrvDirtyBitmap *bitmap,
+                                       MirrorSyncMode sync_mode,
+                                       Error **errp)
 {
     uint64_t granularity;
     BdrvDirtyBitmap *child;
@@ -3191,6 +3193,11 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
     /* Install the successor and freeze the parent */
     bitmap->successor = child;
     bitmap->successor_refcount = 1;
+
+    if (sync_mode == MIRROR_SYNC_MODE_DIFFERENTIAL) {
+        bitmap->act = SUCCESSOR_ACTION_RECLAIM;
+    }
+
     return 0;
 }
 
diff --git a/block/backup.c b/block/backup.c
index a8f7c43..dd808c2 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -390,7 +390,8 @@ static void coroutine_fn backup_run(void *opaque)
             qemu_coroutine_yield();
             job->common.busy = true;
         }
-    } else if (job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
+    } else if ((job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) ||
+               (job->sync_mode == MIRROR_SYNC_MODE_DIFFERENTIAL)) {
         ret = backup_run_incremental(job);
     } else {
         /* Both FULL and TOP SYNC_MODE's require copying.. */
@@ -510,15 +511,18 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target,
         return;
     }
 
-    if (sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
+    if ((sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) ||
+        (sync_mode == MIRROR_SYNC_MODE_DIFFERENTIAL)) {
         if (!sync_bitmap) {
-            error_setg(errp, "must provide a valid bitmap name for "
-                             "\"incremental\" sync mode");
+            error_setg(errp,
+                       "must provide a valid bitmap name for \"%s\" sync mode",
+                       MirrorSyncMode_lookup[sync_mode]);
             return;
         }
 
         /* Create a new bitmap, and freeze/disable this one. */
-        if (bdrv_dirty_bitmap_create_successor(bs, sync_bitmap, errp) < 0) {
+        if (bdrv_dirty_bitmap_create_successor(bs, sync_bitmap,
+                                               sync_mode, errp) < 0) {
             return;
         }
     } else if (sync_bitmap) {
@@ -548,8 +552,7 @@ 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_mode == MIRROR_SYNC_MODE_INCREMENTAL ?
-                       sync_bitmap : NULL;
+    job->sync_bitmap = 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 adf391c..1cde86b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -709,9 +709,14 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
     bool is_none_mode;
     BlockDriverState *base;
 
-    if (mode == MIRROR_SYNC_MODE_INCREMENTAL) {
-        error_setg(errp, "Sync mode 'incremental' not supported");
+    switch (mode) {
+    case MIRROR_SYNC_MODE_INCREMENTAL:
+    case MIRROR_SYNC_MODE_DIFFERENTIAL:
+        error_setg(errp, "Sync mode \"%s\" not supported",
+                   MirrorSyncMode_lookup[mode]);
         return;
+    default:
+        break;
     }
     is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
     base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
diff --git a/include/block/block.h b/include/block/block.h
index e88a332..8169a60 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -462,6 +462,7 @@ BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
                                         Error **errp);
 int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
                                        BdrvDirtyBitmap *bitmap,
+                                       MirrorSyncMode sync_mode,
                                        Error **errp);
 BdrvDirtyBitmap *bdrv_frozen_bitmap_decref(BlockDriverState *bs,
                                            BdrvDirtyBitmap *parent,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 92c9e53..421fd25 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -534,12 +534,14 @@
 #
 # @none: only copy data written from now on
 #
-# @incremental: only copy data described by the dirty bitmap. Since: 2.4
+# @incremental: Copy data described by the bitmap, and clear it. Since: 2.4
+#
+# @differential: Copy data described by the bitmap, don't clear it. Since: 2.4
 #
 # Since: 1.3
 ##
 { 'enum': 'MirrorSyncMode',
-  'data': ['top', 'full', 'none', 'incremental'] }
+  'data': ['top', 'full', 'none', 'incremental', 'differential'] }
 
 ##
 # @BlockJobType:
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 8/9] iotests: 124: support differential backups
  2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
                   ` (6 preceding siblings ...)
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 7/9] block: add differential backup mode John Snow
@ 2015-06-05  0:20 ` John Snow
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 9/9] iotests: add differential backup test John Snow
  2015-06-23 17:00 ` [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
  9 siblings, 0 replies; 18+ messages in thread
From: John Snow @ 2015-06-05  0:20 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, John Snow, jcody, qemu-devel

Rekerjigger the helper functions to be able to tolerate
differential backups.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/124 | 69 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 43 insertions(+), 26 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 07b1a47..c446c81 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -61,34 +61,47 @@ class Bitmap:
         self.backups = list()
 
     def base_target(self):
-        return (self.drive['backup'], None)
+        return { 'type': 'full',
+                 'target': self.drive['backup'],
+                 'reference': None }
 
-    def new_target(self, num=None):
+    def new_target(self, num=None, sync='incremental'):
         if num is None:
             num = self.num
         self.num = num + 1
         base = os.path.join(iotests.test_dir,
                             "%s.%s." % (self.drive['id'], self.name))
         suff = "%i.%s" % (num, self.drive['fmt'])
-        target = base + "inc" + suff
+        target = base + sync[:3] + suff
         reference = base + "ref" + suff
-        self.backups.append((target, reference))
-        return (target, reference)
+
+        self.backups.append({ 'type': sync,
+                              'target': target,
+                              'reference': reference })
+        return self.backups[-1]
 
     def last_target(self):
         if self.backups:
             return self.backups[-1]
         return self.base_target()
 
+    def get_backing_file(self):
+        for backup in reversed(self.backups):
+            if backup['type'] != 'differential':
+                return backup['target']
+        return self.base_target()['target']
+
+    def remove_backup(self, backup):
+        try_remove(backup['target'])
+        try_remove(backup['reference'])
+
     def del_target(self):
-        for image in self.backups.pop():
-            try_remove(image)
+        self.remove_backup(self.backups.pop())
         self.num -= 1
 
     def cleanup(self):
         for backup in self.backups:
-            for image in backup:
-                try_remove(image)
+            self.remove_backup(backup)
 
 
 class TestIncrementalBackup(iotests.QMPTestCase):
@@ -172,7 +185,7 @@ class TestIncrementalBackup(iotests.QMPTestCase):
     def make_reference_backup(self, bitmap=None):
         if bitmap is None:
             bitmap = self.bitmaps[-1]
-        _, reference = bitmap.last_target()
+        reference = bitmap.last_target()['reference']
         res = self.do_qmp_backup(device=bitmap.drive['id'], sync='full',
                                  format=bitmap.drive['fmt'], target=reference)
         self.assertTrue(res)
@@ -187,29 +200,28 @@ class TestIncrementalBackup(iotests.QMPTestCase):
         return bitmap
 
 
-    def prepare_backup(self, bitmap=None, parent=None):
+    def prepare_backup(self, bitmap=None, parent=None, sync='incremental'):
         if bitmap is None:
             bitmap = self.bitmaps[-1]
         if parent is None:
-            parent, _ = bitmap.last_target()
+            parent = bitmap.get_backing_file()
 
-        target, _ = bitmap.new_target()
+        target = bitmap.new_target(sync=sync)['target']
         self.img_create(target, bitmap.drive['fmt'], parent=parent)
         return target
 
 
-    def create_incremental(self, bitmap=None, parent=None,
-                           parentFormat=None, validate=True):
+    def create_delta(self, sync='incremental', bitmap=None, parent=None,
+                     parentFormat=None, validate=True):
         if bitmap is None:
             bitmap = self.bitmaps[-1]
         if parent is None:
-            parent, _ = bitmap.last_target()
+            parent = bitmap.get_backing_file()
 
-        target = self.prepare_backup(bitmap, parent)
-        res = self.do_qmp_backup(device=bitmap.drive['id'],
-                                 sync='incremental', bitmap=bitmap.name,
-                                 format=bitmap.drive['fmt'], target=target,
-                                 mode='existing')
+        target = self.prepare_backup(bitmap, parent, sync)
+        res = self.do_qmp_backup(device=bitmap.drive['id'], sync=sync,
+                                 bitmap=bitmap.name, format=bitmap.drive['fmt'],
+                                 target=target, mode='existing')
         if not res:
             bitmap.del_target();
             self.assertFalse(validate)
@@ -217,14 +229,19 @@ class TestIncrementalBackup(iotests.QMPTestCase):
             self.make_reference_backup(bitmap)
         return res
 
+    def create_incremental(self, *args, **kwargs):
+        return self.create_delta('incremental', *args, **kwargs)
+
+    def create_differential(self, *args, **kwargs):
+        return self.create_delta('differential', *args, **kwargs)
 
     def check_backups(self):
         for bitmap in self.bitmaps:
-            for incremental, reference in bitmap.backups:
-                self.assertTrue(iotests.compare_images(incremental, reference))
-            last = bitmap.last_target()[0]
-            self.assertTrue(iotests.compare_images(last, bitmap.drive['file']))
-
+            for backup in bitmap.backups:
+                self.assertTrue(iotests.compare_images(backup['target'],
+                                                       backup['reference']))
+            self.assertTrue(iotests.compare_images(backup['target'],
+                                                   bitmap.drive['file']))
 
     def hmp_io_writes(self, drive, patterns):
         for pattern in patterns:
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH 9/9] iotests: add differential backup test
  2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
                   ` (7 preceding siblings ...)
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 8/9] iotests: 124: support differential backups John Snow
@ 2015-06-05  0:20 ` John Snow
  2015-06-23 17:00 ` [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
  9 siblings, 0 replies; 18+ messages in thread
From: John Snow @ 2015-06-05  0:20 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, John Snow, jcody, qemu-devel

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/124     | 14 ++++++++++----
 tests/qemu-iotests/124.out |  4 ++--
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index c446c81..17e4e6c 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -249,26 +249,28 @@ class TestIncrementalBackup(iotests.QMPTestCase):
         self.vm.hmp_qemu_io(drive, 'flush')
 
 
-    def do_incremental_simple(self, **kwargs):
+    def do_delta_simple(self, create_delta, **kwargs):
         self.create_anchor_backup()
         self.add_bitmap('bitmap0', self.drives[0], **kwargs)
 
         # Sanity: Create a "hollow" incremental backup
-        self.create_incremental()
+        create_delta()
         # Three writes: One complete overwrite, one new segment,
         # and one partial overlap.
         self.hmp_io_writes(self.drives[0]['id'], (('0xab', 0, 512),
                                                   ('0xfe', '16M', '256k'),
                                                   ('0x64', '32736k', '64k')))
-        self.create_incremental()
+        create_delta()
         # Three more writes, one of each kind, like above
         self.hmp_io_writes(self.drives[0]['id'], (('0x9a', 0, 512),
                                                   ('0x55', '8M', '352k'),
                                                   ('0x78', '15872k', '1M')))
-        self.create_incremental()
+        create_delta()
         self.vm.shutdown()
         self.check_backups()
 
+    def do_incremental_simple(self, **kwargs):
+        return self.do_delta_simple(self.create_incremental, **kwargs)
 
     def test_incremental_simple(self):
         '''
@@ -301,6 +303,10 @@ class TestIncrementalBackup(iotests.QMPTestCase):
         return self.do_incremental_simple(granularity=131072)
 
 
+    def test_differential_simple(self):
+        return self.do_delta_simple(self.create_differential)
+
+
     def test_incremental_transaction(self):
         '''Test: Verify backups made from transactionally created bitmaps.
 
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index dae404e..36376be 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-.........
+..........
 ----------------------------------------------------------------------
-Ran 9 tests
+Ran 10 tests
 
 OK
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 1/9] qapi: Rename 'dirty-bitmap' mode to 'incremental'
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 1/9] qapi: Rename 'dirty-bitmap' mode to 'incremental' John Snow
@ 2015-06-05  2:34   ` Eric Blake
  2015-06-25 16:16   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
  1 sibling, 0 replies; 18+ messages in thread
From: Eric Blake @ 2015-06-05  2:34 UTC (permalink / raw)
  To: John Snow, qemu-block; +Cc: kwolf, jcody, qemu-devel

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

On 06/04/2015 06:20 PM, John Snow wrote:
> If we wish to make differential backups a feature that's easy to access,
> it might be pertinent to rename the "dirty-bitmap" mode to "incremental"
> to make it clear what /type/ of backup the dirty-bitmap is helping us
> perform.
> 
> This is an API breaking change, but 2.4 has not yet gone live,
> so we have this flexibility.

Agreed - I like the new name, and you are right that NOW is the time to
do it.

> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  block/backup.c            | 10 +++++-----
>  block/mirror.c            |  4 ++--
>  docs/bitmaps.md           |  8 ++++----
>  include/block/block_int.h |  2 +-
>  qapi/block-core.json      |  8 ++++----
>  qmp-commands.hx           |  6 +++---
>  tests/qemu-iotests/124    | 10 +++++-----
>  7 files changed, 24 insertions(+), 24 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

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


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 2/9] hbitmap: add hbitmap_copy
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 2/9] hbitmap: add hbitmap_copy John Snow
@ 2015-06-05  2:37   ` Eric Blake
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2015-06-05  2:37 UTC (permalink / raw)
  To: John Snow, qemu-block; +Cc: kwolf, jcody, qemu-devel

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

On 06/04/2015 06:20 PM, John Snow wrote:
> It would be nice to have the flexibility to decide that
> we would like multiple backup chains (perhaps of differing
> frequency, or stored at different sites -- who knows.)
> 
> If the user didn't have the foresight to add all the requisite
> bitmaps before the drive was engaged, the copy function will
> allow them to later differentiate an incremental backup chain
> into two or more chains at will.
> 
> hbitmap_copy here is just the primitive to make copies of the
> implementation bitmap.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  include/qemu/hbitmap.h |  9 +++++++++
>  util/hbitmap.c         | 17 +++++++++++++++++
>  2 files changed, 26 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 3/9] block: add bdrv_copy_dirty_bitmap
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 3/9] block: add bdrv_copy_dirty_bitmap John Snow
@ 2015-06-05  2:42   ` Eric Blake
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2015-06-05  2:42 UTC (permalink / raw)
  To: John Snow, qemu-block; +Cc: kwolf, jcody, qemu-devel

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

On 06/04/2015 06:20 PM, John Snow wrote:
> One step up from hbitmap, we need a copy primitive for
> the BdrvDirtyBitmap type.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  block.c               | 26 ++++++++++++++++++++++++++
>  include/block/block.h |  4 ++++
>  2 files changed, 30 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

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


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 4/9] qapi: add Copy data type for bitmaps
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 4/9] qapi: add Copy data type for bitmaps John Snow
@ 2015-06-05  2:57   ` Eric Blake
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2015-06-05  2:57 UTC (permalink / raw)
  To: John Snow, qemu-block; +Cc: kwolf, jcody, qemu-devel

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

On 06/04/2015 06:20 PM, John Snow wrote:
> We need both a "source" and a "destination" bitmap name,
> so a new type is needed to share with the transactional
> system in a later patch.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  qapi/block-core.json | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

Might be okay to squash this with a patch that starts using it; but I'm
also okay leaving it as-is. Up to you.

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


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 5/9] qmp: add qmp cmd block-dirty-bitmap-copy
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 5/9] qmp: add qmp cmd block-dirty-bitmap-copy John Snow
@ 2015-06-05  3:04   ` Eric Blake
  0 siblings, 0 replies; 18+ messages in thread
From: Eric Blake @ 2015-06-05  3:04 UTC (permalink / raw)
  To: John Snow, qemu-block; +Cc: kwolf, jcody, qemu-devel

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

On 06/04/2015 06:20 PM, John Snow wrote:
> Add the ability to copy one bitmap to a new bitmap.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  blockdev.c           | 22 ++++++++++++++++++++++
>  qapi/block-core.json | 16 ++++++++++++++++
>  qmp-commands.hx      | 30 ++++++++++++++++++++++++++++++
>  3 files changed, 68 insertions(+)

[could be merged with 4]


>  
> +void qmp_block_dirty_bitmap_copy(const char *node, const char *source,
> +                                 const char *dest, Error **errp)
> +{
> +    AioContext *aio_context;
> +    BlockDriverState *bs;
> +    BdrvDirtyBitmap *bitmap;
> +
> +    if (!dest || dest[0] == '\0') {

qapi doesn't allow NULL for a mandatory option, so !dest is currently
dead code.  Of course, someday I'd like to get rid of have_FOO arguments
for pointer types, with NULL possible on optional parameters, but that's
not happening any time soon.

But it's small enough, even if you leave it in, that I don't mind giving:

Reviewed-by: Eric Blake <eblake@redhat.com>

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


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 0/9] block: add differential backup support
  2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
                   ` (8 preceding siblings ...)
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 9/9] iotests: add differential backup test John Snow
@ 2015-06-23 17:00 ` John Snow
  2015-06-24 14:33   ` Stefan Hajnoczi
  9 siblings, 1 reply; 18+ messages in thread
From: John Snow @ 2015-06-23 17:00 UTC (permalink / raw)
  To: qemu-block
  Cc: kwolf, jcody, Vladimir Sementsov-Ogievskiy, qemu-devel, Stefan Hajnoczi

Ping.

(Oh, I forgot to CC Stefan originally. :\)

Stefan: I know you said this wasn't expressly requisite, but I was
encouraged by Kevin to send it out as a usability patch for non-libvirt
users, so here it is.

Final "Yeah sure" or "No, definitely not" as 2.4 approaches?

Also note that if the answer is "Ehhh, maybe not now, nobody's REALLY
asked for it" we should still probably merge the first patch alone,
before our API solidifies -- this will give us the breathing room to add
these features in later if we want.

Thank you,
--John

(Oh, also note that this IS based on the now deprecated transactional
series I wrote, but if there is some use I can likely rebase without
much of a fuss on top of the BlockJobTxn approach.)


On 06/04/2015 08:20 PM, John Snow wrote:
> Requires: 1433454372-16356-1-git-send-email-jsnow@redhat.com
>           [0/10] block: incremental backup transactions
> 
> It's entirely possible to use the incremental backup primitives to
> achieve a differential backup mechanism, but in the interest of
> ease of use, I am proposing the explicit addition of the mechanism
> because it does not particularly complicate the code, add new edge
> cases, or present itself as difficult to test.
> 
> This series actually adds two ease of use features:
> 
> (1) Add a copy primitive for bitmaps to add flexibility to the
>     backup system in case users would like to later run multiple
>     backup chains (weekly vs. monthly or perhaps incremental vs.
>     differential)
> 
> (2) Add a 'differential' backup mode that does what the name says
>     on the tin.
> 
> ==
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch differential-backup
> https://github.com/jnsnow/qemu/tree/differential-backup
> 
> This version is tagged differential-backup-v1:
> https://github.com/jnsnow/qemu/releases/tag/differential-backup-v1
> ==
> 
> John Snow (9):
>   qapi: Rename 'dirty-bitmap' mode to 'incremental'
>   hbitmap: add hbitmap_copy
>   block: add bdrv_copy_dirty_bitmap
>   qapi: add Copy data type for bitmaps
>   qmp: add qmp cmd block-dirty-bitmap-copy
>   qmp: add block-dirty-bitmap-copy transaction
>   block: add differential backup mode
>   iotests: 124: support differential backups
>   iotests: add differential backup test
> 
>  block.c                    | 35 +++++++++++++++++-
>  block/backup.c             | 19 ++++++----
>  block/mirror.c             |  9 ++++-
>  blockdev.c                 | 61 +++++++++++++++++++++++++++++++
>  docs/bitmaps.md            |  8 ++--
>  include/block/block.h      |  5 +++
>  include/block/block_int.h  |  2 +-
>  include/qemu/hbitmap.h     |  9 +++++
>  qapi-schema.json           |  4 +-
>  qapi/block-core.json       | 40 ++++++++++++++++++--
>  qmp-commands.hx            | 36 ++++++++++++++++--
>  tests/qemu-iotests/124     | 91 +++++++++++++++++++++++++++++-----------------
>  tests/qemu-iotests/124.out |  4 +-
>  util/hbitmap.c             | 17 +++++++++
>  14 files changed, 280 insertions(+), 60 deletions(-)
> 

-- 
—js

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH 0/9] block: add differential backup support
  2015-06-23 17:00 ` [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
@ 2015-06-24 14:33   ` Stefan Hajnoczi
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2015-06-24 14:33 UTC (permalink / raw)
  To: John Snow
  Cc: kwolf, jcody, Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block

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

On Tue, Jun 23, 2015 at 01:00:44PM -0400, John Snow wrote:
> Stefan: I know you said this wasn't expressly requisite, but I was
> encouraged by Kevin to send it out as a usability patch for non-libvirt
> users, so here it is.
> 
> Final "Yeah sure" or "No, definitely not" as 2.4 approaches?
> 
> Also note that if the answer is "Ehhh, maybe not now, nobody's REALLY
> asked for it" we should still probably merge the first patch alone,
> before our API solidifies -- this will give us the breathing room to add
> these features in later if we want.

I'll take a look at the first patch.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH 1/9] qapi: Rename 'dirty-bitmap' mode to 'incremental'
  2015-06-05  0:20 ` [Qemu-devel] [PATCH 1/9] qapi: Rename 'dirty-bitmap' mode to 'incremental' John Snow
  2015-06-05  2:34   ` Eric Blake
@ 2015-06-25 16:16   ` Stefan Hajnoczi
  1 sibling, 0 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2015-06-25 16:16 UTC (permalink / raw)
  To: John Snow; +Cc: qemu-devel, qemu-block

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

On Thu, Jun 04, 2015 at 08:20:34PM -0400, John Snow wrote:
> If we wish to make differential backups a feature that's easy to access,
> it might be pertinent to rename the "dirty-bitmap" mode to "incremental"
> to make it clear what /type/ of backup the dirty-bitmap is helping us
> perform.
> 
> This is an API breaking change, but 2.4 has not yet gone live,
> so we have this flexibility.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  block/backup.c            | 10 +++++-----
>  block/mirror.c            |  4 ++--
>  docs/bitmaps.md           |  8 ++++----
>  include/block/block_int.h |  2 +-
>  qapi/block-core.json      |  8 ++++----
>  qmp-commands.hx           |  6 +++---
>  tests/qemu-iotests/124    | 10 +++++-----
>  7 files changed, 24 insertions(+), 24 deletions(-)

Thanks, applied this single patch to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-06-25 16:16 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-05  0:20 [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
2015-06-05  0:20 ` [Qemu-devel] [PATCH 1/9] qapi: Rename 'dirty-bitmap' mode to 'incremental' John Snow
2015-06-05  2:34   ` Eric Blake
2015-06-25 16:16   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2015-06-05  0:20 ` [Qemu-devel] [PATCH 2/9] hbitmap: add hbitmap_copy John Snow
2015-06-05  2:37   ` Eric Blake
2015-06-05  0:20 ` [Qemu-devel] [PATCH 3/9] block: add bdrv_copy_dirty_bitmap John Snow
2015-06-05  2:42   ` Eric Blake
2015-06-05  0:20 ` [Qemu-devel] [PATCH 4/9] qapi: add Copy data type for bitmaps John Snow
2015-06-05  2:57   ` Eric Blake
2015-06-05  0:20 ` [Qemu-devel] [PATCH 5/9] qmp: add qmp cmd block-dirty-bitmap-copy John Snow
2015-06-05  3:04   ` Eric Blake
2015-06-05  0:20 ` [Qemu-devel] [PATCH 6/9] qmp: add block-dirty-bitmap-copy transaction John Snow
2015-06-05  0:20 ` [Qemu-devel] [PATCH 7/9] block: add differential backup mode John Snow
2015-06-05  0:20 ` [Qemu-devel] [PATCH 8/9] iotests: 124: support differential backups John Snow
2015-06-05  0:20 ` [Qemu-devel] [PATCH 9/9] iotests: add differential backup test John Snow
2015-06-23 17:00 ` [Qemu-devel] [PATCH 0/9] block: add differential backup support John Snow
2015-06-24 14:33   ` Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.