[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 2/2] block: Make op blockers recursive
|
From: |
Benoît Canet |
|
Subject: |
[Qemu-devel] [PATCH v2 2/2] block: Make op blockers recursive |
|
Date: |
Mon, 22 Sep 2014 14:40:39 +0200 |
Since the block layer code is starting to modify the BDS graph right in the
middle of BDS chains (block-mirror's replace parameter for example) QEMU needs
to properly block and unblock whole BDS subtrees; recursion is a neat way to
achieve this task.
An optional base arguments was added to the API to optionally allow blocking
only a part of a BDS chain.
This patch also takes care of modifying the op blockers users.
Signed-off-by: Benoit Canet <address@hidden>
---
block-migration.c | 4 +-
block.c | 121 ++++++++++++++++++++++++++++++++++++++++++----
block/blkverify.c | 21 ++++++++
block/commit.c | 2 +
block/mirror.c | 26 +++++++---
block/quorum.c | 29 +++++++++++
block/stream.c | 2 +
block/vmdk.c | 32 ++++++++++++
blockjob.c | 6 +--
include/block/block.h | 12 +++--
include/block/block_int.h | 10 ++++
11 files changed, 240 insertions(+), 25 deletions(-)
diff --git a/block-migration.c b/block-migration.c
index 3ad31a2..ad7aa03 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -362,7 +362,7 @@ static void init_blk_migration_it(void *opaque,
BlockDriverState *bs)
bmds->shared_base = block_mig_state.shared_base;
alloc_aio_bitmap(bmds);
error_setg(&bmds->blocker, "block device is in use by migration");
- bdrv_op_block_all(bs, bmds->blocker);
+ bdrv_op_block_all(bs, NULL, bmds->blocker);
bdrv_ref(bs);
block_mig_state.total_sector_sum += sectors;
@@ -600,7 +600,7 @@ static void blk_mig_cleanup(void)
blk_mig_lock();
while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list)) != NULL) {
QSIMPLEQ_REMOVE_HEAD(&block_mig_state.bmds_list, entry);
- bdrv_op_unblock_all(bmds->bs, bmds->blocker);
+ bdrv_op_unblock_all(bmds->bs, NULL, bmds->blocker);
error_free(bmds->blocker);
bdrv_unref(bmds->bs);
g_free(bmds->aio_bitmap);
diff --git a/block.c b/block.c
index 9a9f8a0..b2d6953 100644
--- a/block.c
+++ b/block.c
@@ -1148,7 +1148,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs,
BlockDriverState *backing_hd)
if (bs->backing_hd) {
assert(bs->backing_blocker);
- bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
+ bdrv_op_unblock_all(bs->backing_hd, NULL, bs->backing_blocker);
} else if (backing_hd) {
error_setg(&bs->backing_blocker,
"device is used as backing hd of '%s'",
@@ -1166,9 +1166,9 @@ void bdrv_set_backing_hd(BlockDriverState *bs,
BlockDriverState *backing_hd)
pstrcpy(bs->backing_format, sizeof(bs->backing_format),
backing_hd->drv ? backing_hd->drv->format_name : "");
- bdrv_op_block_all(bs->backing_hd, bs->backing_blocker);
+ bdrv_op_block_all(bs->backing_hd, NULL, bs->backing_blocker);
/* Otherwise we won't be able to commit due to check in bdrv_commit */
- bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT,
+ bdrv_op_unblock(bs->backing_hd, NULL, BLOCK_OP_TYPE_COMMIT,
bs->backing_blocker);
out:
bdrv_refresh_limits(bs, NULL);
@@ -5482,7 +5482,9 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType
op, Error **errp)
return false;
}
-void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason)
+/* do the real work of blocking a BDS */
+static void bdrv_do_op_block(BlockDriverState *bs, BlockOpType op,
+ Error *reason)
{
BdrvOpBlocker *blocker;
assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
@@ -5492,7 +5494,9 @@ void bdrv_op_block(BlockDriverState *bs, BlockOpType op,
Error *reason)
QLIST_INSERT_HEAD(&bs->op_blockers[op], blocker, list);
}
-void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason)
+/* do the real work of unblocking a BDS */
+static void bdrv_do_op_unblock(BlockDriverState *bs, BlockOpType op,
+ Error *reason)
{
BdrvOpBlocker *blocker, *next;
assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
@@ -5504,19 +5508,118 @@ void bdrv_op_unblock(BlockDriverState *bs, BlockOpType
op, Error *reason)
}
}
-void bdrv_op_block_all(BlockDriverState *bs, Error *reason)
+static bool bdrv_op_is_blocked_by(BlockDriverState *bs, BlockOpType op,
+ Error *reason)
+{
+ BdrvOpBlocker *blocker;
+ assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
+ QLIST_FOREACH(blocker, &bs->op_blockers[op], list) {
+ if (blocker->reason == reason) {
+ return true;
+ }
+ }
+ return false;
+}
+
+/* block recursively a BDS
+ *
+ * If base != NULL the caller must make sure that there is no multiple child
+ * BDS in the subtree pointed to by bs
+ *
+ * @bs: the BDS to start to recurse from
+ * @base: the BDS where the recursion should end
+ * could be NULL if the recursion should go down everywhere
+ * @op: the type of op blocker to block
+ * @reason: the error message associated with the blocking
+ */
+void bdrv_op_block(BlockDriverState *bs, BlockDriverState *base,
+ BlockOpType op, Error *reason)
+{
+ if (!bs) {
+ return;
+ }
+
+ /* did we recurse down to base ? */
+ if (bs == base) {
+ return;
+ }
+
+ /* prevent recursion loop */
+ if (bdrv_op_is_blocked_by(bs, op, reason)) {
+ return;
+ }
+
+ /* block first for recursion loop protection to work */
+ bdrv_do_op_block(bs, op, reason);
+
+ bdrv_op_block(bs->file, base, op, reason);
+
+ if (bs->drv && bs->drv->supports_backing) {
+ bdrv_op_block(bs->backing_hd, base, op, reason);
+ }
+
+ if (bs->drv && bs->drv->bdrv_op_recursive_block) {
+ bs->drv->bdrv_op_recursive_block(bs, base, op, reason);
+ }
+}
+
+/* unblock recursively a BDS
+ *
+ * If base != NULL the caller must make sure that there is no multiple child
+ * BDS in the subtree pointed to by bs
+ *
+ * @bs: the BDS to start to recurse from
+ * @base: the BDS where the recursion should end
+ * could be NULL if the recursion should go down everywhere
+ * @op: the type of op blocker to block
+ * @reason: the error message associated with the blocking
+ */
+void bdrv_op_unblock(BlockDriverState *bs, BlockDriverState *base,
+ BlockOpType op, Error *reason)
+{
+ if (!bs) {
+ return;
+ }
+
+ /* did we recurse down to base ? */
+ if (bs == base) {
+ return;
+ }
+
+ /* prevent recursion loop */
+ if (!bdrv_op_is_blocked_by(bs, op, reason)) {
+ return;
+ }
+
+ /* block first for recursion loop protection to work */
+ bdrv_do_op_unblock(bs, op, reason);
+
+ bdrv_op_unblock(bs->file, base, op, reason);
+
+ if (bs->drv && bs->drv->supports_backing) {
+ bdrv_op_unblock(bs->backing_hd, base, op, reason);
+ }
+
+ if (bs->drv && bs->drv->bdrv_op_recursive_block) {
+ bs->drv->bdrv_op_recursive_unblock(bs, base, op, reason);
+ }
+}
+
+void bdrv_op_block_all(BlockDriverState *bs, BlockDriverState *base,
+ Error *reason)
{
int i;
for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
- bdrv_op_block(bs, i, reason);
+ bdrv_op_block(bs, base, i, reason);
}
}
-void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason)
+void bdrv_op_unblock_all(BlockDriverState *bs, BlockDriverState *base,
+ Error *reason)
{
int i;
for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
- bdrv_op_unblock(bs, i, reason);
+ bdrv_op_unblock(bs, base, i, reason);
}
}
diff --git a/block/blkverify.c b/block/blkverify.c
index 163064c..5a4fa7c 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -349,6 +349,24 @@ static void blkverify_refresh_filename(BlockDriverState
*bs)
}
}
+static void blkverify_op_recursive_block(BlockDriverState *bs,
+ BlockDriverState *base,
+ BlockOpType op,
+ Error *reason)
+{
+ BDRVBlkverifyState *s = bs->opaque;
+ bdrv_op_block(s->test_file, base, op, reason);
+}
+
+static void blkverify_op_recursive_unblock(BlockDriverState *bs,
+ BlockDriverState *base,
+ BlockOpType op,
+ Error *reason)
+{
+ BDRVBlkverifyState *s = bs->opaque;
+ bdrv_op_unblock(s->test_file, base, op, reason);
+}
+
static BlockDriver bdrv_blkverify = {
.format_name = "blkverify",
.protocol_name = "blkverify",
@@ -367,6 +385,9 @@ static BlockDriver bdrv_blkverify = {
.bdrv_attach_aio_context = blkverify_attach_aio_context,
.bdrv_detach_aio_context = blkverify_detach_aio_context,
+ .bdrv_op_recursive_block = blkverify_op_recursive_block,
+ .bdrv_op_recursive_unblock = blkverify_op_recursive_unblock,
+
.is_filter = true,
.bdrv_recurse_is_first_non_filter = blkverify_recurse_is_first_non_filter,
};
diff --git a/block/commit.c b/block/commit.c
index 91517d3..6a514d4 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -142,6 +142,8 @@ wait:
if (!block_job_is_cancelled(&s->common) && sector_num == end) {
/* success */
+ /* unblock only BDS to be dropped */
+ bdrv_op_unblock_all(top, base, s->common.blocker);
ret = bdrv_drop_intermediate(active, top, base, s->backing_file_str);
}
diff --git a/block/mirror.c b/block/mirror.c
index 18b18e0..6f6071a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -322,6 +322,7 @@ static void coroutine_fn mirror_run(void *opaque)
uint64_t last_pause_ns;
BlockDriverInfo bdi;
char backing_filename[1024];
+ BlockDriverState *to_replace;
int ret = 0;
int n;
@@ -510,14 +511,16 @@ immediate_exit:
g_free(s->in_flight_bitmap);
bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);
bdrv_iostatus_disable(s->target);
+ to_replace = s->common.bs;
+ if (s->to_replace) {
+ bdrv_op_unblock_all(s->to_replace, NULL, s->replace_blocker);
+ to_replace = s->to_replace;
+ }
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_op_unblock_all(to_replace, NULL, bs->job->blocker);
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
@@ -528,7 +531,6 @@ immediate_exit:
}
}
if (s->to_replace) {
- bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
error_free(s->replace_blocker);
bdrv_unref(s->to_replace);
}
@@ -581,7 +583,7 @@ static void mirror_complete(BlockJob *job, Error **errp)
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_op_block_all(s->to_replace, NULL, s->replace_blocker);
bdrv_ref(s->to_replace);
}
@@ -640,12 +642,22 @@ static void mirror_start_job(BlockDriverState *bs,
BlockDriverState *target,
return;
}
-
s = block_job_create(driver, bs, speed, cb, opaque, errp);
if (!s) {
return;
}
+ /* block_job_create block all block job operations.
+ * If replaces is specified the block-job-complete code will have to block
+ * BLOCK_OP_TYPE_MIRROR_REPLACE during the time the mirror complete.
+ * Conditionally unblock BLOCK_OP_TYPE_MIRROR_REPLACE so the
+ * block-job-complete can do its work.
+ */
+ if (replaces) {
+ bdrv_op_unblock(bs, NULL, BLOCK_OP_TYPE_MIRROR_REPLACE,
+ bs->job->blocker);
+ }
+
s->replaces = g_strdup(replaces);
s->on_source_error = on_source_error;
s->on_target_error = on_target_error;
diff --git a/block/quorum.c b/block/quorum.c
index 093382e..7b06911 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1065,6 +1065,32 @@ static void quorum_refresh_filename(BlockDriverState *bs)
bs->full_open_options = opts;
}
+static void quorum_op_recursive_block(BlockDriverState *bs,
+ BlockDriverState *base,
+ BlockOpType op,
+ Error *reason)
+{
+ BDRVQuorumState *s = bs->opaque;
+ int i;
+
+ for (i = 0; i < s->num_children; i++) {
+ bdrv_op_block(s->bs[i], base, op, reason);
+ }
+}
+
+static void quorum_op_recursive_unblock(BlockDriverState *bs,
+ BlockDriverState *base,
+ BlockOpType op,
+ Error *reason)
+{
+ BDRVQuorumState *s = bs->opaque;
+ int i;
+
+ for (i = 0; i < s->num_children; i++) {
+ bdrv_op_unblock(s->bs[i], base, op, reason);
+ }
+}
+
static BlockDriver bdrv_quorum = {
.format_name = "quorum",
.protocol_name = "quorum",
@@ -1086,6 +1112,9 @@ static BlockDriver bdrv_quorum = {
.bdrv_detach_aio_context = quorum_detach_aio_context,
.bdrv_attach_aio_context = quorum_attach_aio_context,
+ .bdrv_op_recursive_block = quorum_op_recursive_block,
+ .bdrv_op_recursive_unblock = quorum_op_recursive_unblock,
+
.is_filter = true,
.bdrv_recurse_is_first_non_filter = quorum_recurse_is_first_non_filter,
};
diff --git a/block/stream.c b/block/stream.c
index cdea3e8..a9e69a5 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -192,6 +192,8 @@ wait:
}
}
ret = bdrv_change_backing_file(bs, base_id, base_fmt);
+ /* unblock only BDS to be dropped */
+ bdrv_op_unblock_all(bs->backing_hd, base, s->common.blocker);
close_unused_images(bs, base, base_id);
}
diff --git a/block/vmdk.c b/block/vmdk.c
index afdea1a..257f683 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2221,6 +2221,36 @@ static QemuOptsList vmdk_create_opts = {
}
};
+static void vmdk_op_recursive_block(BlockDriverState *bs,
+ BlockDriverState *base,
+ BlockOpType op,
+ Error *reason)
+{
+ BDRVVmdkState *s = bs->opaque;
+ int i;
+
+ for (i = 0; i < s->num_extents; i++) {
+ if (s->extents[i].file) {
+ bdrv_op_block(s->extents[i].file, base, op, reason);
+ }
+ }
+}
+
+static void vmdk_op_recursive_unblock(BlockDriverState *bs,
+ BlockDriverState *base,
+ BlockOpType op,
+ Error *reason)
+{
+ BDRVVmdkState *s = bs->opaque;
+ int i;
+
+ for (i = 0; i < s->num_extents; i++) {
+ if (s->extents[i].file) {
+ bdrv_op_unblock(s->extents[i].file, base, op, reason);
+ }
+ }
+}
+
static BlockDriver bdrv_vmdk = {
.format_name = "vmdk",
.instance_size = sizeof(BDRVVmdkState),
@@ -2243,6 +2273,8 @@ static BlockDriver bdrv_vmdk = {
.bdrv_get_info = vmdk_get_info,
.bdrv_detach_aio_context = vmdk_detach_aio_context,
.bdrv_attach_aio_context = vmdk_attach_aio_context,
+ .bdrv_op_recursive_block = vmdk_op_recursive_block,
+ .bdrv_op_recursive_unblock = vmdk_op_recursive_unblock,
.supports_backing = true,
.create_opts = &vmdk_create_opts,
diff --git a/blockjob.c b/blockjob.c
index 0689fdd..b7a98f6 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -49,7 +49,7 @@ void *block_job_create(const BlockJobDriver *driver,
BlockDriverState *bs,
job = g_malloc0(driver->instance_size);
error_setg(&job->blocker, "block device is in use by block job: %s",
BlockJobType_lookup[driver->job_type]);
- bdrv_op_block_all(bs, job->blocker);
+ bdrv_op_block_all(bs, NULL, job->blocker);
job->driver = driver;
job->bs = bs;
@@ -65,7 +65,7 @@ void *block_job_create(const BlockJobDriver *driver,
BlockDriverState *bs,
block_job_set_speed(job, speed, &local_err);
if (local_err) {
bs->job = NULL;
- bdrv_op_unblock_all(bs, job->blocker);
+ bdrv_op_unblock_all(bs, NULL, job->blocker);
error_free(job->blocker);
g_free(job);
error_propagate(errp, local_err);
@@ -82,7 +82,7 @@ void block_job_completed(BlockJob *job, int ret)
assert(bs->job == job);
job->cb(job->opaque, ret);
bs->job = NULL;
- bdrv_op_unblock_all(bs, job->blocker);
+ bdrv_op_unblock_all(bs, NULL, job->blocker);
error_free(job->blocker);
g_free(job);
}
diff --git a/include/block/block.h b/include/block/block.h
index 10952ed..6d21a0b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -480,10 +480,14 @@ void bdrv_ref(BlockDriverState *bs);
void bdrv_unref(BlockDriverState *bs);
bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
-void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason);
-void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason);
-void bdrv_op_block_all(BlockDriverState *bs, Error *reason);
-void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason);
+void bdrv_op_block(BlockDriverState *bs, BlockDriverState *base,
+ BlockOpType op, Error *reason);
+void bdrv_op_unblock(BlockDriverState *bs, BlockDriverState *base,
+ BlockOpType op, Error *reason);
+void bdrv_op_block_all(BlockDriverState *bs, BlockDriverState *base,
+ Error *reason);
+void bdrv_op_unblock_all(BlockDriverState *bs, BlockDriverState *base,
+ Error *reason);
bool bdrv_op_blocker_is_empty(BlockDriverState *bs);
typedef enum {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8d86a6c..fc93da6 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -270,6 +270,16 @@ struct BlockDriver {
void (*bdrv_io_unplug)(BlockDriverState *bs);
void (*bdrv_flush_io_queue)(BlockDriverState *bs);
+ /* helps blockers to propagate recursively */
+ void (*bdrv_op_recursive_block)(BlockDriverState *bs,
+ BlockDriverState *base,
+ BlockOpType op,
+ Error *reason);
+ void (*bdrv_op_recursive_unblock)(BlockDriverState *bs,
+ BlockDriverState *base,
+ BlockOpType op,
+ Error *reason);
+
QLIST_ENTRY(BlockDriver) list;
};
--
2.1.0