* [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series
@ 2014-11-26 17:41 John Snow
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 01/10] qapi: Add optional field "name" to block dirty bitmap John Snow
` (9 more replies)
0 siblings, 10 replies; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, John Snow, armbru, mreitz, vsementsov, stefanha, pbonzini
This is the in memory part of the incremental backup feature.
With the added commands, we can create a bitmap on a block backend, from which
point of time all the writes are tracked by the bitmap, marking sectors as
dirty. Later, we call drive-backup and pass the bitmap to it, to do an
incremental backup.
See the last patch which adds some tests for this use case.
Fam
==
This is the next iteration of Fam's incremenetal backup feature.
I have since taken this over from him and have (slowly) worked through
the feedback to the last version of his patch and have made many
tiny little edits.
For convenience: https://github.com/jnsnow/qemu/commits/dbm-backup
John.
v8:
- Changed int64_t return for bdrv_dbm_calc_def_granularity to uint64_t (2/10)
- Updated commit message (2/10)
- Removed redundant check for null in device parameter (2/10)
- Removed comment cruft. (2/10)
- Removed redundant local_err propagation (several)
- Updated commit message (3/10)
- Fix HBitmap copy loop index (4/10)
- Remove redundant ternary (5/10)
- Shift up the block_dirty_bitmap_lookup function (6/10)
- Error messages cleanup (7/10)
- Add an assertion to explain the re-use of .prepare() for two transactions.
(8/10)
- Removed BDS argument from bitmap enable/disable helper; it was unused. (8/10)
v7: (highlights)
- First version being authored by jsnow
- Addressed most list feedback from V6, many small changes.
All feedback was either addressed on-list (as a wontfix) or patched.
- Replaced all error_set with error_setg
- Replaced all bdrv_find with bdrv_lookup_bs()
- Adjusted the max granularity to share a common function with
backup/mirror that attempts to "guess" a reasonable default.
It clamps between [4K,64K] currently.
- The BdrvDirtyBitmap object now counts granularity exclusively in
bytes to match its interface.
It leaves the sector granularity concerns to HBitmap.
- Reworked the backup loop to utilize the hbitmap iterator.
There are some extra concerns to handle arrhythmic cases where the
granularity of the bitmap does not match the backup cluster size.
This iteration works best when it does match, but it's not a
deal-breaker if it doesn't -- it just gets less efficient.
- Reworked the transactional functions so that abort() wouldn't "undo"
a redundant command. They now have been split into a prepare and a
commit function (with state) and do not provide an abort command.
- Added a block_dirty_bitmap_lookup(device, name, errp) function to
shorten a few of the commands added in this series, particularly
qmp_enable, qmp_disable, and the transaction preparations.
v6: Re-send of v5.
v5: Rebase to master.
v4: Last version tailored by Fam Zheng.
Fam Zheng (10):
qapi: Add optional field "name" to block dirty bitmap
qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
block: Introduce bdrv_dirty_bitmap_granularity()
hbitmap: Add hbitmap_copy
block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap
qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable
qmp: Add support of "dirty-bitmap" sync mode for drive-backup
qapi: Add transaction support to block-dirty-bitmap-{add, enable,
disable}
qmp: Add dirty bitmap 'enabled' field in query-block
qemu-iotests: Add tests for drive-backup sync=dirty-bitmap
block-migration.c | 2 +-
block.c | 114 ++++++++++++++++++++--
block/backup.c | 130 +++++++++++++++++++++----
block/mirror.c | 16 ++--
blockdev.c | 218 +++++++++++++++++++++++++++++++++++++++++-
hmp.c | 4 +-
include/block/block.h | 17 +++-
include/block/block_int.h | 6 ++
include/qemu/hbitmap.h | 8 ++
qapi-schema.json | 5 +-
qapi/block-core.json | 120 ++++++++++++++++++++++-
qmp-commands.hx | 66 ++++++++++++-
tests/qemu-iotests/056 | 33 ++++++-
tests/qemu-iotests/056.out | 4 +-
tests/qemu-iotests/iotests.py | 8 ++
util/hbitmap.c | 16 ++++
16 files changed, 712 insertions(+), 55 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v8 01/10] qapi: Add optional field "name" to block dirty bitmap
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
@ 2014-11-26 17:41 ` John Snow
2014-11-27 9:36 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove John Snow
` (8 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Fam Zheng, John Snow, armbru, mreitz, vsementsov,
stefanha, pbonzini
From: Fam Zheng <famz@redhat.com>
This field will be set for user created dirty bitmap. Also pass in an
error pointer to bdrv_create_dirty_bitmap, so when a name is already
taken on this BDS, it can report an error message. This is not global
check, two BDSes can have dirty bitmap with a common name.
Implemented bdrv_find_dirty_bitmap to find a dirty bitmap by name, will
be used later when other QMP commands want to reference dirty bitmap by
name.
Add bdrv_dirty_bitmap_make_anon. This unsets the name of dirty bitmap.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block-migration.c | 2 +-
block.c | 32 +++++++++++++++++++++++++++++++-
block/mirror.c | 2 +-
include/block/block.h | 7 ++++++-
qapi/block-core.json | 4 +++-
5 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index 08db01a..6f3aa18 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -319,7 +319,7 @@ static int set_dirty_tracking(void)
QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE,
- NULL);
+ NULL, NULL);
if (!bmds->dirty_bitmap) {
ret = -errno;
goto fail;
diff --git a/block.c b/block.c
index a612594..f94b753 100644
--- a/block.c
+++ b/block.c
@@ -53,6 +53,7 @@
struct BdrvDirtyBitmap {
HBitmap *bitmap;
+ char *name;
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -5291,7 +5292,28 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov)
return true;
}
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
+{
+ BdrvDirtyBitmap *bm;
+
+ assert(name);
+ QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
+ if (bm->name && !strcmp(name, bm->name)) {
+ return bm;
+ }
+ }
+ return NULL;
+}
+
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+ g_free(bitmap->name);
+ bitmap->name = NULL;
+}
+
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+ int granularity,
+ const char *name,
Error **errp)
{
int64_t bitmap_size;
@@ -5299,6 +5321,10 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
assert((granularity & (granularity - 1)) == 0);
+ if (name && bdrv_find_dirty_bitmap(bs, name)) {
+ error_setg(errp, "Bitmap already exists: %s", name);
+ return NULL;
+ }
granularity >>= BDRV_SECTOR_BITS;
assert(granularity);
bitmap_size = bdrv_nb_sectors(bs);
@@ -5309,6 +5335,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
}
bitmap = g_new0(BdrvDirtyBitmap, 1);
bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+ bitmap->name = g_strdup(name);
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
return bitmap;
}
@@ -5320,6 +5347,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
if (bm == bitmap) {
QLIST_REMOVE(bitmap, list);
hbitmap_free(bitmap->bitmap);
+ g_free(bitmap->name);
g_free(bitmap);
return;
}
@@ -5338,6 +5366,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
info->count = bdrv_get_dirty_count(bs, bm);
info->granularity =
((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+ info->has_name = !!bm->name;
+ info->name = g_strdup(bm->name);
entry->value = info;
*plist = entry;
plist = &entry->next;
diff --git a/block/mirror.c b/block/mirror.c
index 2c6dd2a..858e4ff 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -699,7 +699,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
s->granularity = granularity;
s->buf_size = MAX(buf_size, granularity);
- s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, errp);
+ s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
if (!s->dirty_bitmap) {
return;
}
diff --git a/include/block/block.h b/include/block/block.h
index 5450610..977f7b5 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -428,8 +428,13 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
struct HBitmapIter;
typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
+BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
+ int granularity,
+ const char *name,
Error **errp);
+BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
+ const char *name);
+void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index a14e6ab..569c9f5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -304,6 +304,8 @@
#
# Block dirty bitmap information.
#
+# @name: #optional the name of the dirty bitmap (Since 2.3)
+#
# @count: number of dirty bytes according to the dirty bitmap
#
# @granularity: granularity of the dirty bitmap in bytes (since 1.4)
@@ -311,7 +313,7 @@
# Since: 1.3
##
{ 'type': 'BlockDirtyInfo',
- 'data': {'count': 'int', 'granularity': 'int'} }
+ 'data': {'*name': 'str', 'count': 'int', 'granularity': 'int'} }
##
# @BlockInfo:
--
1.9.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v8 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 01/10] qapi: Add optional field "name" to block dirty bitmap John Snow
@ 2014-11-26 17:41 ` John Snow
2014-11-27 9:41 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 03/10] block: Introduce bdrv_dirty_bitmap_granularity() John Snow
` (7 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Fam Zheng, John Snow, armbru, mreitz, vsementsov,
stefanha, pbonzini
From: Fam Zheng <famz@redhat.com>
The new command pair is added to manage user created dirty bitmap. The
dirty bitmap's name is mandatory and must be unique for the same device,
but different devices can have bitmaps with the same names.
The granularity is an optional field. If it is not specified, we will
choose a default granularity based on the cluster size if available,
clamped to between 4K and 64K (To mirror how the 'mirror' code was
already choosing granularity.) If we do not have cluster size info
available, we choose 64K. This code has been factored out into helper
shared with block/mirror.
The types added to block-core.json will be re-used in future patches
in this series, see:
'qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}'
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 19 ++++++++++++++++++
block/mirror.c | 10 +---------
blockdev.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
include/block/block.h | 1 +
qapi/block-core.json | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
qmp-commands.hx | 49 +++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 179 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index f94b753..c794316 100644
--- a/block.c
+++ b/block.c
@@ -5385,6 +5385,25 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector
}
}
+#define BDB_MIN_DEF_GRANULARITY 4096
+#define BDB_MAX_DEF_GRANULARITY 65536
+#define BDB_DEFAULT_GRANULARITY BDB_MAX_DEF_GRANULARITY
+
+uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs)
+{
+ BlockDriverInfo bdi;
+ uint64_t granularity;
+
+ if (bdrv_get_info(bs, &bdi) >= 0 && bdi.cluster_size != 0) {
+ granularity = MAX(BDB_MIN_DEF_GRANULARITY, bdi.cluster_size);
+ granularity = MIN(BDB_MAX_DEF_GRANULARITY, granularity);
+ } else {
+ granularity = BDB_DEFAULT_GRANULARITY;
+ }
+
+ return granularity;
+}
+
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
{
diff --git a/block/mirror.c b/block/mirror.c
index 858e4ff..3633632 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -664,15 +664,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
MirrorBlockJob *s;
if (granularity == 0) {
- /* Choose the default granularity based on the target file's cluster
- * size, clamped between 4k and 64k. */
- BlockDriverInfo bdi;
- if (bdrv_get_info(target, &bdi) >= 0 && bdi.cluster_size != 0) {
- granularity = MAX(4096, bdi.cluster_size);
- granularity = MIN(65536, granularity);
- } else {
- granularity = 65536;
- }
+ granularity = bdrv_dbm_calc_def_granularity(target);
}
assert ((granularity & (granularity - 1)) == 0);
diff --git a/blockdev.c b/blockdev.c
index 57910b8..af1152f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1810,6 +1810,60 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd,
aio_context_release(aio_context);
}
+void qmp_block_dirty_bitmap_add(const char *device, const char *name,
+ bool has_granularity, int64_t granularity,
+ Error **errp)
+{
+ BlockDriverState *bs;
+
+ bs = bdrv_lookup_bs(device, NULL, errp);
+ if (!bs) {
+ return;
+ }
+
+ if (!name || name[0] == '\0') {
+ error_setg(errp, "Bitmap name cannot be empty");
+ return;
+ }
+ if (has_granularity) {
+ if (granularity < 512 || !is_power_of_2(granularity)) {
+ error_setg(errp, "Granularity must be power of 2 "
+ "and at least 512");
+ return;
+ }
+ } else {
+ /* Default to cluster size, if available: */
+ granularity = bdrv_dbm_calc_def_granularity(bs);
+ }
+
+ bdrv_create_dirty_bitmap(bs, granularity, name, errp);
+}
+
+void qmp_block_dirty_bitmap_remove(const char *device, const char *name,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+
+ bs = bdrv_lookup_bs(device, NULL, errp);
+ if (!bs) {
+ return;
+ }
+
+ if (!name || name[0] == '\0') {
+ error_setg(errp, "Bitmap name cannot be empty");
+ return;
+ }
+ bitmap = bdrv_find_dirty_bitmap(bs, name);
+ if (!bitmap) {
+ error_setg(errp, "Dirty bitmap not found: %s", name);
+ return;
+ }
+
+ bdrv_dirty_bitmap_make_anon(bs, bitmap);
+ bdrv_release_dirty_bitmap(bs, bitmap);
+}
+
int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
const char *id = qdict_get_str(qdict, "id");
diff --git a/include/block/block.h b/include/block/block.h
index 977f7b5..3579e7f 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -437,6 +437,7 @@ BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
+uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
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);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 569c9f5..51638b3 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -864,6 +864,61 @@
'*on-target-error': 'BlockdevOnError' } }
##
+# @BlockDirtyBitmap
+#
+# @device: name of device which the bitmap is tracking
+#
+# @name: name of the dirty bitmap
+#
+# Since 2.3
+##
+{ 'type': 'BlockDirtyBitmap',
+ 'data': { 'device': 'str', 'name': 'str' } }
+
+##
+# @BlockDirtyBitmapAdd
+#
+# @device: name of device which the bitmap is tracking
+#
+# @name: name of the dirty bitmap
+#
+# @granularity: #optional the bitmap granularity, default is 64k for
+# block-dirty-bitmap-add
+#
+# Since 2.3
+##
+{ 'type': 'BlockDirtyBitmapAdd',
+ 'data': { 'device': 'str', 'name': 'str', '*granularity': 'int' } }
+
+##
+# @block-dirty-bitmap-add
+#
+# Create a dirty bitmap with a name on the device
+#
+# Returns: nothing on success
+# If @device is not a valid block device, DeviceNotFound
+# If @name is already taken, GenericError with an explanation
+#
+# Since 2.3
+##
+{'command': 'block-dirty-bitmap-add',
+ 'data': 'BlockDirtyBitmapAdd' }
+
+##
+# @block-dirty-bitmap-remove
+#
+# Remove a dirty bitmap on the device
+#
+# Returns: nothing on success
+# If @device is not a valid block device, DeviceNotFound
+# If @name is not found, GenericError with an explaining message
+#
+# Since 2.3
+##
+{'command': 'block-dirty-bitmap-remove',
+ 'data': 'BlockDirtyBitmap' }
+
+##
# @block_set_io_throttle:
#
# Change I/O throttle limits for a block drive.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 718dd92..94d3293 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1202,6 +1202,55 @@ Example:
EQMP
{
+ .name = "block-dirty-bitmap-add",
+ .args_type = "device:B,name:s,granularity:i?",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_add,
+ },
+ {
+ .name = "block-dirty-bitmap-remove",
+ .args_type = "device:B,name:s",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_remove,
+ },
+
+SQMP
+
+block-dirty-bitmap-add
+----------------------
+
+Create a dirty bitmap with a name on the device, and start tracking the writes.
+
+Arguments:
+
+- "device": device name to create dirty bitmap (json-string)
+- "name": name of the new dirty bitmap (json-string)
+- "granularity": granularity to track writes with (int)
+
+Example:
+
+-> { "execute": "block-dirty-bitmap-add", "arguments": { "device": "drive0",
+ "name": "bitmap0" } }
+<- { "return": {} }
+
+block-dirty-bitmap-remove
+-------------------------
+
+Stop write tracking and remove the dirty bitmap that was created with
+block-dirty-bitmap-add.
+
+Arguments:
+
+- "device": device name to remove dirty bitmap (json-string)
+- "name": name of the dirty bitmap to remove (json-string)
+
+Example:
+
+-> { "execute": "block-dirty-bitmap-remove", "arguments": { "device": "drive0",
+ "name": "bitmap0" } }
+<- { "return": {} }
+
+EQMP
+
+ {
.name = "blockdev-snapshot-sync",
.args_type = "device:s?,node-name:s?,snapshot-file:s,snapshot-node-name:s?,format:s?,mode:s?",
.mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_sync,
--
1.9.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v8 03/10] block: Introduce bdrv_dirty_bitmap_granularity()
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 01/10] qapi: Add optional field "name" to block dirty bitmap John Snow
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove John Snow
@ 2014-11-26 17:41 ` John Snow
2014-11-27 9:42 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 04/10] hbitmap: Add hbitmap_copy John Snow
` (6 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Fam Zheng, John Snow, armbru, mreitz, vsementsov,
stefanha, pbonzini
From: Fam Zheng <famz@redhat.com>
This returns the granularity (in bytes) of dirty bitmap,
which matches the QMP interface and the existing query
interface.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 9 +++++++--
include/block/block.h | 2 ++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index c794316..c851a03 100644
--- a/block.c
+++ b/block.c
@@ -5364,8 +5364,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
info->count = bdrv_get_dirty_count(bs, bm);
- info->granularity =
- ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap));
+ info->granularity = bdrv_dirty_bitmap_granularity(bs, bm);
info->has_name = !!bm->name;
info->name = g_strdup(bm->name);
entry->value = info;
@@ -5404,6 +5403,12 @@ uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs)
return granularity;
}
+int64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap)
+{
+ return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
+}
+
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
{
diff --git a/include/block/block.h b/include/block/block.h
index 3579e7f..c3daa19 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -438,6 +438,8 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs);
+int64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector);
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);
--
1.9.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v8 04/10] hbitmap: Add hbitmap_copy
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
` (2 preceding siblings ...)
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 03/10] block: Introduce bdrv_dirty_bitmap_granularity() John Snow
@ 2014-11-26 17:41 ` John Snow
2014-11-27 9:43 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 05/10] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap John Snow
` (5 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Fam Zheng, John Snow, armbru, mreitz, vsementsov,
stefanha, pbonzini
From: Fam Zheng <famz@redhat.com>
This makes a deep copy of an HBitmap.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
include/qemu/hbitmap.h | 8 ++++++++
util/hbitmap.c | 16 ++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 550d7ce..b645cfc 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -65,6 +65,14 @@ struct HBitmapIter {
HBitmap *hbitmap_alloc(uint64_t size, int granularity);
/**
+ * hbitmap_copy:
+ * @bitmap: The original bitmap to copy.
+ *
+ * Copy a HBitmap.
+ */
+HBitmap *hbitmap_copy(const HBitmap *bitmap);
+
+/**
* hbitmap_empty:
* @hb: HBitmap to operate on.
*
diff --git a/util/hbitmap.c b/util/hbitmap.c
index b3060e6..8aa7406 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -395,3 +395,19 @@ HBitmap *hbitmap_alloc(uint64_t size, int granularity)
hb->levels[0][0] |= 1UL << (BITS_PER_LONG - 1);
return hb;
}
+
+HBitmap *hbitmap_copy(const HBitmap *bitmap)
+{
+ int i;
+ uint64_t size;
+ HBitmap *hb = g_memdup(bitmap, sizeof(HBitmap));
+
+ size = bitmap->size;
+ for (i = HBITMAP_LEVELS - 1; i >= 0; i--) {
+ size = MAX((size + BITS_PER_LONG - 1) >> BITS_PER_LEVEL, 1);
+ hb->levels[i] = g_memdup(bitmap->levels[i],
+ size * sizeof(unsigned long));
+ }
+
+ return hb;
+}
--
1.9.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v8 05/10] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
` (3 preceding siblings ...)
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 04/10] hbitmap: Add hbitmap_copy John Snow
@ 2014-11-26 17:41 ` John Snow
2014-11-27 9:44 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 06/10] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable John Snow
` (4 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Fam Zheng, John Snow, armbru, mreitz, vsementsov,
stefanha, pbonzini
From: Fam Zheng <famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 35 +++++++++++++++++++++++++++++++----
include/block/block.h | 4 ++++
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/block.c b/block.c
index c851a03..40cb9cf 100644
--- a/block.c
+++ b/block.c
@@ -53,6 +53,8 @@
struct BdrvDirtyBitmap {
HBitmap *bitmap;
+ int64_t size;
+ int64_t granularity;
char *name;
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -5311,6 +5313,26 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
bitmap->name = NULL;
}
+BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
+ const BdrvDirtyBitmap *bitmap,
+ const char *name)
+{
+ BdrvDirtyBitmap *new_bitmap;
+
+ new_bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
+ new_bitmap->bitmap = hbitmap_copy(bitmap->bitmap);
+ new_bitmap->size = bitmap->size;
+ new_bitmap->granularity = bitmap->granularity;
+ new_bitmap->name = g_strdup(name);
+ QLIST_INSERT_HEAD(&bs->dirty_bitmaps, new_bitmap, list);
+ return new_bitmap;
+}
+
+void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
+{
+ hbitmap_reset(bitmap->bitmap, 0, bitmap->size);
+}
+
BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
int granularity,
const char *name,
@@ -5318,6 +5340,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
{
int64_t bitmap_size;
BdrvDirtyBitmap *bitmap;
+ int sector_granularity;
assert((granularity & (granularity - 1)) == 0);
@@ -5325,8 +5348,8 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
error_setg(errp, "Bitmap already exists: %s", name);
return NULL;
}
- granularity >>= BDRV_SECTOR_BITS;
- assert(granularity);
+ sector_granularity = granularity >> BDRV_SECTOR_BITS;
+ assert(sector_granularity);
bitmap_size = bdrv_nb_sectors(bs);
if (bitmap_size < 0) {
error_setg_errno(errp, -bitmap_size, "could not get length of device");
@@ -5334,7 +5357,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
return NULL;
}
bitmap = g_new0(BdrvDirtyBitmap, 1);
- bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
+ bitmap->size = bitmap_size;
+ bitmap->granularity = granularity;
+ bitmap->bitmap = hbitmap_alloc(bitmap->size, ffs(sector_granularity) - 1);
bitmap->name = g_strdup(name);
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
return bitmap;
@@ -5406,7 +5431,9 @@ uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs)
int64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap)
{
- return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap);
+ g_assert(BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap) ==
+ bitmap->granularity);
+ return bitmap->granularity;
}
void bdrv_dirty_iter_init(BlockDriverState *bs,
diff --git a/include/block/block.h b/include/block/block.h
index c3daa19..9326705 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -435,6 +435,10 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
const char *name);
void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
+ const BdrvDirtyBitmap *bitmap,
+ const char *name);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs);
--
1.9.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v8 06/10] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
` (4 preceding siblings ...)
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 05/10] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap John Snow
@ 2014-11-26 17:41 ` John Snow
2014-11-27 10:03 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
` (3 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Fam Zheng, John Snow, armbru, mreitz, vsementsov,
stefanha, pbonzini
From: Fam Zheng <famz@redhat.com>
This allows to put the dirty bitmap into a disabled state where no more
writes will be tracked.
It will be used before backup or writing to persistent file.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 15 +++++++++++++
blockdev.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
include/block/block.h | 2 ++
qapi/block-core.json | 28 +++++++++++++++++++++++
qmp-commands.hx | 10 +++++++++
5 files changed, 116 insertions(+)
diff --git a/block.c b/block.c
index 40cb9cf..1aa723b 100644
--- a/block.c
+++ b/block.c
@@ -56,6 +56,7 @@ struct BdrvDirtyBitmap {
int64_t size;
int64_t granularity;
char *name;
+ bool enabled;
QLIST_ENTRY(BdrvDirtyBitmap) list;
};
@@ -5361,6 +5362,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
bitmap->granularity = granularity;
bitmap->bitmap = hbitmap_alloc(bitmap->size, ffs(sector_granularity) - 1);
bitmap->name = g_strdup(name);
+ bitmap->enabled = true;
QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
return bitmap;
}
@@ -5379,6 +5381,16 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
}
}
+void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+ bitmap->enabled = false;
+}
+
+void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+ bitmap->enabled = true;
+}
+
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
{
BdrvDirtyBitmap *bm;
@@ -5447,6 +5459,9 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
{
BdrvDirtyBitmap *bitmap;
QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
+ if (!bitmap->enabled) {
+ continue;
+ }
hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
}
}
diff --git a/blockdev.c b/blockdev.c
index af1152f..276a31b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1159,6 +1159,41 @@ SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device,
return info;
}
+/**
+ * Return a dirty bitmap (if present), after validating
+ * the device and bitmap names. Returns NULL on error,
+ * including when the device and/or bitmap is not found.
+ */
+static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *device,
+ const char *name,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ BdrvDirtyBitmap *bitmap;
+
+ if (!device) {
+ error_setg(errp, "Device cannot be NULL");
+ return NULL;
+ }
+ if (!name) {
+ error_setg(errp, "Bitmap name cannot be NULL");
+ return NULL;
+ }
+
+ bs = bdrv_lookup_bs(device, NULL, errp);
+ if (!bs) {
+ return NULL;
+ }
+
+ bitmap = bdrv_find_dirty_bitmap(bs, name);
+ if (!bitmap) {
+ error_setg(errp, "Dirty bitmap not found: %s", name);
+ return NULL;
+ }
+
+ return bitmap;
+}
+
/* New and old BlockDriverState structs for group snapshots */
typedef struct BlkTransactionState BlkTransactionState;
@@ -1864,6 +1899,32 @@ void qmp_block_dirty_bitmap_remove(const char *device, const char *name,
bdrv_release_dirty_bitmap(bs, bitmap);
}
+void qmp_block_dirty_bitmap_enable(const char *device, const char *name,
+ Error **errp)
+{
+ BdrvDirtyBitmap *bitmap;
+
+ bitmap = block_dirty_bitmap_lookup(device, name, errp);
+ if (!bitmap) {
+ return;
+ }
+
+ bdrv_enable_dirty_bitmap(bitmap);
+}
+
+void qmp_block_dirty_bitmap_disable(const char *device, const char *name,
+ Error **errp)
+{
+ BdrvDirtyBitmap *bitmap;
+
+ bitmap = block_dirty_bitmap_lookup(device, name, errp);
+ if (!bitmap) {
+ return;
+ }
+
+ bdrv_disable_dirty_bitmap(bitmap);
+}
+
int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
const char *id = qdict_get_str(qdict, "id");
diff --git a/include/block/block.h b/include/block/block.h
index 9326705..14a0632 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -440,6 +440,8 @@ BdrvDirtyBitmap *bdrv_copy_dirty_bitmap(BlockDriverState *bs,
const BdrvDirtyBitmap *bitmap,
const char *name);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
+void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
+void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);
BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs);
int64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 51638b3..d77f19d 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -919,6 +919,34 @@
'data': 'BlockDirtyBitmap' }
##
+# @block-dirty-bitmap-enable
+#
+# Enable a dirty bitmap on the device
+#
+# Returns: nothing on success
+# If @device is not a valid block device, DeviceNotFound
+# If @name is not found, GenericError with an explaining message
+#
+# Since 2.3
+##
+{'command': 'block-dirty-bitmap-enable',
+ 'data': 'BlockDirtyBitmap' }
+
+##
+# @block-dirty-bitmap-disable
+#
+# Disable a dirty bitmap on the device
+#
+# Returns: nothing on success
+# If @device is not a valid block device, DeviceNotFound
+# If @name is not found, GenericError with an explaining message
+#
+# Since 2.3
+##
+{'command': 'block-dirty-bitmap-disable',
+ 'data': 'BlockDirtyBitmap' }
+
+##
# @block_set_io_throttle:
#
# Change I/O throttle limits for a block drive.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 94d3293..cc31e22 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1211,6 +1211,16 @@ EQMP
.args_type = "device:B,name:s",
.mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_remove,
},
+ {
+ .name = "block-dirty-bitmap-enable",
+ .args_type = "device:B,name:s",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_enable,
+ },
+ {
+ .name = "block-dirty-bitmap-disable",
+ .args_type = "device:B,name:s",
+ .mhandler.cmd_new = qmp_marshal_input_block_dirty_bitmap_disable,
+ },
SQMP
--
1.9.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v8 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
` (5 preceding siblings ...)
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 06/10] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable John Snow
@ 2014-11-26 17:41 ` John Snow
2014-11-27 9:18 ` Fam Zheng
2014-11-27 10:19 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 08/10] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable} John Snow
` (2 subsequent siblings)
9 siblings, 2 replies; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Fam Zheng, John Snow, armbru, mreitz, vsementsov,
stefanha, pbonzini
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>
---
block.c | 5 ++
block/backup.c | 130 ++++++++++++++++++++++++++++++++++++++--------
block/mirror.c | 4 ++
blockdev.c | 18 ++++++-
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, 175 insertions(+), 30 deletions(-)
diff --git a/block.c b/block.c
index 1aa723b..e341844 100644
--- a/block.c
+++ b/block.c
@@ -5454,6 +5454,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 276a31b..1a56959 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1505,6 +1505,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);
@@ -2235,6 +2237,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)
@@ -2242,6 +2246,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;
@@ -2337,7 +2342,18 @@ 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, "A bitmap name was given, but 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 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.h b/include/block/block.h
index 14a0632..cbbb778 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -451,6 +451,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 a1c17b9..215a27d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -571,6 +571,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.
@@ -581,6 +585,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 d77f19d..3ca9566 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -470,7 +470,7 @@
# Since: 1.3
##
{ 'enum': 'MirrorSyncMode',
- 'data': ['top', 'full', 'none'] }
+ 'data': ['top', 'full', 'none', 'dirty-bitmap'] }
##
# @BlockJobType:
@@ -633,6 +633,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.
@@ -645,14 +660,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).
@@ -670,7 +691,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 cc31e22..fb3545f 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] 24+ messages in thread
* [Qemu-devel] [PATCH v8 08/10] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
` (6 preceding siblings ...)
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
@ 2014-11-26 17:41 ` John Snow
2014-11-27 10:25 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 09/10] qmp: Add dirty bitmap 'enabled' field in query-block John Snow
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 10/10] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap John Snow
9 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Fam Zheng, John Snow, armbru, mreitz, vsementsov,
stefanha, pbonzini
From: Fam Zheng <famz@redhat.com>
This adds three qmp commands to transactions.
Users can stop a dirty bitmap, start backup of it, and start another
dirty bitmap atomically, so that the dirty bitmap is tracked
incrementally and we don't miss any write.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
qapi-schema.json | 5 +++-
2 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index 1a56959..275eb43 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1532,6 +1532,76 @@ static void drive_backup_abort(BlkTransactionState *common)
}
}
+static void block_dirty_bitmap_add_prepare(BlkTransactionState *common,
+ Error **errp)
+{
+ BlockDirtyBitmapAdd *action;
+
+ action = common->action->block_dirty_bitmap_add;
+ qmp_block_dirty_bitmap_add(action->device, action->name,
+ action->has_granularity, action->granularity,
+ errp);
+}
+
+static void block_dirty_bitmap_add_abort(BlkTransactionState *common)
+{
+ BlockDirtyBitmapAdd *action;
+ BdrvDirtyBitmap *bm;
+ BlockDriverState *bs;
+
+ action = common->action->block_dirty_bitmap_add;
+ bs = bdrv_lookup_bs(action->device, NULL, NULL);
+ if (bs) {
+ bm = bdrv_find_dirty_bitmap(bs, action->name);
+ if (bm) {
+ bdrv_release_dirty_bitmap(bs, bm);
+ }
+ }
+}
+
+typedef struct BlockDirtyBitmapState {
+ BlkTransactionState common;
+ BdrvDirtyBitmap *bitmap;
+} BlockDirtyBitmapState;
+
+/**
+ * Enable and Disable re-uses the same preparation.
+ */
+static void block_dirty_bitmap_en_toggle_prepare(BlkTransactionState *common,
+ Error **errp)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ BlockDirtyBitmap *action;
+
+ /* We may be used by either enable or disable;
+ * We use the "enable" member of the union here,
+ * but "disable" should be functionally equivalent: */
+ action = common->action->block_dirty_bitmap_enable;
+ assert(action == common->action->block_dirty_bitmap_disable);
+
+ state->bitmap = block_dirty_bitmap_lookup(action->device,
+ action->name,
+ errp);
+ if (!state->bitmap) {
+ return;
+ }
+}
+
+static void block_dirty_bitmap_enable_commit(BlkTransactionState *common)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ bdrv_enable_dirty_bitmap(state->bitmap);
+}
+
+static void block_dirty_bitmap_disable_commit(BlkTransactionState *common)
+{
+ BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
+ common, common);
+ bdrv_disable_dirty_bitmap(state->bitmap);
+}
+
static void abort_prepare(BlkTransactionState *common, Error **errp)
{
error_setg(errp, "Transaction aborted using Abort action");
@@ -1564,6 +1634,21 @@ static const BdrvActionOps actions[] = {
.prepare = internal_snapshot_prepare,
.abort = internal_snapshot_abort,
},
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ADD] = {
+ .instance_size = sizeof(BlkTransactionState),
+ .prepare = block_dirty_bitmap_add_prepare,
+ .abort = block_dirty_bitmap_add_abort,
+ },
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ENABLE] = {
+ .instance_size = sizeof(BlockDirtyBitmapState),
+ .prepare = block_dirty_bitmap_en_toggle_prepare,
+ .commit = block_dirty_bitmap_enable_commit,
+ },
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_DISABLE] = {
+ .instance_size = sizeof(BlockDirtyBitmapState),
+ .prepare = block_dirty_bitmap_en_toggle_prepare,
+ .commit = block_dirty_bitmap_disable_commit,
+ },
};
/*
diff --git a/qapi-schema.json b/qapi-schema.json
index 9ffdcf8..793031b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1260,7 +1260,10 @@
'blockdev-snapshot-sync': 'BlockdevSnapshot',
'drive-backup': 'DriveBackup',
'abort': 'Abort',
- 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal'
+ 'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
+ 'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
+ 'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
+ 'block-dirty-bitmap-disable': 'BlockDirtyBitmap'
} }
##
--
1.9.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v8 09/10] qmp: Add dirty bitmap 'enabled' field in query-block
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
` (7 preceding siblings ...)
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 08/10] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable} John Snow
@ 2014-11-26 17:41 ` John Snow
2014-11-27 10:26 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 10/10] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap John Snow
9 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Fam Zheng, John Snow, armbru, mreitz, vsementsov,
stefanha, pbonzini
From: Fam Zheng <famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
block.c | 1 +
qapi/block-core.json | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index e341844..3ad1af3 100644
--- a/block.c
+++ b/block.c
@@ -5404,6 +5404,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
info->granularity = bdrv_dirty_bitmap_granularity(bs, bm);
info->has_name = !!bm->name;
info->name = g_strdup(bm->name);
+ info->enabled = bm->enabled;
entry->value = info;
*plist = entry;
plist = &entry->next;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3ca9566..b948dcc 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -310,10 +310,13 @@
#
# @granularity: granularity of the dirty bitmap in bytes (since 1.4)
#
+# @enabled: whether the dirty bitmap is enabled (Since 2.3)
+#
# Since: 1.3
##
{ 'type': 'BlockDirtyInfo',
- 'data': {'*name': 'str', 'count': 'int', 'granularity': 'int'} }
+ 'data': {'*name': 'str', 'count': 'int', 'granularity': 'int',
+ 'enabled': 'bool'} }
##
# @BlockInfo:
--
1.9.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Qemu-devel] [PATCH v8 10/10] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
` (8 preceding siblings ...)
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 09/10] qmp: Add dirty bitmap 'enabled' field in query-block John Snow
@ 2014-11-26 17:41 ` John Snow
2014-11-27 10:27 ` Max Reitz
9 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2014-11-26 17:41 UTC (permalink / raw)
To: qemu-devel
Cc: kwolf, Fam Zheng, John Snow, armbru, mreitz, vsementsov,
stefanha, pbonzini
From: Fam Zheng <famz@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/056 | 33 ++++++++++++++++++++++++++++++---
tests/qemu-iotests/056.out | 4 ++--
tests/qemu-iotests/iotests.py | 8 ++++++++
3 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
index 54e4bd0..fc9114e 100755
--- a/tests/qemu-iotests/056
+++ b/tests/qemu-iotests/056
@@ -23,17 +23,17 @@
import time
import os
import iotests
-from iotests import qemu_img, qemu_io, create_image
+from iotests import qemu_img, qemu_img_map_assert, qemu_io, create_image
backing_img = os.path.join(iotests.test_dir, 'backing.img')
test_img = os.path.join(iotests.test_dir, 'test.img')
target_img = os.path.join(iotests.test_dir, 'target.img')
-class TestSyncModesNoneAndTop(iotests.QMPTestCase):
+class TestSyncModes(iotests.QMPTestCase):
image_len = 64 * 1024 * 1024 # MB
def setUp(self):
- create_image(backing_img, TestSyncModesNoneAndTop.image_len)
+ create_image(backing_img, TestSyncModes.image_len)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
qemu_io('-c', 'write -P0x41 0 512', test_img)
qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
@@ -64,6 +64,33 @@ class TestSyncModesNoneAndTop(iotests.QMPTestCase):
self.assertTrue(iotests.compare_images(test_img, target_img),
'target image does not match source after backup')
+ def test_sync_dirty_bitmap_missing(self):
+ self.assert_no_active_block_jobs()
+ result = self.vm.qmp('drive-backup', device='drive0', sync='dirty-bitmap',
+ format=iotests.imgfmt, target=target_img)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_sync_dirty_bitmap_not_found(self):
+ self.assert_no_active_block_jobs()
+ result = self.vm.qmp('drive-backup', device='drive0', sync='dirty-bitmap',
+ bitmap='unknown',
+ format=iotests.imgfmt, target=target_img)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_sync_dirty_bitmap(self):
+ self.assert_no_active_block_jobs()
+ result = self.vm.qmp('block-dirty-bitmap-add', device='drive0', name='bitmap0')
+ self.assert_qmp(result, 'return', {})
+ self.vm.hmp_qemu_io('drive0', 'write -P0x5a 0 512')
+ self.vm.hmp_qemu_io('drive0', 'write -P0x5a 48M 512')
+ result = self.vm.qmp('drive-backup', device='drive0', sync='dirty-bitmap',
+ bitmap='bitmap0',
+ format=iotests.imgfmt, target=target_img)
+ self.assert_qmp(result, 'return', {})
+ self.wait_until_completed(check_offset=False)
+ self.assert_no_active_block_jobs()
+ qemu_img_map_assert(target_img, [0, 0x3000000])
+
def test_cancel_sync_none(self):
self.assert_no_active_block_jobs()
diff --git a/tests/qemu-iotests/056.out b/tests/qemu-iotests/056.out
index fbc63e6..914e373 100644
--- a/tests/qemu-iotests/056.out
+++ b/tests/qemu-iotests/056.out
@@ -1,5 +1,5 @@
-..
+.....
----------------------------------------------------------------------
-Ran 2 tests
+Ran 5 tests
OK
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f57f154..95bb959 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -55,6 +55,14 @@ def qemu_img_pipe(*args):
'''Run qemu-img and return its output'''
return subprocess.Popen(qemu_img_args + list(args), stdout=subprocess.PIPE).communicate()[0]
+def qemu_img_map_assert(img, offsets):
+ '''Run qemu-img map on img and check the mapped ranges'''
+ offs = []
+ for line in qemu_img_pipe('map', img).splitlines()[1:]:
+ offset, length, mapped, fname = line.split()
+ offs.append(int(offset, 16))
+ assert set(offs) == set(offsets), "mapped offsets in image '%s' not equal to '%s'" % (str(offs), str(offsets))
+
def qemu_io(*args):
'''Run qemu-io and return the stdout data'''
args = qemu_io_args + list(args)
--
1.9.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
@ 2014-11-27 9:18 ` Fam Zheng
2014-11-27 10:19 ` Max Reitz
1 sibling, 0 replies; 24+ messages in thread
From: Fam Zheng @ 2014-11-27 9:18 UTC (permalink / raw)
To: John Snow
Cc: kwolf, armbru, qemu-devel, mreitz, vsementsov, stefanha, pbonzini
On Wed, 11/26 12:41, 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.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 5 ++
> block/backup.c | 130 ++++++++++++++++++++++++++++++++++++++--------
> block/mirror.c | 4 ++
> blockdev.c | 18 ++++++-
> 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, 175 insertions(+), 30 deletions(-)
>
> diff --git a/block.c b/block.c
> index 1aa723b..e341844 100644
> --- a/block.c
> +++ b/block.c
> @@ -5454,6 +5454,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 276a31b..1a56959 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1505,6 +1505,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);
> @@ -2235,6 +2237,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)
> @@ -2242,6 +2246,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;
> @@ -2337,7 +2342,18 @@ 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, "A bitmap name was given, but bitmap '%s' could not be found",
This line is too long.
Fam
> + 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 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.h b/include/block/block.h
> index 14a0632..cbbb778 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -451,6 +451,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 a1c17b9..215a27d 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -571,6 +571,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.
> @@ -581,6 +585,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 d77f19d..3ca9566 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -470,7 +470,7 @@
> # Since: 1.3
> ##
> { 'enum': 'MirrorSyncMode',
> - 'data': ['top', 'full', 'none'] }
> + 'data': ['top', 'full', 'none', 'dirty-bitmap'] }
>
> ##
> # @BlockJobType:
> @@ -633,6 +633,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.
> @@ -645,14 +660,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).
> @@ -670,7 +691,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 cc31e22..fb3545f 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 [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 01/10] qapi: Add optional field "name" to block dirty bitmap
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 01/10] qapi: Add optional field "name" to block dirty bitmap John Snow
@ 2014-11-27 9:36 ` Max Reitz
0 siblings, 0 replies; 24+ messages in thread
From: Max Reitz @ 2014-11-27 9:36 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-11-26 at 18:41, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
>
> This field will be set for user created dirty bitmap. Also pass in an
> error pointer to bdrv_create_dirty_bitmap, so when a name is already
> taken on this BDS, it can report an error message. This is not global
> check, two BDSes can have dirty bitmap with a common name.
>
> Implemented bdrv_find_dirty_bitmap to find a dirty bitmap by name, will
> be used later when other QMP commands want to reference dirty bitmap by
> name.
>
> Add bdrv_dirty_bitmap_make_anon. This unsets the name of dirty bitmap.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block-migration.c | 2 +-
> block.c | 32 +++++++++++++++++++++++++++++++-
> block/mirror.c | 2 +-
> include/block/block.h | 7 ++++++-
> qapi/block-core.json | 4 +++-
> 5 files changed, 42 insertions(+), 5 deletions(-)
You can keep R-bs on patches which you don't have modified (or only
contextually, or directly according to what the reviewer made a
condition for the R-b). Just add them below the S-o-b in the commit message.
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove John Snow
@ 2014-11-27 9:41 ` Max Reitz
2014-12-01 18:52 ` John Snow
0 siblings, 1 reply; 24+ messages in thread
From: Max Reitz @ 2014-11-27 9:41 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-11-26 at 18:41, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
>
> The new command pair is added to manage user created dirty bitmap. The
> dirty bitmap's name is mandatory and must be unique for the same device,
> but different devices can have bitmaps with the same names.
>
> The granularity is an optional field. If it is not specified, we will
> choose a default granularity based on the cluster size if available,
> clamped to between 4K and 64K (To mirror how the 'mirror' code was
> already choosing granularity.) If we do not have cluster size info
Maybe swap the right parenthesis and the full stop?
> available, we choose 64K. This code has been factored out into helper
Naturally you're better at English than me, but shouldn't this be "into
a helper"?
> shared with block/mirror.
>
> The types added to block-core.json will be re-used in future patches
> in this series, see:
> 'qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}'
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 19 ++++++++++++++++++
> block/mirror.c | 10 +---------
> blockdev.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/block/block.h | 1 +
> qapi/block-core.json | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
> qmp-commands.hx | 49 +++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 179 insertions(+), 9 deletions(-)
Anyway, with or without these minor changes:
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 03/10] block: Introduce bdrv_dirty_bitmap_granularity()
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 03/10] block: Introduce bdrv_dirty_bitmap_granularity() John Snow
@ 2014-11-27 9:42 ` Max Reitz
0 siblings, 0 replies; 24+ messages in thread
From: Max Reitz @ 2014-11-27 9:42 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-11-26 at 18:41, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
>
> This returns the granularity (in bytes) of dirty bitmap,
> which matches the QMP interface and the existing query
> interface.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 9 +++++++--
> include/block/block.h | 2 ++
> 2 files changed, 9 insertions(+), 2 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 04/10] hbitmap: Add hbitmap_copy
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 04/10] hbitmap: Add hbitmap_copy John Snow
@ 2014-11-27 9:43 ` Max Reitz
0 siblings, 0 replies; 24+ messages in thread
From: Max Reitz @ 2014-11-27 9:43 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-11-26 at 18:41, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
>
> This makes a deep copy of an HBitmap.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> include/qemu/hbitmap.h | 8 ++++++++
> util/hbitmap.c | 16 ++++++++++++++++
> 2 files changed, 24 insertions(+)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 05/10] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 05/10] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap John Snow
@ 2014-11-27 9:44 ` Max Reitz
0 siblings, 0 replies; 24+ messages in thread
From: Max Reitz @ 2014-11-27 9:44 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-11-26 at 18:41, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 35 +++++++++++++++++++++++++++++++----
> include/block/block.h | 4 ++++
> 2 files changed, 35 insertions(+), 4 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 06/10] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 06/10] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable John Snow
@ 2014-11-27 10:03 ` Max Reitz
0 siblings, 0 replies; 24+ messages in thread
From: Max Reitz @ 2014-11-27 10:03 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-11-26 at 18:41, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
>
> This allows to put the dirty bitmap into a disabled state where no more
> writes will be tracked.
>
> It will be used before backup or writing to persistent file.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 15 +++++++++++++
> blockdev.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
> include/block/block.h | 2 ++
> qapi/block-core.json | 28 +++++++++++++++++++++++
> qmp-commands.hx | 10 +++++++++
> 5 files changed, 116 insertions(+)
>
> diff --git a/block.c b/block.c
> index 40cb9cf..1aa723b 100644
> --- a/block.c
> +++ b/block.c
> @@ -56,6 +56,7 @@ struct BdrvDirtyBitmap {
> int64_t size;
> int64_t granularity;
> char *name;
> + bool enabled;
> QLIST_ENTRY(BdrvDirtyBitmap) list;
> };
>
> @@ -5361,6 +5362,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
> bitmap->granularity = granularity;
> bitmap->bitmap = hbitmap_alloc(bitmap->size, ffs(sector_granularity) - 1);
> bitmap->name = g_strdup(name);
> + bitmap->enabled = true;
> QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
> return bitmap;
> }
> @@ -5379,6 +5381,16 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
> }
> }
>
> +void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> +{
> + bitmap->enabled = false;
> +}
> +
> +void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
> +{
> + bitmap->enabled = true;
> +}
> +
> BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
> {
> BdrvDirtyBitmap *bm;
> @@ -5447,6 +5459,9 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,
> {
> BdrvDirtyBitmap *bitmap;
> QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
> + if (!bitmap->enabled) {
> + continue;
> + }
> hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
> }
> }
> diff --git a/blockdev.c b/blockdev.c
> index af1152f..276a31b 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1159,6 +1159,41 @@ SnapshotInfo *qmp_blockdev_snapshot_delete_internal_sync(const char *device,
> return info;
This conflicts with Stefan's AioContext series, so it doesn't apply to
block-next. The above line should read "return NULL;" instead.
> }
>
> +/**
> + * Return a dirty bitmap (if present), after validating
> + * the device and bitmap names. Returns NULL on error,
> + * including when the device and/or bitmap is not found.
> + */
> +static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *device,
> + const char *name,
> + Error **errp)
> +{
> + BlockDriverState *bs;
> + BdrvDirtyBitmap *bitmap;
> +
> + if (!device) {
> + error_setg(errp, "Device cannot be NULL");
> + return NULL;
> + }
> + if (!name) {
> + error_setg(errp, "Bitmap name cannot be NULL");
> + return NULL;
> + }
> +
> + bs = bdrv_lookup_bs(device, NULL, errp);
> + if (!bs) {
> + return NULL;
> + }
> +
> + bitmap = bdrv_find_dirty_bitmap(bs, name);
> + if (!bitmap) {
> + error_setg(errp, "Dirty bitmap not found: %s", name);
> + return NULL;
> + }
> +
> + return bitmap;
> +}
> +
> /* New and old BlockDriverState structs for group snapshots */
And this line should read "/* New and old BlockDriverState structs for
atomic group operations */". With those changes it applies.
With that changed (it's not even a change of the code, just a change of
the patch):
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
2014-11-27 9:18 ` Fam Zheng
@ 2014-11-27 10:19 ` Max Reitz
1 sibling, 0 replies; 24+ messages in thread
From: Max Reitz @ 2014-11-27 10:19 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-11-26 at 18:41, 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.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 5 ++
> block/backup.c | 130 ++++++++++++++++++++++++++++++++++++++--------
> block/mirror.c | 4 ++
> blockdev.c | 18 ++++++-
> 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, 175 insertions(+), 30 deletions(-)
[snip]
> diff --git a/block/backup.c b/block/backup.c
> index 792e655..2aab68f 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -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;
Well, this is basically the opposite of what I'd done, but making it
difficult by formatting a nice error message for the user in places
which should never be reached instead of just aborting is not wrong, in
my opinion (I do know that others have other opinions).
[snip]
> diff --git a/blockdev.c b/blockdev.c
> index 276a31b..1a56959 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2337,7 +2342,18 @@ 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, "A bitmap name was given, but bitmap '%s' could not be found",
> + bitmap);
Well, I do know I said that I'd translate "has_bitmap was set" to "A
bitmap name was specified", but I assumed you'd remove the redundancy
yourself. ;-)
Since this line needs to be reworked anyway (see Fam's reply), it can be
(imho) shortened to "Bitmap '%s' could not be found". The user should
know that he/she has specified a bitmap, and that it did have that name;
the only thing he/she doesn't know so far is that qemu failed to find
that bitmap, so that's the information we need to give.
With that line somehow shortened/split to 80 characters (I'd be fine
with keeping its content as-is, I just think it'd sound strange to the
user):
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 08/10] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable}
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 08/10] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable} John Snow
@ 2014-11-27 10:25 ` Max Reitz
0 siblings, 0 replies; 24+ messages in thread
From: Max Reitz @ 2014-11-27 10:25 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-11-26 at 18:41, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
>
> This adds three qmp commands to transactions.
>
> Users can stop a dirty bitmap, start backup of it, and start another
> dirty bitmap atomically, so that the dirty bitmap is tracked
> incrementally and we don't miss any write.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> blockdev.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> qapi-schema.json | 5 +++-
> 2 files changed, 89 insertions(+), 1 deletion(-)
[snip]
> diff --git a/blockdev.c b/blockdev.c
> index 1a56959..275eb43 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1564,6 +1634,21 @@ static const BdrvActionOps actions[] = {
> .prepare = internal_snapshot_prepare,
> .abort = internal_snapshot_abort,
> },
I probably missed that in my review for v7 because I based that on
my/Kevin's block-next branch instead of Stefan's (which I'm doing now):
This conflicts with Stefan's AioContext series. Now there's a .clean, so
it should read:
.abort = internal_snapshot_abort,
.clean = internal_snapshot_clean,
},
(Hopefully nbsps will keep Thunderbird from breaking the indentation...)
With that fixed:
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 09/10] qmp: Add dirty bitmap 'enabled' field in query-block
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 09/10] qmp: Add dirty bitmap 'enabled' field in query-block John Snow
@ 2014-11-27 10:26 ` Max Reitz
0 siblings, 0 replies; 24+ messages in thread
From: Max Reitz @ 2014-11-27 10:26 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-11-26 at 18:41, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> block.c | 1 +
> qapi/block-core.json | 5 ++++-
> 2 files changed, 5 insertions(+), 1 deletion(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 10/10] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 10/10] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap John Snow
@ 2014-11-27 10:27 ` Max Reitz
0 siblings, 0 replies; 24+ messages in thread
From: Max Reitz @ 2014-11-27 10:27 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-11-26 at 18:41, John Snow wrote:
> From: Fam Zheng <famz@redhat.com>
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> tests/qemu-iotests/056 | 33 ++++++++++++++++++++++++++++++---
> tests/qemu-iotests/056.out | 4 ++--
> tests/qemu-iotests/iotests.py | 8 ++++++++
> 3 files changed, 40 insertions(+), 5 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
2014-11-27 9:41 ` Max Reitz
@ 2014-12-01 18:52 ` John Snow
2014-12-02 9:34 ` Max Reitz
0 siblings, 1 reply; 24+ messages in thread
From: John Snow @ 2014-12-01 18:52 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 11/27/2014 04:41 AM, Max Reitz wrote:
> On 2014-11-26 at 18:41, John Snow wrote:
>> From: Fam Zheng <famz@redhat.com>
>>
>> The new command pair is added to manage user created dirty bitmap. The
>> dirty bitmap's name is mandatory and must be unique for the same device,
>> but different devices can have bitmaps with the same names.
>>
>> The granularity is an optional field. If it is not specified, we will
>> choose a default granularity based on the cluster size if available,
>> clamped to between 4K and 64K (To mirror how the 'mirror' code was
>> already choosing granularity.) If we do not have cluster size info
>
> Maybe swap the right parenthesis and the full stop?
This is an American thing, the difference between "aesthetic
punctuation" and "logical punctuation." (<-- aesthetic.)
http://www.slate.com/articles/life/the_good_word/2011/05/the_rise_of_logical_punctuation.html
I can make a mental note in the future to not use the American style, I
just thought it would be fun to explain it.
>> available, we choose 64K. This code has been factored out into helper
>
> Naturally you're better at English than me, but shouldn't this be "into
> a helper"?
This, on the other hand, is just a typo where my brain filled in the
missing glue for me.
>> shared with block/mirror.
>>
>> The types added to block-core.json will be re-used in future patches
>> in this series, see:
>> 'qapi: Add transaction support to block-dirty-bitmap-{add, enable,
>> disable}'
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>> block.c | 19 ++++++++++++++++++
>> block/mirror.c | 10 +---------
>> blockdev.c | 54
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> include/block/block.h | 1 +
>> qapi/block-core.json | 55
>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>> qmp-commands.hx | 49
>> +++++++++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 179 insertions(+), 9 deletions(-)
>
> Anyway, with or without these minor changes:
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Qemu-devel] [PATCH v8 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
2014-12-01 18:52 ` John Snow
@ 2014-12-02 9:34 ` Max Reitz
0 siblings, 0 replies; 24+ messages in thread
From: Max Reitz @ 2014-12-02 9:34 UTC (permalink / raw)
To: John Snow, qemu-devel
Cc: kwolf, Fam Zheng, armbru, vsementsov, stefanha, pbonzini
On 2014-12-01 at 19:52, John Snow wrote:
>
>
> On 11/27/2014 04:41 AM, Max Reitz wrote:
>> On 2014-11-26 at 18:41, John Snow wrote:
>>> From: Fam Zheng <famz@redhat.com>
>>>
>>> The new command pair is added to manage user created dirty bitmap. The
>>> dirty bitmap's name is mandatory and must be unique for the same
>>> device,
>>> but different devices can have bitmaps with the same names.
>>>
>>> The granularity is an optional field. If it is not specified, we will
>>> choose a default granularity based on the cluster size if available,
>>> clamped to between 4K and 64K (To mirror how the 'mirror' code was
>>> already choosing granularity.) If we do not have cluster size info
>>
>> Maybe swap the right parenthesis and the full stop?
>
> This is an American thing, the difference between "aesthetic
> punctuation" and "logical punctuation." (<-- aesthetic.)
>
> http://www.slate.com/articles/life/the_good_word/2011/05/the_rise_of_logical_punctuation.html
>
>
> I can make a mental note in the future to not use the American style,
> I just thought it would be fun to explain it.
No need to not use that style if it's just me learning English by false
(or maybe in 50 years it'll be right) accusations. Thanks for your
explanation!
Max
>>> available, we choose 64K. This code has been factored out into helper
>>
>> Naturally you're better at English than me, but shouldn't this be "into
>> a helper"?
>
> This, on the other hand, is just a typo where my brain filled in the
> missing glue for me.
>
>>> shared with block/mirror.
>>>
>>> The types added to block-core.json will be re-used in future patches
>>> in this series, see:
>>> 'qapi: Add transaction support to block-dirty-bitmap-{add, enable,
>>> disable}'
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>> block.c | 19 ++++++++++++++++++
>>> block/mirror.c | 10 +---------
>>> blockdev.c | 54
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> include/block/block.h | 1 +
>>> qapi/block-core.json | 55
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>> qmp-commands.hx | 49
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>> 6 files changed, 179 insertions(+), 9 deletions(-)
>>
>> Anyway, with or without these minor changes:
>>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2014-12-02 9:34 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-26 17:41 [Qemu-devel] [PATCH v8 00/10] block: Incremental backup series John Snow
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 01/10] qapi: Add optional field "name" to block dirty bitmap John Snow
2014-11-27 9:36 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove John Snow
2014-11-27 9:41 ` Max Reitz
2014-12-01 18:52 ` John Snow
2014-12-02 9:34 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 03/10] block: Introduce bdrv_dirty_bitmap_granularity() John Snow
2014-11-27 9:42 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 04/10] hbitmap: Add hbitmap_copy John Snow
2014-11-27 9:43 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 05/10] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap John Snow
2014-11-27 9:44 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 06/10] qmp: Add block-dirty-bitmap-enable and block-dirty-bitmap-disable John Snow
2014-11-27 10:03 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 07/10] qmp: Add support of "dirty-bitmap" sync mode for drive-backup John Snow
2014-11-27 9:18 ` Fam Zheng
2014-11-27 10:19 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 08/10] qapi: Add transaction support to block-dirty-bitmap-{add, enable, disable} John Snow
2014-11-27 10:25 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 09/10] qmp: Add dirty bitmap 'enabled' field in query-block John Snow
2014-11-27 10:26 ` Max Reitz
2014-11-26 17:41 ` [Qemu-devel] [PATCH v8 10/10] qemu-iotests: Add tests for drive-backup sync=dirty-bitmap John Snow
2014-11-27 10:27 ` 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.