* [Qemu-devel] [PATCH v6 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
2014-10-30 3:22 [Qemu-devel] [PATCH v6 00/10] block: Incremental backup series Fam Zheng
@ 2014-10-30 3:22 ` Fam Zheng
2014-11-04 10:53 ` Max Reitz
0 siblings, 1 reply; 3+ messages in thread
From: Fam Zheng @ 2014-10-30 3:22 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Benoit Canet, Vladimir Sementsov-Ogievskij,
Markus Armbruster, Max Reitz, John Snow, Stefan Hajnoczi, Jd,
Paolo Bonzini, Luiz Capitulino
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>
---
block/backup.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++-
block/mirror.c | 4 ++++
blockdev.c | 9 +++++++-
hmp.c | 4 +++-
include/block/block_int.h | 4 ++++
qapi/block-core.json | 30 ++++++++++++++++++++++----
qmp-commands.hx | 7 +++---
7 files changed, 102 insertions(+), 10 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index e334740..4870694 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 */
+ int sync_bitmap_gran;
MirrorSyncMode sync_mode;
RateLimit limit;
BlockdevOnError on_source_error;
@@ -263,7 +267,7 @@ static void coroutine_fn backup_run(void *opaque)
job->common.busy = true;
}
} else {
- /* Both FULL and TOP SYNC_MODE's require copying.. */
+ /* full, top and dirty_bitmap modes require copying.. */
for (; start < end; start++) {
bool error_is_read;
@@ -317,7 +321,21 @@ static void coroutine_fn backup_run(void *opaque)
if (alloced == 0) {
continue;
}
+ } else if (job->sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
+ int i, dirty = 0;
+ for (i = 0; i < BACKUP_SECTORS_PER_CLUSTER;
+ i += job->sync_bitmap_gran) {
+ if (bdrv_get_dirty(bs, job->sync_bitmap,
+ start * BACKUP_SECTORS_PER_CLUSTER + i)) {
+ dirty = 1;
+ break;
+ }
+ }
+ if (!dirty) {
+ continue;
+ }
}
+
/* FULL sync mode we copy the whole drive. */
ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER,
BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
@@ -341,6 +359,9 @@ static void coroutine_fn backup_run(void *opaque)
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);
@@ -351,12 +372,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);
@@ -369,6 +393,28 @@ 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:
+ assert(0);
+ }
+ bdrv_disable_dirty_bitmap(bs, sync_bitmap);
+ }
+
len = bdrv_getlength(bs);
if (len < 0) {
error_setg_errno(errp, -len, "unable to get length for '%s'",
@@ -386,6 +432,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 cecd448..8dfd908 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -679,6 +679,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 c7e98ad..fca909b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1463,6 +1463,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);
@@ -2186,6 +2188,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)
@@ -2282,7 +2286,10 @@ void qmp_drive_backup(const char *device, const char *target,
return;
}
- backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
+ backup_start(bs, target_bs, speed, sync,
+ has_bitmap ? bdrv_find_dirty_bitmap(bs, bitmap) : NULL,
+ 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 63d7686..bc5a2d2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -966,7 +966,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_int.h b/include/block/block_int.h
index 8898c6c..0671c89 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -567,6 +567,8 @@ 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 name of dirty bitmap if sync_mode is
+ * MIRROR_SYNC_MODE_DIRTY_BITMAP.
* @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.
@@ -577,6 +579,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 97c31bf..e225220 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -473,7 +473,7 @@
# Since: 1.3
##
{ 'enum': 'MirrorSyncMode',
- 'data': ['top', 'full', 'none'] }
+ 'data': ['top', 'full', 'none', 'dirty-bitmap'] }
##
# @BlockJobType:
@@ -634,6 +634,21 @@
'*format': 'str', '*mode': 'NewImageMode' } }
##
+# @BitmapUseMode
+#
+# An enumeration that tells QEMU what operation to take when using a bitmap
+# in drive backup sync mode.
+#
+# @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.
@@ -646,14 +661,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.1)
+#
# @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).
@@ -671,7 +692,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 6d0c944..75035e1 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)
--
1.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v6 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
2014-10-30 3:22 ` [Qemu-devel] [PATCH v6 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup Fam Zheng
@ 2014-11-04 10:53 ` Max Reitz
0 siblings, 0 replies; 3+ messages in thread
From: Max Reitz @ 2014-11-04 10:53 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Kevin Wolf, Benoit Canet, Vladimir Sementsov-Ogievskij,
Markus Armbruster, Luiz Capitulino, John Snow, Stefan Hajnoczi,
Jd, Paolo Bonzini
On 2014-10-30 at 04:22, Fam Zheng wrote:
> 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>
> ---
> block/backup.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++-
> block/mirror.c | 4 ++++
> blockdev.c | 9 +++++++-
> hmp.c | 4 +++-
> include/block/block_int.h | 4 ++++
> qapi/block-core.json | 30 ++++++++++++++++++++++----
> qmp-commands.hx | 7 +++---
> 7 files changed, 102 insertions(+), 10 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index e334740..4870694 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 */
> + int sync_bitmap_gran;
> MirrorSyncMode sync_mode;
> RateLimit limit;
> BlockdevOnError on_source_error;
> @@ -263,7 +267,7 @@ static void coroutine_fn backup_run(void *opaque)
> job->common.busy = true;
> }
> } else {
> - /* Both FULL and TOP SYNC_MODE's require copying.. */
> + /* full, top and dirty_bitmap modes require copying.. */
> for (; start < end; start++) {
> bool error_is_read;
>
> @@ -317,7 +321,21 @@ static void coroutine_fn backup_run(void *opaque)
> if (alloced == 0) {
> continue;
> }
> + } else if (job->sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
> + int i, dirty = 0;
> + for (i = 0; i < BACKUP_SECTORS_PER_CLUSTER;
> + i += job->sync_bitmap_gran) {
->sync_bitmap_gran should be a sector count here... [1]
> + if (bdrv_get_dirty(bs, job->sync_bitmap,
> + start * BACKUP_SECTORS_PER_CLUSTER + i)) {
> + dirty = 1;
> + break;
> + }
> + }
> + if (!dirty) {
> + continue;
> + }
> }
> +
> /* FULL sync mode we copy the whole drive. */
> ret = backup_do_cow(bs, start * BACKUP_SECTORS_PER_CLUSTER,
> BACKUP_SECTORS_PER_CLUSTER, &error_is_read);
> @@ -341,6 +359,9 @@ static void coroutine_fn backup_run(void *opaque)
> 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);
> @@ -351,12 +372,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);
> @@ -369,6 +393,28 @@ 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;
> + }
How about an error when sync_mode != MIRROR_SYNC_MODE_DIRTY_BITMAP and
sync_bitmap != NULL? It's not really fatal, but I wouldn't call it a
valid combination either.
> +
> + 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:
> + assert(0);
Better use abort() directly (or g_assert_not_reached(), which however
suffers from the same problem as assert(0) which is NDEBUG or the
equivalent for glib).
> + }
> + bdrv_disable_dirty_bitmap(bs, sync_bitmap);
> + }
> +
> len = bdrv_getlength(bs);
> if (len < 0) {
> error_setg_errno(errp, -len, "unable to get length for '%s'",
> @@ -386,6 +432,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);
[1] so this should return a sector count (relevant for patch 3);
although I'd still personally prefer it to return a byte count which is
converted to sectors here (but it doesn't matter from a technical
perspective).
> + }
> 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 cecd448..8dfd908 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -679,6 +679,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 c7e98ad..fca909b 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1463,6 +1463,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);
> @@ -2186,6 +2188,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)
> @@ -2282,7 +2286,10 @@ void qmp_drive_backup(const char *device, const char *target,
> return;
> }
>
This conflicts with Stefan's blockjob/dataplane series (can be easily
fixed, though).
> - backup_start(bs, target_bs, speed, sync, on_source_error, on_target_error,
> + backup_start(bs, target_bs, speed, sync,
> + has_bitmap ? bdrv_find_dirty_bitmap(bs, bitmap) : NULL,
Shouldn't we be checking the return value of bdrv_find_dirty_bitmap() here?
I know backup_start() does a check itself, but I'd prefer a check here,
too, nonetheless.
> + 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 63d7686..bc5a2d2 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -966,7 +966,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_int.h b/include/block/block_int.h
> index 8898c6c..0671c89 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -567,6 +567,8 @@ 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 name of dirty bitmap if sync_mode is
> + * MIRROR_SYNC_MODE_DIRTY_BITMAP.
It's not the name, it's the bitmap itself. Also, documentation for
bitmap_mode is missing.
> * @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.
> @@ -577,6 +579,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 97c31bf..e225220 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -473,7 +473,7 @@
> # Since: 1.3
> ##
> { 'enum': 'MirrorSyncMode',
> - 'data': ['top', 'full', 'none'] }
> + 'data': ['top', 'full', 'none', 'dirty-bitmap'] }
>
> ##
> # @BlockJobType:
> @@ -634,6 +634,21 @@
> '*format': 'str', '*mode': 'NewImageMode' } }
>
> ##
> +# @BitmapUseMode
> +#
> +# An enumeration that tells QEMU what operation to take when using a bitmap
> +# in drive backup sync mode.
Did you mean "when using a bitmap in drive backup sync mode
dirty-bitmap"? (or just "when using a bitmap for drive-backup")
> +#
> +# @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.
> @@ -646,14 +661,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.1)
*2.3
Max
> +#
> # @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).
> @@ -671,7 +692,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 6d0c944..75035e1 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)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v6 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
@ 2014-11-12 11:23 Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 3+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2014-11-12 11:23 UTC (permalink / raw)
To: famz
Cc: Kevin Wolf, Benoit Canet, John Snow, Markus Armbruster,
qemu-devel, Max Reitz, Stefan Hajnoczi, Jd, Paolo Bonzini,
Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]
> @@ -317,7 +321,21 @@ static void coroutine_fn backup_run(void *opaque)
> if (alloced == 0) {
> continue;
> }
> + } else if (job->sync_mode == MIRROR_SYNC_MODE_DIRTY_BITMAP) {
> + int i, dirty = 0;
> + for (i = 0; i < BACKUP_SECTORS_PER_CLUSTER;
> + i += job->sync_bitmap_gran) {
> + if (bdrv_get_dirty(bs, job->sync_bitmap,
> + start * BACKUP_SECTORS_PER_CLUSTER + i)) {
> + dirty = 1;
> + break;
> + }
> + }
> + if (!dirty) {
> + continue;
> + }
> }
> +
With such solution we don't use the power of HBitmap, which provides
"hbitmap_iter_skip_words", with O(log(n)) complexity. Here we look
through the whole bitmap (O(n)), instead of jumping to the next dirty bit.
--
С уважением,
Владимир
[-- Attachment #2: Type: text/html, Size: 1630 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-12 11:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-12 11:23 [Qemu-devel] [PATCH v6 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup Vladimir Sementsov-Ogievskiy
-- strict thread matches above, loose matches on Subject: below --
2014-10-30 3:22 [Qemu-devel] [PATCH v6 00/10] block: Incremental backup series Fam Zheng
2014-10-30 3:22 ` [Qemu-devel] [PATCH v6 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup Fam Zheng
2014-11-04 10:53 ` Max Reitz
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.