diff mbox

[v9,07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup

Message ID 1417465816-19345-8-git-send-email-jsnow@redhat.com
State New
Headers show

Commit Message

John Snow Dec. 1, 2014, 8:30 p.m. UTC
From: Fam Zheng <famz@redhat.com>

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

There are two bitmap use modes for sync=dirty-bitmap:

 - reset: backup job makes a copy of bitmap and resets the original
   one.
 - consume: backup job makes the original anonymous (invisible to user)
   and releases it after use.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block.c                   |   5 ++
 block/backup.c            | 130 ++++++++++++++++++++++++++++++++++++++--------
 block/mirror.c            |   4 ++
 blockdev.c                |  17 +++++-
 hmp.c                     |   4 +-
 include/block/block.h     |   1 +
 include/block/block_int.h |   6 +++
 qapi/block-core.json      |  30 +++++++++--
 qmp-commands.hx           |   7 +--
 9 files changed, 174 insertions(+), 30 deletions(-)

Comments

Stefan Hajnoczi Dec. 9, 2014, 5:44 p.m. UTC | #1
On Mon, Dec 01, 2014 at 03:30:13PM -0500, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
> 
> For "dirty-bitmap" sync mode, the block job will iterate through the
> given dirty bitmap to decide if a sector needs backup (backup all the
> dirty clusters and skip clean ones), just as allocation conditions of
> "top" sync mode.
> 
> There are two bitmap use modes for sync=dirty-bitmap:
> 
>  - reset: backup job makes a copy of bitmap and resets the original
>    one.
>  - consume: backup job makes the original anonymous (invisible to user)
>    and releases it after use.

It's not obvious to me that we need both modes.  Can you explain why the
choice between reset and consume is necessary?

> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c                   |   5 ++
>  block/backup.c            | 130 ++++++++++++++++++++++++++++++++++++++--------
>  block/mirror.c            |   4 ++
>  blockdev.c                |  17 +++++-
>  hmp.c                     |   4 +-
>  include/block/block.h     |   1 +
>  include/block/block_int.h |   6 +++
>  qapi/block-core.json      |  30 +++++++++--
>  qmp-commands.hx           |   7 +--
>  9 files changed, 174 insertions(+), 30 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 85215b3..42244f6 100644
> --- a/block.c
> +++ b/block.c
> @@ -5489,6 +5489,11 @@ void bdrv_dirty_iter_init(BlockDriverState *bs,
>      hbitmap_iter_init(hbi, bitmap->bitmap, 0);
>  }
>  
> +void bdrv_dirty_iter_set(HBitmapIter *hbi, int64_t offset)
> +{
> +    hbitmap_iter_init(hbi, hbi->hb, offset);
> +}
> +
>  void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
>                      int nr_sectors)
>  {
> diff --git a/block/backup.c b/block/backup.c
> index 792e655..2aab68f 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -37,6 +37,10 @@ typedef struct CowRequest {
>  typedef struct BackupBlockJob {
>      BlockJob common;
>      BlockDriverState *target;
> +    /* bitmap for sync=dirty-bitmap */
> +    BdrvDirtyBitmap *sync_bitmap;
> +    /* dirty bitmap granularity */
> +    int64_t sync_bitmap_gran;

What is the point of this field?

I think we've duplicated granularity twice now (originally stored in
HBitmap, once in BdrvDirtyBitmap, and now again).

Maybe the one user of this field should just call
bdrv_dirty_bitmap_granularity().

>      MirrorSyncMode sync_mode;
>      RateLimit limit;
>      BlockdevOnError on_source_error;
> @@ -242,6 +246,31 @@ static void backup_complete(BlockJob *job, void *opaque)
>      g_free(data);
>  }
>  
> +static bool yield_and_check(BackupBlockJob *job)

missing coroutine_fn annotation.  coroutine_fn tells the reader that
this function may only be called from coroutine context.

(In this case the function and contains "yield" so that's a big hint but
please still always mark coroutine functions coroutine_fn so that we can
enforce static checking in the future.)
Fam Zheng Dec. 10, 2014, 1:34 a.m. UTC | #2
On Tue, 12/09 17:44, Stefan Hajnoczi wrote:
> On Mon, Dec 01, 2014 at 03:30:13PM -0500, John Snow wrote:
> > From: Fam Zheng <famz@redhat.com>
> > 
> > For "dirty-bitmap" sync mode, the block job will iterate through the
> > given dirty bitmap to decide if a sector needs backup (backup all the
> > dirty clusters and skip clean ones), just as allocation conditions of
> > "top" sync mode.
> > 
> > There are two bitmap use modes for sync=dirty-bitmap:
> > 
> >  - reset: backup job makes a copy of bitmap and resets the original
> >    one.
> >  - consume: backup job makes the original anonymous (invisible to user)
> >    and releases it after use.
> 
> It's not obvious to me that we need both modes.  Can you explain why the
> choice between reset and consume is necessary?

Reset is used in continuous incremental backup, which is obvious: user can
track the new data that comes after this drive-backup starts.

Consume is better when user has no intention to use this dirty bitmap
afterward. It comes with more convenience and less overhead: no need to
explicitly free the bitmap, and no need for drive-backup job to copy the dirty
bitmap.

Copy shouldn't be too slow if it's just in memory, but it does require
2x memory anyway.

Alternatively these can all be implemented with transactions.

Fam
Stefan Hajnoczi Dec. 12, 2014, 10:58 a.m. UTC | #3
On Wed, Dec 10, 2014 at 09:34:00AM +0800, Fam Zheng wrote:
> On Tue, 12/09 17:44, Stefan Hajnoczi wrote:
> > On Mon, Dec 01, 2014 at 03:30:13PM -0500, John Snow wrote:
> > > From: Fam Zheng <famz@redhat.com>
> > > 
> > > For "dirty-bitmap" sync mode, the block job will iterate through the
> > > given dirty bitmap to decide if a sector needs backup (backup all the
> > > dirty clusters and skip clean ones), just as allocation conditions of
> > > "top" sync mode.
> > > 
> > > There are two bitmap use modes for sync=dirty-bitmap:
> > > 
> > >  - reset: backup job makes a copy of bitmap and resets the original
> > >    one.
> > >  - consume: backup job makes the original anonymous (invisible to user)
> > >    and releases it after use.
> > 
> > It's not obvious to me that we need both modes.  Can you explain why the
> > choice between reset and consume is necessary?
> 
> Reset is used in continuous incremental backup, which is obvious: user can
> track the new data that comes after this drive-backup starts.
> 
> Consume is better when user has no intention to use this dirty bitmap
> afterward. It comes with more convenience and less overhead: no need to
> explicitly free the bitmap, and no need for drive-backup job to copy the dirty
> bitmap.
> 
> Copy shouldn't be too slow if it's just in memory, but it does require
> 2x memory anyway.
> 
> Alternatively these can all be implemented with transactions.

I'm concerned about how error cases and live migration will be handled.

It is important not to lose the contents of the dirty bitmap until
backup has completed successfully.

If there is an I/O error, what happens?  While in error state can QEMU
be shut down or can live migration take place?

I prefer if we have just one mode that is fully tested and thought
through.

Stefan
diff mbox

Patch

diff --git a/block.c b/block.c
index 85215b3..42244f6 100644
--- a/block.c
+++ b/block.c
@@ -5489,6 +5489,11 @@  void bdrv_dirty_iter_init(BlockDriverState *bs,
     hbitmap_iter_init(hbi, bitmap->bitmap, 0);
 }
 
+void bdrv_dirty_iter_set(HBitmapIter *hbi, int64_t offset)
+{
+    hbitmap_iter_init(hbi, hbi->hb, offset);
+}
+
 void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
                     int nr_sectors)
 {
diff --git a/block/backup.c b/block/backup.c
index 792e655..2aab68f 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -37,6 +37,10 @@  typedef struct CowRequest {
 typedef struct BackupBlockJob {
     BlockJob common;
     BlockDriverState *target;
+    /* bitmap for sync=dirty-bitmap */
+    BdrvDirtyBitmap *sync_bitmap;
+    /* dirty bitmap granularity */
+    int64_t sync_bitmap_gran;
     MirrorSyncMode sync_mode;
     RateLimit limit;
     BlockdevOnError on_source_error;
@@ -242,6 +246,31 @@  static void backup_complete(BlockJob *job, void *opaque)
     g_free(data);
 }
 
+static bool yield_and_check(BackupBlockJob *job)
+{
+    if (block_job_is_cancelled(&job->common)) {
+        return true;
+    }
+
+    /* we need to yield so that qemu_aio_flush() returns.
+     * (without, VM does not reboot)
+     */
+    if (job->common.speed) {
+        uint64_t delay_ns = ratelimit_calculate_delay(&job->limit,
+                                                      job->sectors_read);
+        job->sectors_read = 0;
+        block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns);
+    } else {
+        block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, 0);
+    }
+
+    if (block_job_is_cancelled(&job->common)) {
+        return true;
+    }
+
+    return false;
+}
+
 static void coroutine_fn backup_run(void *opaque)
 {
     BackupBlockJob *job = opaque;
@@ -254,13 +283,13 @@  static void coroutine_fn backup_run(void *opaque)
     };
     int64_t start, end;
     int ret = 0;
+    bool error_is_read;
 
     QLIST_INIT(&job->inflight_reqs);
     qemu_co_rwlock_init(&job->flush_rwlock);
 
     start = 0;
-    end = DIV_ROUND_UP(job->common.len / BDRV_SECTOR_SIZE,
-                       BACKUP_SECTORS_PER_CLUSTER);
+    end = DIV_ROUND_UP(job->common.len, BACKUP_CLUSTER_SIZE);
 
     job->bitmap = hbitmap_alloc(end, 0);
 
@@ -278,28 +307,44 @@  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) {
+        /* Dirty Bitmap sync has a slightly different iteration method */
+        HBitmapIter hbi;
+        int64_t sector;
+        int64_t cluster;
+        bool polyrhythmic;
+
+        bdrv_dirty_iter_init(bs, job->sync_bitmap, &hbi);
+        /* Does the granularity happen to match our backup cluster size? */
+        polyrhythmic = (job->sync_bitmap_gran != BACKUP_CLUSTER_SIZE);
+
+        /* Find the next dirty /sector/ and copy that /cluster/ */
+        while ((sector = hbitmap_iter_next(&hbi)) != -1) {
+            if (yield_and_check(job)) {
+                goto leave;
+            }
+            cluster = sector / BACKUP_SECTORS_PER_CLUSTER;
+
+            do {
+                ret = backup_do_cow(bs, cluster * BACKUP_SECTORS_PER_CLUSTER,
+                                    BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
+                if ((ret < 0) &&
+                    backup_error_action(job, error_is_read, -ret) ==
+                    BLOCK_ERROR_ACTION_REPORT) {
+                    goto leave;
+                }
+            } while (ret < 0);
+
+            /* Advance (or rewind) our iterator if we need to. */
+            if (polyrhythmic) {
+                bdrv_dirty_iter_set(&hbi,
+                                    (cluster + 1) * BACKUP_SECTORS_PER_CLUSTER);
+            }
+        }
     } else {
         /* Both FULL and TOP SYNC_MODE's require copying.. */
         for (; start < end; start++) {
-            bool error_is_read;
-
-            if (block_job_is_cancelled(&job->common)) {
-                break;
-            }
-
-            /* we need to yield so that qemu_aio_flush() returns.
-             * (without, VM does not reboot)
-             */
-            if (job->common.speed) {
-                uint64_t delay_ns = ratelimit_calculate_delay(
-                        &job->limit, job->sectors_read);
-                job->sectors_read = 0;
-                block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns);
-            } else {
-                block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, 0);
-            }
-
-            if (block_job_is_cancelled(&job->common)) {
+            if (yield_and_check(job)) {
                 break;
             }
 
@@ -351,12 +396,16 @@  static void coroutine_fn backup_run(void *opaque)
         }
     }
 
+leave:
     notifier_with_return_remove(&before_write);
 
     /* wait until pending backup_do_cow() calls have completed */
     qemu_co_rwlock_wrlock(&job->flush_rwlock);
     qemu_co_rwlock_unlock(&job->flush_rwlock);
 
+    if (job->sync_bitmap) {
+        bdrv_release_dirty_bitmap(bs, job->sync_bitmap);
+    }
     hbitmap_free(job->bitmap);
 
     bdrv_iostatus_disable(target);
@@ -368,12 +417,15 @@  static void coroutine_fn backup_run(void *opaque)
 
 void backup_start(BlockDriverState *bs, BlockDriverState *target,
                   int64_t speed, MirrorSyncMode sync_mode,
+                  BdrvDirtyBitmap *sync_bitmap,
+                  BitmapUseMode bitmap_mode,
                   BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
                   BlockCompletionFunc *cb, void *opaque,
                   Error **errp)
 {
     int64_t len;
+    BdrvDirtyBitmap *original;
 
     assert(bs);
     assert(target);
@@ -386,6 +438,36 @@  void backup_start(BlockDriverState *bs, BlockDriverState *target,
         return;
     }
 
+    if (sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+        if (!sync_bitmap) {
+            error_setg(errp, "must provide a valid bitmap name for "
+                             "\"dirty-bitmap\" sync mode");
+            return;
+        }
+
+        switch (bitmap_mode) {
+        case BITMAP_USE_MODE_RESET:
+            original = sync_bitmap;
+            sync_bitmap = bdrv_copy_dirty_bitmap(bs, sync_bitmap, NULL);
+            bdrv_reset_dirty_bitmap(bs, original);
+            break;
+        case BITMAP_USE_MODE_CONSUME:
+            bdrv_dirty_bitmap_make_anon(bs, sync_bitmap);
+            break;
+        default:
+            error_setg(errp, "Invalid BitmapUseMode (%s) given to backup_start",
+                       BitmapUseMode_lookup[bitmap_mode]);
+            return;
+        }
+        bdrv_disable_dirty_bitmap(sync_bitmap);
+    } else if (sync_bitmap) {
+        error_setg(errp,
+                   "a sync_bitmap was provided to backup_run, "
+                   "but received an incompatible sync_mode (%s)",
+                   BitmapUseMode_lookup[sync_mode]);
+        return;
+    }
+
     len = bdrv_getlength(bs);
     if (len < 0) {
         error_setg_errno(errp, -len, "unable to get length for '%s'",
@@ -403,6 +485,12 @@  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 ?
+                       sync_bitmap : NULL;
+    if (sync_bitmap) {
+        job->sync_bitmap_gran =
+            bdrv_dirty_bitmap_granularity(bs, job->sync_bitmap);
+    }
     job->common.len = len;
     job->common.co = qemu_coroutine_create(backup_run);
     qemu_coroutine_enter(job->common.co, job);
diff --git a/block/mirror.c b/block/mirror.c
index 3633632..af91ae0 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -714,6 +714,10 @@  void mirror_start(BlockDriverState *bs, BlockDriverState *target,
     bool is_none_mode;
     BlockDriverState *base;
 
+    if (mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+        error_setg(errp, "Sync mode 'dirty-bitmap' not supported");
+        return;
+    }
     is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
     base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
     mirror_start_job(bs, target, replaces,
diff --git a/blockdev.c b/blockdev.c
index c6b0bf1..3ab3404 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1562,6 +1562,8 @@  static void drive_backup_prepare(BlkTransactionState *common, Error **errp)
                      backup->sync,
                      backup->has_mode, backup->mode,
                      backup->has_speed, backup->speed,
+                     backup->has_bitmap, backup->bitmap,
+                     backup->has_bitmap_use_mode, backup->bitmap_use_mode,
                      backup->has_on_source_error, backup->on_source_error,
                      backup->has_on_target_error, backup->on_target_error,
                      &local_err);
@@ -2319,6 +2321,8 @@  void qmp_drive_backup(const char *device, const char *target,
                       enum MirrorSyncMode sync,
                       bool has_mode, enum NewImageMode mode,
                       bool has_speed, int64_t speed,
+                      bool has_bitmap, const char *bitmap,
+                      bool has_bitmap_use_mode, enum BitmapUseMode bitmap_mode,
                       bool has_on_source_error, BlockdevOnError on_source_error,
                       bool has_on_target_error, BlockdevOnError on_target_error,
                       Error **errp)
@@ -2326,6 +2330,7 @@  void qmp_drive_backup(const char *device, const char *target,
     BlockDriverState *bs;
     BlockDriverState *target_bs;
     BlockDriverState *source = NULL;
+    BdrvDirtyBitmap *bmap = NULL;
     AioContext *aio_context;
     BlockDriver *drv = NULL;
     Error *local_err = NULL;
@@ -2421,7 +2426,17 @@  void qmp_drive_backup(const char *device, const char *target,
 
     bdrv_set_aio_context(target_bs, aio_context);
 
-    backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
+    if (has_bitmap) {
+        bmap = bdrv_find_dirty_bitmap(bs, bitmap);
+        if (!bmap) {
+            error_setg(errp, "Bitmap '%s' could not be found", bitmap);
+            goto out;
+        }
+    }
+
+    backup_start(bs, target_bs, speed, sync, bmap,
+                 has_bitmap_use_mode ? bitmap_mode : BITMAP_USE_MODE_RESET,
+                 on_source_error, on_target_error,
                  block_job_cb, bs, &local_err);
     if (local_err != NULL) {
         bdrv_unref(target_bs);
diff --git a/hmp.c b/hmp.c
index 481be80..17a2a2b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1021,7 +1021,9 @@  void hmp_drive_backup(Monitor *mon, const QDict *qdict)
 
     qmp_drive_backup(device, filename, !!format, format,
                      full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
-                     true, mode, false, 0, false, 0, false, 0, &err);
+                     true, mode, false, 0, false, NULL,
+                     false, 0,
+                     false, 0, false, 0, &err);
     hmp_handle_error(mon, &err);
 }
 
diff --git a/include/block/block.h b/include/block/block.h
index b583457..b861433 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -453,6 +453,7 @@  void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
 void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors);
 void bdrv_dirty_iter_init(BlockDriverState *bs,
                           BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi);
+void bdrv_dirty_iter_set(struct HBitmapIter *hbi, int64_t offset);
 int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
 
 void bdrv_enable_copy_on_read(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 192fce8..ae4b475 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -576,6 +576,10 @@  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.
+ * @bitmap_mode: BITMAP_USE_MODE_{CONSUME, RESET}
+ *               Reset: Make a copy and reset the original bitmap.
+ *               Consume: Anonymize the bitmap and free it after completion.
  * @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.
@@ -586,6 +590,8 @@  void mirror_start(BlockDriverState *bs, BlockDriverState *target,
  */
 void backup_start(BlockDriverState *bs, BlockDriverState *target,
                   int64_t speed, MirrorSyncMode sync_mode,
+                  BdrvDirtyBitmap *sync_bitmap,
+                  BitmapUseMode bitmap_mode,
                   BlockdevOnError on_source_error,
                   BlockdevOnError on_target_error,
                   BlockCompletionFunc *cb, void *opaque,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 64b5755..04f9824 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -500,7 +500,7 @@ 
 # Since: 1.3
 ##
 { 'enum': 'MirrorSyncMode',
-  'data': ['top', 'full', 'none'] }
+  'data': ['top', 'full', 'none', 'dirty-bitmap'] }
 
 ##
 # @BlockJobType:
@@ -663,6 +663,21 @@ 
             '*format': 'str', '*mode': 'NewImageMode' } }
 
 ##
+# @BitmapUseMode
+#
+# An enumeration that tells QEMU what operation to take when using a bitmap
+# in drive backup sync mode dirty-bitmap.
+#
+# @consume: QEMU should just consume the bitmap and release it after using
+#
+# @reset: QEMU should reset the dirty bitmap
+#
+# Since: 2.3
+##
+{ 'enum': 'BitmapUseMode',
+'data': [ 'consume', 'reset' ] }
+
+##
 # @DriveBackup
 #
 # @device: the name of the device which should be copied.
@@ -675,14 +690,20 @@ 
 #          probe if @mode is 'existing', else the format of the source
 #
 # @sync: what parts of the disk image should be copied to the destination
-#        (all the disk, only the sectors allocated in the topmost image, or
-#        only new I/O).
+#        (all the disk, only the sectors allocated in the topmost image, from a
+#        dirty bitmap, or only new I/O).
 #
 # @mode: #optional whether and how QEMU should create a new image, default is
 #        'absolute-paths'.
 #
 # @speed: #optional the maximum speed, in bytes per second
 #
+# @bitmap: #optional the name of dirty bitmap if sync is "dirty-bitmap"
+#          (Since 2.3)
+#
+# @bitmap-use-mode: #optional which operation to take when consuming @bitmap,
+#                   default is reset. (Since 2.3)
+#
 # @on-source-error: #optional the action to take on an error on the source,
 #                   default 'report'.  'stop' and 'enospc' can only be used
 #                   if the block device supports io-status (see BlockInfo).
@@ -700,7 +721,8 @@ 
 { 'type': 'DriveBackup',
   'data': { 'device': 'str', 'target': 'str', '*format': 'str',
             'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
-            '*speed': 'int',
+            '*speed': 'int', '*bitmap': 'str',
+            '*bitmap-use-mode': 'BitmapUseMode',
             '*on-source-error': 'BlockdevOnError',
             '*on-target-error': 'BlockdevOnError' } }
 
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9a69633..1eba583 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1048,7 +1048,7 @@  EQMP
     {
         .name       = "drive-backup",
         .args_type  = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
-                      "on-source-error:s?,on-target-error:s?",
+                      "bitmap:s?,on-source-error:s?,on-target-error:s?",
         .mhandler.cmd_new = qmp_marshal_input_drive_backup,
     },
 
@@ -1075,8 +1075,9 @@  Arguments:
             (json-string, optional)
 - "sync": what parts of the disk image should be copied to the destination;
   possibilities include "full" for all the disk, "top" for only the sectors
-  allocated in the topmost image, or "none" to only replicate new I/O
-  (MirrorSyncMode).
+  allocated in the topmost image, "dirty-bitmap" for only the dirty sectors in
+  the bitmap, or "none" to only replicate new I/O (MirrorSyncMode).
+- "bitmap": dirty bitmap name for sync==dirty-bitmap
 - "mode": whether and how QEMU should create a new image
           (NewImageMode, optional, default 'absolute-paths')
 - "speed": the maximum speed, in bytes per second (json-int, optional)