[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 09/11] block: let mirror blockjob run in BDS AioC
From: |
Stefan Hajnoczi |
Subject: |
[Qemu-devel] [PATCH v2 09/11] block: let mirror blockjob run in BDS AioContext |
Date: |
Tue, 21 Oct 2014 12:03:58 +0100 |
The mirror block job must run in the BlockDriverState AioContext so that
it works with dataplane.
Acquire the AioContext in blockdev.c so starting the block job is safe.
Note that to_replace is treated separately from other BlockDriverStates
in that it does not need to be in the same AioContext. Explicitly
acquire/release to_replace's AioContext when accessing it.
The completion code in block/mirror.c must perform BDS graph
manipulation and bdrv_reopen() from the main loop. Use
block_job_defer_to_main_loop() to achieve that.
The bdrv_drain_all() call is not allowed outside the main loop since it
could lead to lock ordering problems. Use bdrv_drain(bs) instead
because we have acquired the AioContext so nothing else can sneak in
I/O.
Signed-off-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
---
block.c | 13 +++++++--
block/mirror.c | 85 ++++++++++++++++++++++++++++++++++++++++------------------
blockdev.c | 38 ++++++++++++++++++--------
3 files changed, 97 insertions(+), 39 deletions(-)
diff --git a/block.c b/block.c
index 76ed5cd..6f47bda 100644
--- a/block.c
+++ b/block.c
@@ -5929,13 +5929,19 @@ bool bdrv_is_first_non_filter(BlockDriverState
*candidate)
BlockDriverState *check_to_replace_node(const char *node_name, Error **errp)
{
BlockDriverState *to_replace_bs = bdrv_find_node(node_name);
+ AioContext *aio_context;
+
if (!to_replace_bs) {
error_setg(errp, "Node name '%s' not found", node_name);
return NULL;
}
+ aio_context = bdrv_get_aio_context(to_replace_bs);
+ aio_context_acquire(aio_context);
+
if (bdrv_op_is_blocked(to_replace_bs, BLOCK_OP_TYPE_REPLACE, errp)) {
- return NULL;
+ to_replace_bs = NULL;
+ goto out;
}
/* We don't want arbitrary node of the BDS chain to be replaced only the
top
@@ -5945,9 +5951,12 @@ BlockDriverState *check_to_replace_node(const char
*node_name, Error **errp)
*/
if (!bdrv_is_first_non_filter(to_replace_bs)) {
error_setg(errp, "Only top most non filter can be replaced");
- return NULL;
+ to_replace_bs = NULL;
+ goto out;
}
+out:
+ aio_context_release(aio_context);
return to_replace_bs;
}
diff --git a/block/mirror.c b/block/mirror.c
index 18b18e0..19b87d0 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -314,9 +314,56 @@ static void mirror_drain(MirrorBlockJob *s)
}
}
+typedef struct {
+ int ret;
+} MirrorExitData;
+
+static void mirror_exit(BlockJob *job, void *opaque)
+{
+ MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+ MirrorExitData *data = opaque;
+ AioContext *replace_aio_context = NULL;
+
+ if (s->to_replace) {
+ replace_aio_context = bdrv_get_aio_context(s->to_replace);
+ aio_context_acquire(replace_aio_context);
+ }
+
+ if (s->should_complete && data->ret == 0) {
+ BlockDriverState *to_replace = s->common.bs;
+ if (s->to_replace) {
+ to_replace = s->to_replace;
+ }
+ if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
+ bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
+ }
+ bdrv_swap(s->target, to_replace);
+ if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
+ /* drop the bs loop chain formed by the swap: break the loop then
+ * trigger the unref from the top one */
+ BlockDriverState *p = s->base->backing_hd;
+ bdrv_set_backing_hd(s->base, NULL);
+ bdrv_unref(p);
+ }
+ }
+ if (s->to_replace) {
+ bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
+ error_free(s->replace_blocker);
+ bdrv_unref(s->to_replace);
+ }
+ if (replace_aio_context) {
+ aio_context_release(replace_aio_context);
+ }
+ g_free(s->replaces);
+ bdrv_unref(s->target);
+ block_job_completed(&s->common, data->ret);
+ g_free(data);
+}
+
static void coroutine_fn mirror_run(void *opaque)
{
MirrorBlockJob *s = opaque;
+ MirrorExitData *data;
BlockDriverState *bs = s->common.bs;
int64_t sector_num, end, sectors_per_chunk, length;
uint64_t last_pause_ns;
@@ -467,7 +514,7 @@ static void coroutine_fn mirror_run(void *opaque)
* mirror_populate runs.
*/
trace_mirror_before_drain(s, cnt);
- bdrv_drain_all();
+ bdrv_drain(bs);
cnt = bdrv_get_dirty_count(bs, s->dirty_bitmap);
}
@@ -510,31 +557,10 @@ immediate_exit:
g_free(s->in_flight_bitmap);
bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);
bdrv_iostatus_disable(s->target);
- if (s->should_complete && ret == 0) {
- BlockDriverState *to_replace = s->common.bs;
- if (s->to_replace) {
- to_replace = s->to_replace;
- }
- if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
- bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
- }
- bdrv_swap(s->target, to_replace);
- if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
- /* drop the bs loop chain formed by the swap: break the loop then
- * trigger the unref from the top one */
- BlockDriverState *p = s->base->backing_hd;
- bdrv_set_backing_hd(s->base, NULL);
- bdrv_unref(p);
- }
- }
- if (s->to_replace) {
- bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
- error_free(s->replace_blocker);
- bdrv_unref(s->to_replace);
- }
- g_free(s->replaces);
- bdrv_unref(s->target);
- block_job_completed(&s->common, ret);
+
+ data = g_malloc(sizeof(*data));
+ data->ret = ret;
+ block_job_defer_to_main_loop(&s->common, mirror_exit, data);
}
static void mirror_set_speed(BlockJob *job, int64_t speed, Error **errp)
@@ -573,16 +599,23 @@ static void mirror_complete(BlockJob *job, Error **errp)
/* check the target bs is not blocked and block all operations on it */
if (s->replaces) {
+ AioContext *replace_aio_context;
+
s->to_replace = check_to_replace_node(s->replaces, &local_err);
if (!s->to_replace) {
error_propagate(errp, local_err);
return;
}
+ replace_aio_context = bdrv_get_aio_context(s->to_replace);
+ aio_context_acquire(replace_aio_context);
+
error_setg(&s->replace_blocker,
"block device is in use by block-job-complete");
bdrv_op_block_all(s->to_replace, s->replace_blocker);
bdrv_ref(s->to_replace);
+
+ aio_context_release(replace_aio_context);
}
s->should_complete = true;
diff --git a/blockdev.c b/blockdev.c
index 5d905ce..2bd6c58 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2269,6 +2269,7 @@ void qmp_drive_mirror(const char *device, const char
*target,
{
BlockDriverState *bs;
BlockDriverState *source, *target_bs;
+ AioContext *aio_context;
BlockDriver *drv = NULL;
Error *local_err = NULL;
QDict *options = NULL;
@@ -2311,9 +2312,12 @@ void qmp_drive_mirror(const char *device, const char
*target,
return;
}
+ aio_context = bdrv_get_aio_context(bs);
+ aio_context_acquire(aio_context);
+
if (!bdrv_is_inserted(bs)) {
error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
- return;
+ goto out;
}
if (!has_format) {
@@ -2323,12 +2327,12 @@ void qmp_drive_mirror(const char *device, const char
*target,
drv = bdrv_find_format(format);
if (!drv) {
error_set(errp, QERR_INVALID_BLOCK_FORMAT, format);
- return;
+ goto out;
}
}
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR, errp)) {
- return;
+ goto out;
}
flags = bs->open_flags | BDRV_O_RDWR;
@@ -2343,29 +2347,36 @@ void qmp_drive_mirror(const char *device, const char
*target,
size = bdrv_getlength(bs);
if (size < 0) {
error_setg_errno(errp, -size, "bdrv_getlength failed");
- return;
+ goto out;
}
if (has_replaces) {
BlockDriverState *to_replace_bs;
+ AioContext *replace_aio_context;
+ int64_t replace_size;
if (!has_node_name) {
error_setg(errp, "a node-name must be provided when replacing a"
" named node of the graph");
- return;
+ goto out;
}
to_replace_bs = check_to_replace_node(replaces, &local_err);
if (!to_replace_bs) {
error_propagate(errp, local_err);
- return;
+ goto out;
}
- if (size != bdrv_getlength(to_replace_bs)) {
+ replace_aio_context = bdrv_get_aio_context(to_replace_bs);
+ aio_context_acquire(replace_aio_context);
+ replace_size = bdrv_getlength(to_replace_bs);
+ aio_context_release(replace_aio_context);
+
+ if (size != replace_size) {
error_setg(errp, "cannot replace image with a mirror image of "
"different size");
- return;
+ goto out;
}
}
@@ -2394,7 +2405,7 @@ void qmp_drive_mirror(const char *device, const char
*target,
if (local_err) {
error_propagate(errp, local_err);
- return;
+ goto out;
}
if (has_node_name) {
@@ -2410,9 +2421,11 @@ void qmp_drive_mirror(const char *device, const char
*target,
flags | BDRV_O_NO_BACKING, drv, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
- return;
+ goto out;
}
+ 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
*/
@@ -2424,8 +2437,11 @@ void qmp_drive_mirror(const char *device, const char
*target,
if (local_err != NULL) {
bdrv_unref(target_bs);
error_propagate(errp, local_err);
- return;
+ goto out;
}
+
+out:
+ aio_context_release(aio_context);
}
/* Get the block job for a given device name and acquire its AioContext */
--
1.9.3
- [Qemu-devel] [PATCH v2 01/11] block: acquire AioContext in generic blockjob QMP commands, (continued)
- [Qemu-devel] [PATCH v2 01/11] block: acquire AioContext in generic blockjob QMP commands, Stefan Hajnoczi, 2014/10/21
- [Qemu-devel] [PATCH v2 02/11] blockdev: acquire AioContext in do_qmp_query_block_jobs_one(), Stefan Hajnoczi, 2014/10/21
- [Qemu-devel] [PATCH v2 03/11] blockdev: acquire AioContext in blockdev_mark_auto_del(), Stefan Hajnoczi, 2014/10/21
- [Qemu-devel] [PATCH v2 04/11] blockdev: add note that block_job_cb() must be thread-safe, Stefan Hajnoczi, 2014/10/21
- [Qemu-devel] [PATCH v2 05/11] blockjob: add block_job_defer_to_main_loop(), Stefan Hajnoczi, 2014/10/21
- [Qemu-devel] [PATCH v2 06/11] block: add bdrv_drain(), Stefan Hajnoczi, 2014/10/21
- [Qemu-devel] [PATCH v2 07/11] block: let backup blockjob run in BDS AioContext, Stefan Hajnoczi, 2014/10/21
- [Qemu-devel] [PATCH v2 09/11] block: let mirror blockjob run in BDS AioContext,
Stefan Hajnoczi <=
- [Qemu-devel] [PATCH v2 08/11] block: let stream blockjob run in BDS AioContext, Stefan Hajnoczi, 2014/10/21
- [Qemu-devel] [PATCH v2 10/11] block: let commit blockjob run in BDS AioContext, Stefan Hajnoczi, 2014/10/21
- [Qemu-devel] [PATCH v2 11/11] block: declare blockjobs and dataplane friends!, Stefan Hajnoczi, 2014/10/21
- Re: [Qemu-devel] [PATCH v2 00/11] block: allow blockjobs to coexist with dataplane, Stefan Hajnoczi, 2014/10/29