* [Qemu-devel] [PATCH 0/6] qmp: Add blockdev-mirror
@ 2015-06-09 2:13 Fam Zheng
2015-06-09 2:13 ` [Qemu-devel] [PATCH 1/6] block: Add blocker on mirror target Fam Zheng
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Fam Zheng @ 2015-06-09 2:13 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster, mreitz,
Stefan Hajnoczi, pbonzini
This is the counterpart of blockdev-backup. The biggest value of this command
is to allow full flexibility on target image open options, via blockdev-add.
For example this could help solve the target provisioning issue in:
http://lists.gnu.org/archive/html/qemu-devel/2015-06/msg02139.html
Because mirror job uses bdrv_swap, this series depends on Max's BlockBackend
series:
[PATCH v3 00/38] blockdev: BlockBackend and media
http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg01109.html
which makes it possible for blockdev-add to insert a BDS without BB (by
omitting "id=" while only specifying "node-name=").
Fam Zheng (6):
block: Add blocker on mirror target
block: Rename BLOCK_OP_TYPE_MIRROR to BLOCK_OP_TYPE_MIRROR_SOURCE
block: Extract blockdev part of qmp_drive_mirror
block: Add check on mirror target
qmp: Add blockdev-mirror command
iotests: Add test cases for blockdev-mirror
block/mirror.c | 2 +
blockdev.c | 177 ++++++++++++++++++++++++++++++----------
hw/block/dataplane/virtio-blk.c | 2 +-
include/block/block.h | 3 +-
qapi/block-core.json | 47 +++++++++++
qmp-commands.hx | 48 +++++++++++
tests/qemu-iotests/041 | 100 ++++++++++++++++++-----
tests/qemu-iotests/041.out | 4 +-
8 files changed, 313 insertions(+), 70 deletions(-)
--
2.4.2
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 1/6] block: Add blocker on mirror target
2015-06-09 2:13 [Qemu-devel] [PATCH 0/6] qmp: Add blockdev-mirror Fam Zheng
@ 2015-06-09 2:13 ` Fam Zheng
2015-06-24 16:14 ` Max Reitz
2015-06-09 2:13 ` [Qemu-devel] [PATCH 2/6] block: Rename BLOCK_OP_TYPE_MIRROR to BLOCK_OP_TYPE_MIRROR_SOURCE Fam Zheng
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2015-06-09 2:13 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster, mreitz,
Stefan Hajnoczi, pbonzini
In block/backup.c, we already check and add blocker on the target bs,
which is necessary so that it won't be intervened with other operations.
In block/mirror.c we should also protect the mirror target bs, because it
could have a node-name (drive-mirror ... node-name=XXX), and on top of
that it's safe to add blockdev-mirror.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/mirror.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/block/mirror.c b/block/mirror.c
index 33c640f..8b23ddd 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -360,6 +360,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
aio_context_release(replace_aio_context);
}
g_free(s->replaces);
+ bdrv_op_unblock_all(s->target, s->common.blocker);
bdrv_unref(s->target);
block_job_completed(&s->common, data->ret);
g_free(data);
@@ -682,6 +683,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
return;
}
+ bdrv_op_block_all(target, s->common.blocker);
s->replaces = g_strdup(replaces);
s->on_source_error = on_source_error;
s->on_target_error = on_target_error;
--
2.4.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/6] block: Rename BLOCK_OP_TYPE_MIRROR to BLOCK_OP_TYPE_MIRROR_SOURCE
2015-06-09 2:13 [Qemu-devel] [PATCH 0/6] qmp: Add blockdev-mirror Fam Zheng
2015-06-09 2:13 ` [Qemu-devel] [PATCH 1/6] block: Add blocker on mirror target Fam Zheng
@ 2015-06-09 2:13 ` Fam Zheng
2015-06-24 16:18 ` Max Reitz
2015-06-09 2:13 ` [Qemu-devel] [PATCH 3/6] block: Extract blockdev part of qmp_drive_mirror Fam Zheng
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2015-06-09 2:13 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster, mreitz,
Stefan Hajnoczi, pbonzini
It's necessary to distinguish source and target before we can add
blockdev-mirror, because we would want a concrete type of operation to
check on target bs before starting.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
blockdev.c | 2 +-
hw/block/dataplane/virtio-blk.c | 2 +-
include/block/block.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 90f6e77..b573e56 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2962,7 +2962,7 @@ void qmp_drive_mirror(const char *device, const char *target,
}
}
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR, errp)) {
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
goto out;
}
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 3db139b..dac37de 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -206,7 +206,7 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, s->blocker);
blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
s->blocker);
- blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_MIRROR, s->blocker);
+ blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_MIRROR_SOURCE, s->blocker);
blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker);
blk_op_unblock(conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker);
diff --git a/include/block/block.h b/include/block/block.h
index f5c0a90..69cb676 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -155,7 +155,7 @@ typedef enum BlockOpType {
BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT,
BLOCK_OP_TYPE_INTERNAL_SNAPSHOT,
BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
- BLOCK_OP_TYPE_MIRROR,
+ BLOCK_OP_TYPE_MIRROR_SOURCE,
BLOCK_OP_TYPE_RESIZE,
BLOCK_OP_TYPE_STREAM,
BLOCK_OP_TYPE_REPLACE,
--
2.4.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 3/6] block: Extract blockdev part of qmp_drive_mirror
2015-06-09 2:13 [Qemu-devel] [PATCH 0/6] qmp: Add blockdev-mirror Fam Zheng
2015-06-09 2:13 ` [Qemu-devel] [PATCH 1/6] block: Add blocker on mirror target Fam Zheng
2015-06-09 2:13 ` [Qemu-devel] [PATCH 2/6] block: Rename BLOCK_OP_TYPE_MIRROR to BLOCK_OP_TYPE_MIRROR_SOURCE Fam Zheng
@ 2015-06-09 2:13 ` Fam Zheng
2015-06-24 16:34 ` Max Reitz
2015-06-09 2:13 ` [Qemu-devel] [PATCH 4/6] block: Add check on mirror target Fam Zheng
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2015-06-09 2:13 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster, mreitz,
Stefan Hajnoczi, pbonzini
This is the part that will be reused by blockdev-mirror.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
blockdev.c | 158 +++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 92 insertions(+), 66 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index b573e56..c32a9a9 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2883,29 +2883,22 @@ out:
#define DEFAULT_MIRROR_BUF_SIZE (10 << 20)
-void qmp_drive_mirror(const char *device, const char *target,
- bool has_format, const char *format,
- bool has_node_name, const char *node_name,
- bool has_replaces, const char *replaces,
- enum MirrorSyncMode sync,
- bool has_mode, enum NewImageMode mode,
- bool has_speed, int64_t speed,
- bool has_granularity, uint32_t granularity,
- bool has_buf_size, int64_t buf_size,
- bool has_on_source_error, BlockdevOnError on_source_error,
- bool has_on_target_error, BlockdevOnError on_target_error,
- Error **errp)
+/* Parameter check and block job starting for mirroring.
+ * Caller should hold @device and @target's aio context (They must to be on the
+ * same aio context). */
+static void blockdev_mirror_common(BlockDriverState *bs,
+ BlockDriverState *target,
+ bool has_replaces, const char *replaces,
+ enum MirrorSyncMode sync,
+ bool has_speed, int64_t speed,
+ bool has_granularity, uint32_t granularity,
+ bool has_buf_size, int64_t buf_size,
+ bool has_on_source_error,
+ BlockdevOnError on_source_error,
+ bool has_on_target_error,
+ BlockdevOnError on_target_error,
+ Error **errp)
{
- BlockBackend *blk;
- BlockDriverState *bs;
- BlockDriverState *source, *target_bs;
- AioContext *aio_context;
- BlockDriver *drv = NULL;
- Error *local_err = NULL;
- QDict *options = NULL;
- int flags;
- int64_t size;
- int ret;
if (!has_speed) {
speed = 0;
@@ -2916,9 +2909,6 @@ void qmp_drive_mirror(const char *device, const char *target,
if (!has_on_target_error) {
on_target_error = BLOCKDEV_ON_ERROR_REPORT;
}
- if (!has_mode) {
- mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
- }
if (!has_granularity) {
granularity = 0;
}
@@ -2936,35 +2926,73 @@ void qmp_drive_mirror(const char *device, const char *target,
return;
}
- blk = blk_by_name(device);
- if (!blk) {
- error_set(errp, QERR_DEVICE_NOT_FOUND, device);
- return;
- }
-
- aio_context = blk_get_aio_context(blk);
- aio_context_acquire(aio_context);
-
- if (!blk_is_available(blk)) {
- error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
- goto out;
- }
- bs = blk_bs(blk);
-
- if (!has_format) {
- format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
- }
- if (format) {
- drv = bdrv_find_format(format);
- if (!drv) {
- error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
- goto out;
- }
- }
-
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
+ return;
+ }
+
+ if (!bs->backing_hd && sync == MIRROR_SYNC_MODE_TOP) {
+ sync = MIRROR_SYNC_MODE_FULL;
+ }
+
+ /* pass the node name to replace to mirror start since it's loose coupling
+ * and will allow to check whether the node still exist at mirror completion
+ */
+ mirror_start(bs, target,
+ has_replaces ? replaces : NULL,
+ speed, granularity, buf_size, sync,
+ on_source_error, on_target_error,
+ block_job_cb, bs, errp);
+}
+
+void qmp_drive_mirror(const char *device, const char *target,
+ bool has_format, const char *format,
+ bool has_node_name, const char *node_name,
+ bool has_replaces, const char *replaces,
+ enum MirrorSyncMode sync,
+ bool has_mode, enum NewImageMode mode,
+ bool has_speed, int64_t speed,
+ bool has_granularity, uint32_t granularity,
+ bool has_buf_size, int64_t buf_size,
+ bool has_on_source_error, BlockdevOnError on_source_error,
+ bool has_on_target_error, BlockdevOnError on_target_error,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ BlockBackend *blk;
+ BlockDriverState *source, *target_bs;
+ AioContext *aio_context;
+ BlockDriver *drv = NULL;
+ Error *local_err = NULL;
+ QDict *options = NULL;
+ int flags;
+ int64_t size;
+ int ret;
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+ return;
+ }
+
+ aio_context = blk_get_aio_context(blk);
+ aio_context_acquire(aio_context);
+
+ if (!blk_is_available(blk)) {
+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
goto out;
}
+ bs = blk_bs(blk);
+
+ if (!has_format) {
+ format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
+ }
+ if (format) {
+ drv = bdrv_find_format(format);
+ if (!drv) {
+ error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
+ goto out;
+ }
+ }
flags = bs->open_flags | BDRV_O_RDWR;
source = bs->backing_hd;
@@ -2989,14 +3017,14 @@ void qmp_drive_mirror(const char *device, const char *target,
if (!has_node_name) {
error_setg(errp, "a node-name must be provided when replacing a"
" named node of the graph");
- goto out;
+ return;
}
to_replace_bs = check_to_replace_node(replaces, &local_err);
if (!to_replace_bs) {
error_propagate(errp, local_err);
- goto out;
+ return;
}
replace_aio_context = bdrv_get_aio_context(to_replace_bs);
@@ -3007,7 +3035,7 @@ void qmp_drive_mirror(const char *device, const char *target,
if (size != replace_size) {
error_setg(errp, "cannot replace image with a mirror image of "
"different size");
- goto out;
+ return;
}
}
@@ -3057,20 +3085,18 @@ void qmp_drive_mirror(const char *device, const char *target,
bdrv_set_aio_context(target_bs, aio_context);
- /* pass the node name to replace to mirror start since it's loose coupling
- * and will allow to check whether the node still exist at mirror completion
- */
- mirror_start(bs, target_bs,
- has_replaces ? replaces : NULL,
- speed, granularity, buf_size, sync,
- on_source_error, on_target_error,
- block_job_cb, bs, &local_err);
- if (local_err != NULL) {
- bdrv_unref(target_bs);
+ blockdev_mirror_common(bs, target_bs,
+ has_replaces, replaces, sync,
+ has_speed, speed,
+ has_granularity, granularity,
+ has_buf_size, buf_size,
+ has_on_source_error, on_source_error,
+ has_on_target_error, on_target_error,
+ &local_err);
+ if (local_err) {
error_propagate(errp, local_err);
- goto out;
+ bdrv_unref(target_bs);
}
-
out:
aio_context_release(aio_context);
}
--
2.4.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 4/6] block: Add check on mirror target
2015-06-09 2:13 [Qemu-devel] [PATCH 0/6] qmp: Add blockdev-mirror Fam Zheng
` (2 preceding siblings ...)
2015-06-09 2:13 ` [Qemu-devel] [PATCH 3/6] block: Extract blockdev part of qmp_drive_mirror Fam Zheng
@ 2015-06-09 2:13 ` Fam Zheng
2015-06-24 16:40 ` Max Reitz
2015-06-09 2:13 ` [Qemu-devel] [PATCH 5/6] qmp: Add blockdev-mirror command Fam Zheng
2015-06-09 2:13 ` [Qemu-devel] [PATCH 6/6] iotests: Add test cases for blockdev-mirror Fam Zheng
5 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2015-06-09 2:13 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster, mreitz,
Stefan Hajnoczi, pbonzini
Signed-off-by: Fam Zheng <famz@redhat.com>
---
blockdev.c | 3 +++
include/block/block.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index c32a9a9..44030da 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2929,6 +2929,9 @@ static void blockdev_mirror_common(BlockDriverState *bs,
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
return;
}
+ if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_MIRROR_TARGET, errp)) {
+ return;
+ }
if (!bs->backing_hd && sync == MIRROR_SYNC_MODE_TOP) {
sync = MIRROR_SYNC_MODE_FULL;
diff --git a/include/block/block.h b/include/block/block.h
index 69cb676..c96d47b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -156,6 +156,7 @@ typedef enum BlockOpType {
BLOCK_OP_TYPE_INTERNAL_SNAPSHOT,
BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE,
BLOCK_OP_TYPE_MIRROR_SOURCE,
+ BLOCK_OP_TYPE_MIRROR_TARGET,
BLOCK_OP_TYPE_RESIZE,
BLOCK_OP_TYPE_STREAM,
BLOCK_OP_TYPE_REPLACE,
--
2.4.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 5/6] qmp: Add blockdev-mirror command
2015-06-09 2:13 [Qemu-devel] [PATCH 0/6] qmp: Add blockdev-mirror Fam Zheng
` (3 preceding siblings ...)
2015-06-09 2:13 ` [Qemu-devel] [PATCH 4/6] block: Add check on mirror target Fam Zheng
@ 2015-06-09 2:13 ` Fam Zheng
2015-06-24 17:05 ` Max Reitz
2015-06-09 2:13 ` [Qemu-devel] [PATCH 6/6] iotests: Add test cases for blockdev-mirror Fam Zheng
5 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2015-06-09 2:13 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster, mreitz,
Stefan Hajnoczi, pbonzini
This will start a mirror job from a named device to another named
device, its relation with drive-mirror is similar with blockdev-backup
to drive-backup.
In blockdev-mirror, the target node should be prepared by blockdev-add,
which will be responsible for assigning a name to the new node, so
'node-name' in drive-mirror is dropped.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
blockdev.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
qapi/block-core.json | 47 ++++++++++++++++++++++++++++++++++++++++
qmp-commands.hx | 48 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 155 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index 44030da..6726b8d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2986,6 +2986,9 @@ void qmp_drive_mirror(const char *device, const char *target,
}
bs = blk_bs(blk);
+ if (!has_mode) {
+ mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+ }
if (!has_format) {
format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
}
@@ -3104,6 +3107,63 @@ out:
aio_context_release(aio_context);
}
+void qmp_blockdev_mirror(const char *device, const char *target,
+ bool has_replaces, const char *replaces,
+ MirrorSyncMode sync,
+ bool has_speed, int64_t speed,
+ bool has_granularity, uint32_t granularity,
+ bool has_buf_size, int64_t buf_size,
+ bool has_on_source_error,
+ BlockdevOnError on_source_error,
+ bool has_on_target_error,
+ BlockdevOnError on_target_error,
+ Error **errp)
+{
+ BlockDriverState *bs;
+ BlockBackend *blk;
+ BlockDriverState *target_bs;
+ AioContext *aio_context;
+ Error *local_err = NULL;
+
+ blk = blk_by_name(device);
+ if (!blk) {
+ error_setg(errp, "Device '%s' not found", device);
+ return;
+ }
+ bs = blk_bs(blk);
+
+ if (!bs) {
+ error_setg(errp, "Device '%s' has no media", device);
+ return;
+ }
+
+ target_bs = bdrv_lookup_bs(target, target, errp);
+ if (!target_bs) {
+ return;
+ }
+
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
+ bdrv_ref(target_bs);
+ bdrv_set_aio_context(target_bs, aio_context);
+
+ blockdev_mirror_common(bs, target_bs,
+ has_replaces, replaces, sync,
+ has_speed, speed,
+ has_granularity, granularity,
+ has_buf_size, buf_size,
+ has_on_source_error, on_source_error,
+ has_on_target_error, on_target_error,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ bdrv_unref(target_bs);
+ }
+
+ aio_context_release(aio_context);
+}
+
/* Get the block job for a given device name and acquire its AioContext */
static BlockJob *find_block_job(const char *device, AioContext **aio_context,
Error **errp)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b5c4559..bd163f7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1059,6 +1059,53 @@
'data': 'BlockDirtyBitmap' }
##
+# @blockdev-mirror
+#
+# Start mirroring a block device's writes to a new destination.
+#
+# @device: the name of the device whose writes should be mirrored.
+#
+# @target: the name of the device to mirror to.
+#
+# @replaces: #optional with sync=full graph node name to be replaced by the new
+# image when a whole image copy is done. This can be used to repair
+# broken Quorum files.
+#
+# @speed: #optional the maximum speed, in bytes per second
+#
+# @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).
+#
+# @granularity: #optional granularity of the dirty bitmap, default is 64K
+# if the image format doesn't have clusters, 4K if the clusters
+# are smaller than that, else the cluster size. Must be a
+# power of 2 between 512 and 64M
+#
+# @buf-size: #optional maximum amount of data in flight from source to
+# target
+#
+# @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).
+#
+# @on-target-error: #optional the action to take on an error on the target,
+# default 'report' (no limitations, since this applies to
+# a different block device than @device).
+#
+# Returns: nothing on success; error message on failure.
+#
+# Since 2.4
+##
+{ 'command': 'blockdev-mirror',
+ 'data': { 'device': 'str', 'target': 'str',
+ '*replaces': 'str',
+ 'sync': 'MirrorSyncMode',
+ '*speed': 'int', '*granularity': 'uint32',
+ '*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
+ '*on-target-error': 'BlockdevOnError' } }
+
+##
# @block_set_io_throttle:
#
# Change I/O throttle limits for a block drive.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 90e0135..646db78 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1560,6 +1560,54 @@ Example:
EQMP
{
+ .name = "blockdev-mirror",
+ .args_type = "sync:s,device:B,target:B,replaces:s?,speed:i?,"
+ "on-source-error:s?,on-target-error:s?,"
+ "granularity:i?,buf-size:i?",
+ .mhandler.cmd_new = qmp_marshal_input_blockdev_mirror,
+ },
+
+SQMP
+blockdev-mirror
+------------
+
+Start mirroring a block device's writes to another block device. target
+specifies the target of mirror operation.
+
+Arguments:
+
+- "device": device name to operate on (json-string)
+- "target": device name to mirror to (json-string)
+- "replaces": the block driver node name to replace when finished
+ (json-string, optional)
+- "speed": maximum speed of the streaming job, in bytes per second
+ (json-int)
+- "granularity": granularity of the dirty bitmap, in bytes (json-int, optional)
+- "buf_size": maximum amount of data in flight from source to target, in bytes
+ (json-int, default 10M)
+- "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).
+- "on-source-error": the action to take on an error on the source
+ (BlockdevOnError, default 'report')
+- "on-target-error": the action to take on an error on the target
+ (BlockdevOnError, default 'report')
+
+The default value of the granularity is the image cluster size clamped
+between 4096 and 65536, if the image format defines one. If the format
+does not define a cluster size, the default value of the granularity
+is 65536.
+
+Example:
+
+-> { "execute": "blockdev-mirror", "arguments": { "device": "ide-hd0",
+ "target": "target0",
+ "sync": "full" } }
+<- { "return": {} }
+
+EQMP
+ {
.name = "change-backing-file",
.args_type = "device:s,image-node-name:s,backing-file:s",
.mhandler.cmd_new = qmp_marshal_input_change_backing_file,
--
2.4.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 6/6] iotests: Add test cases for blockdev-mirror
2015-06-09 2:13 [Qemu-devel] [PATCH 0/6] qmp: Add blockdev-mirror Fam Zheng
` (4 preceding siblings ...)
2015-06-09 2:13 ` [Qemu-devel] [PATCH 5/6] qmp: Add blockdev-mirror command Fam Zheng
@ 2015-06-09 2:13 ` Fam Zheng
2015-06-24 17:10 ` Max Reitz
5 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2015-06-09 2:13 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster, mreitz,
Stefan Hajnoczi, pbonzini
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/qemu-iotests/041 | 100 +++++++++++++++++++++++++++++++++++----------
tests/qemu-iotests/041.out | 4 +-
2 files changed, 80 insertions(+), 24 deletions(-)
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 59a8f73..922f53c 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -65,8 +65,23 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
event = self.wait_until_completed(drive=drive)
self.assert_qmp(event, 'data/type', 'mirror')
+ def blockdev_add(self, filename, drive_id=None, node_name=None):
+ options = {'driver': iotests.imgfmt,
+ 'file': {
+ 'filename': filename,
+ 'driver': 'file'}}
+ if drive_id:
+ options['id'] = drive_id
+ if node_name:
+ options['node-name'] = node_name
+ result = self.vm.qmp('blockdev-add', options=options)
+ self.assert_qmp(result, 'return', {})
+
class TestSingleDrive(ImageMirroringTestCase):
image_len = 1 * 1024 * 1024 # MB
+ qmp_cmd = 'drive-mirror'
+ qmp_target = target_img
+ not_found_error = 'DeviceNotFound'
def setUp(self):
iotests.create_image(backing_img, self.image_len)
@@ -86,8 +101,8 @@ class TestSingleDrive(ImageMirroringTestCase):
def test_complete(self):
self.assert_no_active_block_jobs()
- result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- target=target_img)
+ result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ target=self.qmp_target)
self.assert_qmp(result, 'return', {})
self.complete_and_wait()
@@ -100,8 +115,8 @@ class TestSingleDrive(ImageMirroringTestCase):
def test_cancel(self):
self.assert_no_active_block_jobs()
- result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- target=target_img)
+ result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ target=self.qmp_target)
self.assert_qmp(result, 'return', {})
self.cancel_and_wait(force=True)
@@ -112,8 +127,8 @@ class TestSingleDrive(ImageMirroringTestCase):
def test_cancel_after_ready(self):
self.assert_no_active_block_jobs()
- result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- target=target_img)
+ result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ target=self.qmp_target)
self.assert_qmp(result, 'return', {})
self.wait_ready_and_cancel()
@@ -126,8 +141,8 @@ class TestSingleDrive(ImageMirroringTestCase):
def test_pause(self):
self.assert_no_active_block_jobs()
- result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- target=target_img)
+ result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ target=self.qmp_target)
self.assert_qmp(result, 'return', {})
result = self.vm.qmp('block-job-pause', device='drive0')
@@ -153,8 +168,8 @@ class TestSingleDrive(ImageMirroringTestCase):
self.assert_no_active_block_jobs()
# A small buffer is rounded up automatically
- result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- buf_size=4096, target=target_img)
+ result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ buf_size=4096, target=self.qmp_target)
self.assert_qmp(result, 'return', {})
self.complete_and_wait()
@@ -169,8 +184,8 @@ class TestSingleDrive(ImageMirroringTestCase):
qemu_img('create', '-f', iotests.imgfmt, '-o', 'cluster_size=%d,size=%d'
% (self.image_len, self.image_len), target_img)
- result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- buf_size=65536, mode='existing', target=target_img)
+ result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ buf_size=65536, mode='existing', target=self.qmp_target)
self.assert_qmp(result, 'return', {})
self.complete_and_wait()
@@ -185,8 +200,8 @@ class TestSingleDrive(ImageMirroringTestCase):
qemu_img('create', '-f', iotests.imgfmt, '-o', 'cluster_size=%d,backing_file=%s'
% (self.image_len, backing_img), target_img)
- result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- mode='existing', target=target_img)
+ result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ mode='existing', target=self.qmp_target)
self.assert_qmp(result, 'return', {})
self.complete_and_wait()
@@ -197,30 +212,72 @@ class TestSingleDrive(ImageMirroringTestCase):
'target image does not match source after mirroring')
def test_medium_not_found(self):
- result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full',
- target=target_img)
+ result = self.vm.qmp(self.qmp_cmd, device='ide1-cd0', sync='full',
+ target=self.qmp_target)
self.assert_qmp(result, 'error/class', 'GenericError')
def test_image_not_found(self):
- result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
- mode='existing', target=target_img)
+ result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
+ mode='existing', target=self.qmp_target)
self.assert_qmp(result, 'error/class', 'GenericError')
def test_device_not_found(self):
- result = self.vm.qmp('drive-mirror', device='nonexistent', sync='full',
- target=target_img)
- self.assert_qmp(result, 'error/class', 'DeviceNotFound')
+ result = self.vm.qmp(self.qmp_cmd, device='nonexistent', sync='full',
+ target=self.qmp_target)
+ self.assert_qmp(result, 'error/class', self.not_found_error)
+
+class TestSingleBlockdev(TestSingleDrive):
+ qmp_cmd = 'blockdev-mirror'
+ qmp_target = 'drive1'
+ not_found_error = 'GenericError'
+
+ def setUp(self):
+ TestSingleDrive.setUp(self)
+ qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, target_img)
+ self.blockdev_add(target_img, node_name='drive1')
+
+ test_large_cluster = None
+ test_image_not_found = None
+ test_small_buffer2 = None
+
+class TestBlockdevAttached(ImageMirroringTestCase):
+ image_len = 1 * 1024 * 1024 # MB
+
+ def setUp(self):
+ iotests.create_image(backing_img, self.image_len)
+ qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
+ qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, target_img)
+ self.vm = iotests.VM().add_drive(test_img)
+ self.vm.launch()
+
+ def tearDown(self):
+ self.vm.shutdown()
+ os.remove(test_img)
+ os.remove(target_img)
+
+ def test_blockdev_attached(self):
+ self.assert_no_active_block_jobs()
+ self.blockdev_add(target_img, drive_id='drive1')
+ result = self.vm.qmp('blockdev-mirror', device='drive0', sync='full',
+ target='drive1')
+ self.assert_qmp(result, 'error/class', 'GenericError')
class TestSingleDriveZeroLength(TestSingleDrive):
image_len = 0
test_small_buffer2 = None
test_large_cluster = None
+class TestSingleBlockdevZeroLength(TestSingleBlockdev):
+ image_len = 0
+
class TestSingleDriveUnalignedLength(TestSingleDrive):
image_len = 1025 * 1024
test_small_buffer2 = None
test_large_cluster = None
+class TestSingleBlockdevUnalignedLength(TestSingleBlockdev):
+ image_len = 1025 * 1024
+
class TestMirrorNoBacking(ImageMirroringTestCase):
image_len = 2 * 1024 * 1024 # MB
@@ -949,6 +1006,5 @@ class TestRepairQuorum(ImageMirroringTestCase):
# TODO: a better test requiring some QEMU infrastructure will be added
# to check that this file is really driven by quorum
self.vm.shutdown()
-
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
index 24093bc..b67d050 100644
--- a/tests/qemu-iotests/041.out
+++ b/tests/qemu-iotests/041.out
@@ -1,5 +1,5 @@
-......................................................
+............................................................................
----------------------------------------------------------------------
-Ran 54 tests
+Ran 76 tests
OK
--
2.4.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] block: Add blocker on mirror target
2015-06-09 2:13 ` [Qemu-devel] [PATCH 1/6] block: Add blocker on mirror target Fam Zheng
@ 2015-06-24 16:14 ` Max Reitz
2015-06-25 2:27 ` Fam Zheng
0 siblings, 1 reply; 15+ messages in thread
From: Max Reitz @ 2015-06-24 16:14 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster,
Stefan Hajnoczi, pbonzini
On 09.06.2015 04:13, Fam Zheng wrote:
> In block/backup.c, we already check and add blocker on the target bs,
> which is necessary so that it won't be intervened with other operations.
>
> In block/mirror.c we should also protect the mirror target bs, because it
> could have a node-name (drive-mirror ... node-name=XXX), and on top of
> that it's safe to add blockdev-mirror.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block/mirror.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index 33c640f..8b23ddd 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -360,6 +360,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
> aio_context_release(replace_aio_context);
> }
> g_free(s->replaces);
> + bdrv_op_unblock_all(s->target, s->common.blocker);
> bdrv_unref(s->target);
> block_job_completed(&s->common, data->ret);
> g_free(data);
> @@ -682,6 +683,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
> return;
> }
>
> + bdrv_op_block_all(target, s->common.blocker);
> s->replaces = g_strdup(replaces);
> s->on_source_error = on_source_error;
> s->on_target_error = on_target_error;
Do we need a bdrv_op_unblock_all() if bdrv_create_dirty_bitmap() fails?
Max
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] block: Rename BLOCK_OP_TYPE_MIRROR to BLOCK_OP_TYPE_MIRROR_SOURCE
2015-06-09 2:13 ` [Qemu-devel] [PATCH 2/6] block: Rename BLOCK_OP_TYPE_MIRROR to BLOCK_OP_TYPE_MIRROR_SOURCE Fam Zheng
@ 2015-06-24 16:18 ` Max Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2015-06-24 16:18 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster,
Stefan Hajnoczi, pbonzini
On 09.06.2015 04:13, Fam Zheng wrote:
> It's necessary to distinguish source and target before we can add
> blockdev-mirror, because we would want a concrete type of operation to
> check on target bs before starting.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> blockdev.c | 2 +-
> hw/block/dataplane/virtio-blk.c | 2 +-
> include/block/block.h | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/6] block: Extract blockdev part of qmp_drive_mirror
2015-06-09 2:13 ` [Qemu-devel] [PATCH 3/6] block: Extract blockdev part of qmp_drive_mirror Fam Zheng
@ 2015-06-24 16:34 ` Max Reitz
2015-06-25 2:33 ` Fam Zheng
0 siblings, 1 reply; 15+ messages in thread
From: Max Reitz @ 2015-06-24 16:34 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster,
Stefan Hajnoczi, pbonzini
On 09.06.2015 04:13, Fam Zheng wrote:
> This is the part that will be reused by blockdev-mirror.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> blockdev.c | 158 +++++++++++++++++++++++++++++++++++--------------------------
> 1 file changed, 92 insertions(+), 66 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index b573e56..c32a9a9 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2883,29 +2883,22 @@ out:
>
> #define DEFAULT_MIRROR_BUF_SIZE (10 << 20)
>
> -void qmp_drive_mirror(const char *device, const char *target,
> - bool has_format, const char *format,
> - bool has_node_name, const char *node_name,
> - bool has_replaces, const char *replaces,
> - enum MirrorSyncMode sync,
> - bool has_mode, enum NewImageMode mode,
> - bool has_speed, int64_t speed,
> - bool has_granularity, uint32_t granularity,
> - bool has_buf_size, int64_t buf_size,
> - bool has_on_source_error, BlockdevOnError on_source_error,
> - bool has_on_target_error, BlockdevOnError on_target_error,
> - Error **errp)
> +/* Parameter check and block job starting for mirroring.
> + * Caller should hold @device and @target's aio context (They must to be on the
> + * same aio context). */
> +static void blockdev_mirror_common(BlockDriverState *bs,
> + BlockDriverState *target,
> + bool has_replaces, const char *replaces,
> + enum MirrorSyncMode sync,
> + bool has_speed, int64_t speed,
> + bool has_granularity, uint32_t granularity,
> + bool has_buf_size, int64_t buf_size,
> + bool has_on_source_error,
> + BlockdevOnError on_source_error,
> + bool has_on_target_error,
> + BlockdevOnError on_target_error,
> + Error **errp)
> {
> - BlockBackend *blk;
> - BlockDriverState *bs;
> - BlockDriverState *source, *target_bs;
> - AioContext *aio_context;
> - BlockDriver *drv = NULL;
> - Error *local_err = NULL;
> - QDict *options = NULL;
> - int flags;
> - int64_t size;
> - int ret;
>
> if (!has_speed) {
> speed = 0;
> @@ -2916,9 +2909,6 @@ void qmp_drive_mirror(const char *device, const char *target,
> if (!has_on_target_error) {
> on_target_error = BLOCKDEV_ON_ERROR_REPORT;
> }
> - if (!has_mode) {
> - mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
> - }
> if (!has_granularity) {
> granularity = 0;
> }
> @@ -2936,35 +2926,73 @@ void qmp_drive_mirror(const char *device, const char *target,
> return;
> }
>
> - blk = blk_by_name(device);
> - if (!blk) {
> - error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> - return;
> - }
> -
> - aio_context = blk_get_aio_context(blk);
> - aio_context_acquire(aio_context);
> -
> - if (!blk_is_available(blk)) {
> - error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> - goto out;
> - }
> - bs = blk_bs(blk);
> -
> - if (!has_format) {
> - format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
> - }
> - if (format) {
> - drv = bdrv_find_format(format);
> - if (!drv) {
> - error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
> - goto out;
> - }
> - }
> -
> if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
> + return;
> + }
> +
> + if (!bs->backing_hd && sync == MIRROR_SYNC_MODE_TOP) {
> + sync = MIRROR_SYNC_MODE_FULL;
> + }
> +
> + /* pass the node name to replace to mirror start since it's loose coupling
> + * and will allow to check whether the node still exist at mirror completion
> + */
> + mirror_start(bs, target,
> + has_replaces ? replaces : NULL,
> + speed, granularity, buf_size, sync,
> + on_source_error, on_target_error,
> + block_job_cb, bs, errp);
> +}
> +
> +void qmp_drive_mirror(const char *device, const char *target,
> + bool has_format, const char *format,
> + bool has_node_name, const char *node_name,
> + bool has_replaces, const char *replaces,
> + enum MirrorSyncMode sync,
> + bool has_mode, enum NewImageMode mode,
> + bool has_speed, int64_t speed,
> + bool has_granularity, uint32_t granularity,
> + bool has_buf_size, int64_t buf_size,
> + bool has_on_source_error, BlockdevOnError on_source_error,
> + bool has_on_target_error, BlockdevOnError on_target_error,
> + Error **errp)
> +{
> + BlockDriverState *bs;
> + BlockBackend *blk;
> + BlockDriverState *source, *target_bs;
> + AioContext *aio_context;
> + BlockDriver *drv = NULL;
> + Error *local_err = NULL;
> + QDict *options = NULL;
> + int flags;
> + int64_t size;
> + int ret;
> +
> + blk = blk_by_name(device);
> + if (!blk) {
> + error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> + return;
> + }
> +
> + aio_context = blk_get_aio_context(blk);
> + aio_context_acquire(aio_context);
> +
> + if (!blk_is_available(blk)) {
> + error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> goto out;
> }
> + bs = blk_bs(blk);
> +
> + if (!has_format) {
> + format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
> + }
> + if (format) {
> + drv = bdrv_find_format(format);
> + if (!drv) {
> + error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
> + goto out;
> + }
> + }
>
> flags = bs->open_flags | BDRV_O_RDWR;
> source = bs->backing_hd;
> @@ -2989,14 +3017,14 @@ void qmp_drive_mirror(const char *device, const char *target,
> if (!has_node_name) {
> error_setg(errp, "a node-name must be provided when replacing a"
> " named node of the graph");
> - goto out;
> + return;
> }
>
> to_replace_bs = check_to_replace_node(replaces, &local_err);
>
> if (!to_replace_bs) {
> error_propagate(errp, local_err);
> - goto out;
> + return;
> }
>
> replace_aio_context = bdrv_get_aio_context(to_replace_bs);
> @@ -3007,7 +3035,7 @@ void qmp_drive_mirror(const char *device, const char *target,
> if (size != replace_size) {
> error_setg(errp, "cannot replace image with a mirror image of "
> "different size");
> - goto out;
> + return;
> }
> }
There are still a couple of "return;" statements left in the "if
(has_replaces)" block. I think they should be replaced, too.
Max
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/6] block: Add check on mirror target
2015-06-09 2:13 ` [Qemu-devel] [PATCH 4/6] block: Add check on mirror target Fam Zheng
@ 2015-06-24 16:40 ` Max Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2015-06-24 16:40 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster,
Stefan Hajnoczi, pbonzini
On 09.06.2015 04:13, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> blockdev.c | 3 +++
> include/block/block.h | 1 +
> 2 files changed, 4 insertions(+)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] qmp: Add blockdev-mirror command
2015-06-09 2:13 ` [Qemu-devel] [PATCH 5/6] qmp: Add blockdev-mirror command Fam Zheng
@ 2015-06-24 17:05 ` Max Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2015-06-24 17:05 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster,
Stefan Hajnoczi, pbonzini
On 09.06.2015 04:13, Fam Zheng wrote:
> This will start a mirror job from a named device to another named
> device, its relation with drive-mirror is similar with blockdev-backup
> to drive-backup.
>
> In blockdev-mirror, the target node should be prepared by blockdev-add,
> which will be responsible for assigning a name to the new node, so
> 'node-name' in drive-mirror is dropped.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> blockdev.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> qapi/block-core.json | 47 ++++++++++++++++++++++++++++++++++++++++
> qmp-commands.hx | 48 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 155 insertions(+)
>
> diff --git a/blockdev.c b/blockdev.c
> index 44030da..6726b8d 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2986,6 +2986,9 @@ void qmp_drive_mirror(const char *device, const char *target,
> }
> bs = blk_bs(blk);
>
> + if (!has_mode) {
> + mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
> + }
I think this hunk should be in patch 3, not here.
Other than that, looks good. (apart from a pedantic comment below)
Max
> if (!has_format) {
> format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
> }
> @@ -3104,6 +3107,63 @@ out:
> aio_context_release(aio_context);
> }
>
> +void qmp_blockdev_mirror(const char *device, const char *target,
> + bool has_replaces, const char *replaces,
> + MirrorSyncMode sync,
> + bool has_speed, int64_t speed,
> + bool has_granularity, uint32_t granularity,
> + bool has_buf_size, int64_t buf_size,
> + bool has_on_source_error,
> + BlockdevOnError on_source_error,
> + bool has_on_target_error,
> + BlockdevOnError on_target_error,
> + Error **errp)
> +{
> + BlockDriverState *bs;
> + BlockBackend *blk;
> + BlockDriverState *target_bs;
> + AioContext *aio_context;
> + Error *local_err = NULL;
> +
> + blk = blk_by_name(device);
> + if (!blk) {
> + error_setg(errp, "Device '%s' not found", device);
> + return;
> + }
> + bs = blk_bs(blk);
> +
> + if (!bs) {
> + error_setg(errp, "Device '%s' has no media", device);
> + return;
> + }
> +
> + target_bs = bdrv_lookup_bs(target, target, errp);
> + if (!target_bs) {
> + return;
> + }
> +
> + aio_context = bdrv_get_aio_context(bs);
> + aio_context_acquire(aio_context);
> +
> + bdrv_ref(target_bs);
> + bdrv_set_aio_context(target_bs, aio_context);
> +
> + blockdev_mirror_common(bs, target_bs,
> + has_replaces, replaces, sync,
> + has_speed, speed,
> + has_granularity, granularity,
> + has_buf_size, buf_size,
> + has_on_source_error, on_source_error,
> + has_on_target_error, on_target_error,
> + &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + bdrv_unref(target_bs);
> + }
> +
> + aio_context_release(aio_context);
> +}
> +
> /* Get the block job for a given device name and acquire its AioContext */
> static BlockJob *find_block_job(const char *device, AioContext **aio_context,
> Error **errp)
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index b5c4559..bd163f7 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1059,6 +1059,53 @@
> 'data': 'BlockDirtyBitmap' }
>
> ##
> +# @blockdev-mirror
> +#
> +# Start mirroring a block device's writes to a new destination.
> +#
> +# @device: the name of the device whose writes should be mirrored.
Superfluous space here. (</pedantic>)
> +#
> +# @target: the name of the device to mirror to.
> +#
> +# @replaces: #optional with sync=full graph node name to be replaced by the new
> +# image when a whole image copy is done. This can be used to repair
> +# broken Quorum files.
> +#
> +# @speed: #optional the maximum speed, in bytes per second
> +#
> +# @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).
> +#
> +# @granularity: #optional granularity of the dirty bitmap, default is 64K
> +# if the image format doesn't have clusters, 4K if the clusters
> +# are smaller than that, else the cluster size. Must be a
> +# power of 2 between 512 and 64M
> +#
> +# @buf-size: #optional maximum amount of data in flight from source to
> +# target
> +#
> +# @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).
> +#
> +# @on-target-error: #optional the action to take on an error on the target,
> +# default 'report' (no limitations, since this applies to
> +# a different block device than @device).
> +#
> +# Returns: nothing on success; error message on failure.
> +#
> +# Since 2.4
> +##
> +{ 'command': 'blockdev-mirror',
> + 'data': { 'device': 'str', 'target': 'str',
> + '*replaces': 'str',
> + 'sync': 'MirrorSyncMode',
> + '*speed': 'int', '*granularity': 'uint32',
> + '*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
> + '*on-target-error': 'BlockdevOnError' } }
> +
> +##
> # @block_set_io_throttle:
> #
> # Change I/O throttle limits for a block drive.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 90e0135..646db78 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1560,6 +1560,54 @@ Example:
> EQMP
>
> {
> + .name = "blockdev-mirror",
> + .args_type = "sync:s,device:B,target:B,replaces:s?,speed:i?,"
> + "on-source-error:s?,on-target-error:s?,"
> + "granularity:i?,buf-size:i?",
> + .mhandler.cmd_new = qmp_marshal_input_blockdev_mirror,
> + },
> +
> +SQMP
> +blockdev-mirror
> +------------
> +
> +Start mirroring a block device's writes to another block device. target
> +specifies the target of mirror operation.
> +
> +Arguments:
> +
> +- "device": device name to operate on (json-string)
> +- "target": device name to mirror to (json-string)
> +- "replaces": the block driver node name to replace when finished
> + (json-string, optional)
> +- "speed": maximum speed of the streaming job, in bytes per second
> + (json-int)
> +- "granularity": granularity of the dirty bitmap, in bytes (json-int, optional)
> +- "buf_size": maximum amount of data in flight from source to target, in bytes
> + (json-int, default 10M)
> +- "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).
> +- "on-source-error": the action to take on an error on the source
> + (BlockdevOnError, default 'report')
> +- "on-target-error": the action to take on an error on the target
> + (BlockdevOnError, default 'report')
> +
> +The default value of the granularity is the image cluster size clamped
> +between 4096 and 65536, if the image format defines one. If the format
> +does not define a cluster size, the default value of the granularity
> +is 65536.
> +
> +Example:
> +
> +-> { "execute": "blockdev-mirror", "arguments": { "device": "ide-hd0",
> + "target": "target0",
> + "sync": "full" } }
> +<- { "return": {} }
> +
> +EQMP
> + {
> .name = "change-backing-file",
> .args_type = "device:s,image-node-name:s,backing-file:s",
> .mhandler.cmd_new = qmp_marshal_input_change_backing_file,
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 6/6] iotests: Add test cases for blockdev-mirror
2015-06-09 2:13 ` [Qemu-devel] [PATCH 6/6] iotests: Add test cases for blockdev-mirror Fam Zheng
@ 2015-06-24 17:10 ` Max Reitz
0 siblings, 0 replies; 15+ messages in thread
From: Max Reitz @ 2015-06-24 17:10 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Kevin Wolf, qemu-block, Jeff Cody, Markus Armbruster,
Stefan Hajnoczi, pbonzini
On 09.06.2015 04:13, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> tests/qemu-iotests/041 | 100 +++++++++++++++++++++++++++++++++++----------
> tests/qemu-iotests/041.out | 4 +-
> 2 files changed, 80 insertions(+), 24 deletions(-)
>
> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
> index 59a8f73..922f53c 100755
> --- a/tests/qemu-iotests/041
> +++ b/tests/qemu-iotests/041
> @@ -65,8 +65,23 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
> event = self.wait_until_completed(drive=drive)
> self.assert_qmp(event, 'data/type', 'mirror')
>
> + def blockdev_add(self, filename, drive_id=None, node_name=None):
> + options = {'driver': iotests.imgfmt,
> + 'file': {
> + 'filename': filename,
> + 'driver': 'file'}}
> + if drive_id:
> + options['id'] = drive_id
> + if node_name:
> + options['node-name'] = node_name
> + result = self.vm.qmp('blockdev-add', options=options)
> + self.assert_qmp(result, 'return', {})
> +
> class TestSingleDrive(ImageMirroringTestCase):
> image_len = 1 * 1024 * 1024 # MB
> + qmp_cmd = 'drive-mirror'
> + qmp_target = target_img
> + not_found_error = 'DeviceNotFound'
>
> def setUp(self):
> iotests.create_image(backing_img, self.image_len)
> @@ -86,8 +101,8 @@ class TestSingleDrive(ImageMirroringTestCase):
> def test_complete(self):
> self.assert_no_active_block_jobs()
>
> - result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
> - target=target_img)
> + result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
> + target=self.qmp_target)
> self.assert_qmp(result, 'return', {})
>
> self.complete_and_wait()
> @@ -100,8 +115,8 @@ class TestSingleDrive(ImageMirroringTestCase):
> def test_cancel(self):
> self.assert_no_active_block_jobs()
>
> - result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
> - target=target_img)
> + result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
> + target=self.qmp_target)
> self.assert_qmp(result, 'return', {})
>
> self.cancel_and_wait(force=True)
> @@ -112,8 +127,8 @@ class TestSingleDrive(ImageMirroringTestCase):
> def test_cancel_after_ready(self):
> self.assert_no_active_block_jobs()
>
> - result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
> - target=target_img)
> + result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
> + target=self.qmp_target)
> self.assert_qmp(result, 'return', {})
>
> self.wait_ready_and_cancel()
> @@ -126,8 +141,8 @@ class TestSingleDrive(ImageMirroringTestCase):
> def test_pause(self):
> self.assert_no_active_block_jobs()
>
> - result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
> - target=target_img)
> + result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
> + target=self.qmp_target)
> self.assert_qmp(result, 'return', {})
>
> result = self.vm.qmp('block-job-pause', device='drive0')
> @@ -153,8 +168,8 @@ class TestSingleDrive(ImageMirroringTestCase):
> self.assert_no_active_block_jobs()
>
> # A small buffer is rounded up automatically
> - result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
> - buf_size=4096, target=target_img)
> + result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
> + buf_size=4096, target=self.qmp_target)
> self.assert_qmp(result, 'return', {})
>
> self.complete_and_wait()
> @@ -169,8 +184,8 @@ class TestSingleDrive(ImageMirroringTestCase):
>
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'cluster_size=%d,size=%d'
> % (self.image_len, self.image_len), target_img)
> - result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
> - buf_size=65536, mode='existing', target=target_img)
> + result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
> + buf_size=65536, mode='existing', target=self.qmp_target)
> self.assert_qmp(result, 'return', {})
>
> self.complete_and_wait()
> @@ -185,8 +200,8 @@ class TestSingleDrive(ImageMirroringTestCase):
>
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'cluster_size=%d,backing_file=%s'
> % (self.image_len, backing_img), target_img)
> - result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
> - mode='existing', target=target_img)
> + result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
> + mode='existing', target=self.qmp_target)
> self.assert_qmp(result, 'return', {})
>
> self.complete_and_wait()
> @@ -197,30 +212,72 @@ class TestSingleDrive(ImageMirroringTestCase):
> 'target image does not match source after mirroring')
>
> def test_medium_not_found(self):
> - result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full',
> - target=target_img)
> + result = self.vm.qmp(self.qmp_cmd, device='ide1-cd0', sync='full',
> + target=self.qmp_target)
> self.assert_qmp(result, 'error/class', 'GenericError')
>
> def test_image_not_found(self):
> - result = self.vm.qmp('drive-mirror', device='drive0', sync='full',
> - mode='existing', target=target_img)
> + result = self.vm.qmp(self.qmp_cmd, device='drive0', sync='full',
> + mode='existing', target=self.qmp_target)
> self.assert_qmp(result, 'error/class', 'GenericError')
>
> def test_device_not_found(self):
> - result = self.vm.qmp('drive-mirror', device='nonexistent', sync='full',
> - target=target_img)
> - self.assert_qmp(result, 'error/class', 'DeviceNotFound')
> + result = self.vm.qmp(self.qmp_cmd, device='nonexistent', sync='full',
> + target=self.qmp_target)
> + self.assert_qmp(result, 'error/class', self.not_found_error)
> +
> +class TestSingleBlockdev(TestSingleDrive):
> + qmp_cmd = 'blockdev-mirror'
> + qmp_target = 'drive1'
> + not_found_error = 'GenericError'
> +
> + def setUp(self):
> + TestSingleDrive.setUp(self)
> + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, target_img)
> + self.blockdev_add(target_img, node_name='drive1')
> +
> + test_large_cluster = None
> + test_image_not_found = None
> + test_small_buffer2 = None
> +
> +class TestBlockdevAttached(ImageMirroringTestCase):
> + image_len = 1 * 1024 * 1024 # MB
> +
> + def setUp(self):
> + iotests.create_image(backing_img, self.image_len)
> + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
> + qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, target_img)
> + self.vm = iotests.VM().add_drive(test_img)
> + self.vm.launch()
> +
> + def tearDown(self):
> + self.vm.shutdown()
> + os.remove(test_img)
> + os.remove(target_img)
> +
> + def test_blockdev_attached(self):
> + self.assert_no_active_block_jobs()
> + self.blockdev_add(target_img, drive_id='drive1')
> + result = self.vm.qmp('blockdev-mirror', device='drive0', sync='full',
> + target='drive1')
> + self.assert_qmp(result, 'error/class', 'GenericError')
>
> class TestSingleDriveZeroLength(TestSingleDrive):
> image_len = 0
> test_small_buffer2 = None
> test_large_cluster = None
>
> +class TestSingleBlockdevZeroLength(TestSingleBlockdev):
> + image_len = 0
> +
> class TestSingleDriveUnalignedLength(TestSingleDrive):
> image_len = 1025 * 1024
> test_small_buffer2 = None
> test_large_cluster = None
>
> +class TestSingleBlockdevUnalignedLength(TestSingleBlockdev):
> + image_len = 1025 * 1024
> +
> class TestMirrorNoBacking(ImageMirroringTestCase):
> image_len = 2 * 1024 * 1024 # MB
>
> @@ -949,6 +1006,5 @@ class TestRepairQuorum(ImageMirroringTestCase):
> # TODO: a better test requiring some QEMU infrastructure will be added
> # to check that this file is really driven by quorum
> self.vm.shutdown()
> -
Intentional?
Without this hunk:
Reviewed-by: Max Reitz <mreitz@redhat.com>
> if __name__ == '__main__':
> iotests.main(supported_fmts=['qcow2', 'qed'])
> diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
> index 24093bc..b67d050 100644
> --- a/tests/qemu-iotests/041.out
> +++ b/tests/qemu-iotests/041.out
> @@ -1,5 +1,5 @@
> -......................................................
> +............................................................................
> ----------------------------------------------------------------------
> -Ran 54 tests
> +Ran 76 tests
>
> OK
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] block: Add blocker on mirror target
2015-06-24 16:14 ` Max Reitz
@ 2015-06-25 2:27 ` Fam Zheng
0 siblings, 0 replies; 15+ messages in thread
From: Fam Zheng @ 2015-06-25 2:27 UTC (permalink / raw)
To: Max Reitz
Cc: Kevin Wolf, qemu-block, Jeff Cody, qemu-devel, Markus Armbruster,
Stefan Hajnoczi, pbonzini
On Wed, 06/24 18:14, Max Reitz wrote:
> On 09.06.2015 04:13, Fam Zheng wrote:
> >In block/backup.c, we already check and add blocker on the target bs,
> >which is necessary so that it won't be intervened with other operations.
> >
> >In block/mirror.c we should also protect the mirror target bs, because it
> >could have a node-name (drive-mirror ... node-name=XXX), and on top of
> >that it's safe to add blockdev-mirror.
> >
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >---
> > block/mirror.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> >diff --git a/block/mirror.c b/block/mirror.c
> >index 33c640f..8b23ddd 100644
> >--- a/block/mirror.c
> >+++ b/block/mirror.c
> >@@ -360,6 +360,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
> > aio_context_release(replace_aio_context);
> > }
> > g_free(s->replaces);
> >+ bdrv_op_unblock_all(s->target, s->common.blocker);
> > bdrv_unref(s->target);
> > block_job_completed(&s->common, data->ret);
> > g_free(data);
> >@@ -682,6 +683,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
> > return;
> > }
> >+ bdrv_op_block_all(target, s->common.blocker);
> > s->replaces = g_strdup(replaces);
> > s->on_source_error = on_source_error;
> > s->on_target_error = on_target_error;
>
> Do we need a bdrv_op_unblock_all() if bdrv_create_dirty_bitmap() fails?
>
Yes, good catch!
Fam
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/6] block: Extract blockdev part of qmp_drive_mirror
2015-06-24 16:34 ` Max Reitz
@ 2015-06-25 2:33 ` Fam Zheng
0 siblings, 0 replies; 15+ messages in thread
From: Fam Zheng @ 2015-06-25 2:33 UTC (permalink / raw)
To: Max Reitz
Cc: Kevin Wolf, qemu-block, Jeff Cody, qemu-devel, Markus Armbruster,
Stefan Hajnoczi, pbonzini
On Wed, 06/24 18:34, Max Reitz wrote:
> On 09.06.2015 04:13, Fam Zheng wrote:
> >This is the part that will be reused by blockdev-mirror.
> >
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >---
> > blockdev.c | 158 +++++++++++++++++++++++++++++++++++--------------------------
> > 1 file changed, 92 insertions(+), 66 deletions(-)
> >
> >diff --git a/blockdev.c b/blockdev.c
> >index b573e56..c32a9a9 100644
> >--- a/blockdev.c
> >+++ b/blockdev.c
> >@@ -2883,29 +2883,22 @@ out:
> > #define DEFAULT_MIRROR_BUF_SIZE (10 << 20)
> >-void qmp_drive_mirror(const char *device, const char *target,
> >- bool has_format, const char *format,
> >- bool has_node_name, const char *node_name,
> >- bool has_replaces, const char *replaces,
> >- enum MirrorSyncMode sync,
> >- bool has_mode, enum NewImageMode mode,
> >- bool has_speed, int64_t speed,
> >- bool has_granularity, uint32_t granularity,
> >- bool has_buf_size, int64_t buf_size,
> >- bool has_on_source_error, BlockdevOnError on_source_error,
> >- bool has_on_target_error, BlockdevOnError on_target_error,
> >- Error **errp)
> >+/* Parameter check and block job starting for mirroring.
> >+ * Caller should hold @device and @target's aio context (They must to be on the
> >+ * same aio context). */
> >+static void blockdev_mirror_common(BlockDriverState *bs,
> >+ BlockDriverState *target,
> >+ bool has_replaces, const char *replaces,
> >+ enum MirrorSyncMode sync,
> >+ bool has_speed, int64_t speed,
> >+ bool has_granularity, uint32_t granularity,
> >+ bool has_buf_size, int64_t buf_size,
> >+ bool has_on_source_error,
> >+ BlockdevOnError on_source_error,
> >+ bool has_on_target_error,
> >+ BlockdevOnError on_target_error,
> >+ Error **errp)
> > {
> >- BlockBackend *blk;
> >- BlockDriverState *bs;
> >- BlockDriverState *source, *target_bs;
> >- AioContext *aio_context;
> >- BlockDriver *drv = NULL;
> >- Error *local_err = NULL;
> >- QDict *options = NULL;
> >- int flags;
> >- int64_t size;
> >- int ret;
> > if (!has_speed) {
> > speed = 0;
> >@@ -2916,9 +2909,6 @@ void qmp_drive_mirror(const char *device, const char *target,
> > if (!has_on_target_error) {
> > on_target_error = BLOCKDEV_ON_ERROR_REPORT;
> > }
> >- if (!has_mode) {
> >- mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
> >- }
> > if (!has_granularity) {
> > granularity = 0;
> > }
> >@@ -2936,35 +2926,73 @@ void qmp_drive_mirror(const char *device, const char *target,
> > return;
> > }
> >- blk = blk_by_name(device);
> >- if (!blk) {
> >- error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> >- return;
> >- }
> >-
> >- aio_context = blk_get_aio_context(blk);
> >- aio_context_acquire(aio_context);
> >-
> >- if (!blk_is_available(blk)) {
> >- error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> >- goto out;
> >- }
> >- bs = blk_bs(blk);
> >-
> >- if (!has_format) {
> >- format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
> >- }
> >- if (format) {
> >- drv = bdrv_find_format(format);
> >- if (!drv) {
> >- error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
> >- goto out;
> >- }
> >- }
> >-
> > if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
> >+ return;
> >+ }
> >+
> >+ if (!bs->backing_hd && sync == MIRROR_SYNC_MODE_TOP) {
> >+ sync = MIRROR_SYNC_MODE_FULL;
> >+ }
> >+
> >+ /* pass the node name to replace to mirror start since it's loose coupling
> >+ * and will allow to check whether the node still exist at mirror completion
> >+ */
> >+ mirror_start(bs, target,
> >+ has_replaces ? replaces : NULL,
> >+ speed, granularity, buf_size, sync,
> >+ on_source_error, on_target_error,
> >+ block_job_cb, bs, errp);
> >+}
> >+
> >+void qmp_drive_mirror(const char *device, const char *target,
> >+ bool has_format, const char *format,
> >+ bool has_node_name, const char *node_name,
> >+ bool has_replaces, const char *replaces,
> >+ enum MirrorSyncMode sync,
> >+ bool has_mode, enum NewImageMode mode,
> >+ bool has_speed, int64_t speed,
> >+ bool has_granularity, uint32_t granularity,
> >+ bool has_buf_size, int64_t buf_size,
> >+ bool has_on_source_error, BlockdevOnError on_source_error,
> >+ bool has_on_target_error, BlockdevOnError on_target_error,
> >+ Error **errp)
> >+{
> >+ BlockDriverState *bs;
> >+ BlockBackend *blk;
> >+ BlockDriverState *source, *target_bs;
> >+ AioContext *aio_context;
> >+ BlockDriver *drv = NULL;
> >+ Error *local_err = NULL;
> >+ QDict *options = NULL;
> >+ int flags;
> >+ int64_t size;
> >+ int ret;
> >+
> >+ blk = blk_by_name(device);
> >+ if (!blk) {
> >+ error_set(errp, QERR_DEVICE_NOT_FOUND, device);
> >+ return;
> >+ }
> >+
> >+ aio_context = blk_get_aio_context(blk);
> >+ aio_context_acquire(aio_context);
> >+
> >+ if (!blk_is_available(blk)) {
> >+ error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> > goto out;
> > }
> >+ bs = blk_bs(blk);
> >+
> >+ if (!has_format) {
> >+ format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
> >+ }
> >+ if (format) {
> >+ drv = bdrv_find_format(format);
> >+ if (!drv) {
> >+ error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
> >+ goto out;
> >+ }
> >+ }
> > flags = bs->open_flags | BDRV_O_RDWR;
> > source = bs->backing_hd;
> >@@ -2989,14 +3017,14 @@ void qmp_drive_mirror(const char *device, const char *target,
> > if (!has_node_name) {
> > error_setg(errp, "a node-name must be provided when replacing a"
> > " named node of the graph");
> >- goto out;
> >+ return;
> > }
> > to_replace_bs = check_to_replace_node(replaces, &local_err);
> > if (!to_replace_bs) {
> > error_propagate(errp, local_err);
> >- goto out;
> >+ return;
> > }
> > replace_aio_context = bdrv_get_aio_context(to_replace_bs);
> >@@ -3007,7 +3035,7 @@ void qmp_drive_mirror(const char *device, const char *target,
> > if (size != replace_size) {
> > error_setg(errp, "cannot replace image with a mirror image of "
> > "different size");
> >- goto out;
> >+ return;
> > }
> > }
>
> There are still a couple of "return;" statements left in the "if
> (has_replaces)" block. I think they should be replaced, too.
You're right! Will fix.
Fam
>
> Max
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-06-25 2:33 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-09 2:13 [Qemu-devel] [PATCH 0/6] qmp: Add blockdev-mirror Fam Zheng
2015-06-09 2:13 ` [Qemu-devel] [PATCH 1/6] block: Add blocker on mirror target Fam Zheng
2015-06-24 16:14 ` Max Reitz
2015-06-25 2:27 ` Fam Zheng
2015-06-09 2:13 ` [Qemu-devel] [PATCH 2/6] block: Rename BLOCK_OP_TYPE_MIRROR to BLOCK_OP_TYPE_MIRROR_SOURCE Fam Zheng
2015-06-24 16:18 ` Max Reitz
2015-06-09 2:13 ` [Qemu-devel] [PATCH 3/6] block: Extract blockdev part of qmp_drive_mirror Fam Zheng
2015-06-24 16:34 ` Max Reitz
2015-06-25 2:33 ` Fam Zheng
2015-06-09 2:13 ` [Qemu-devel] [PATCH 4/6] block: Add check on mirror target Fam Zheng
2015-06-24 16:40 ` Max Reitz
2015-06-09 2:13 ` [Qemu-devel] [PATCH 5/6] qmp: Add blockdev-mirror command Fam Zheng
2015-06-24 17:05 ` Max Reitz
2015-06-09 2:13 ` [Qemu-devel] [PATCH 6/6] iotests: Add test cases for blockdev-mirror Fam Zheng
2015-06-24 17:10 ` 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.