[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL v3 35/35] block: Keep nodes drained between reopen_qu
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] [PULL v3 35/35] block: Keep nodes drained between reopen_queue/multiple |
Date: |
Fri, 22 Dec 2017 16:18:46 +0100 |
The bdrv_reopen*() implementation doesn't like it if the graph is
changed between queuing nodes for reopen and actually reopening them
(one of the reasons is that queuing can be recursive).
So instead of draining the device only in bdrv_reopen_multiple(),
require that callers already drained all affected nodes, and assert this
in bdrv_reopen_queue().
Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Fam Zheng <address@hidden>
---
block.c | 23 ++++++++++++++++-------
block/replication.c | 6 ++++++
qemu-io-cmds.c | 3 +++
3 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/block.c b/block.c
index 8b46ba21b1..a8da4f2b25 100644
--- a/block.c
+++ b/block.c
@@ -2766,6 +2766,7 @@ BlockDriverState *bdrv_open(const char *filename, const
char *reference,
* returns a pointer to bs_queue, which is either the newly allocated
* bs_queue, or the existing bs_queue being used.
*
+ * bs must be drained between bdrv_reopen_queue() and bdrv_reopen_multiple().
*/
static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
BlockDriverState *bs,
@@ -2781,6 +2782,11 @@ static BlockReopenQueue
*bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
BdrvChild *child;
QDict *old_options, *explicit_options;
+ /* Make sure that the caller remembered to use a drained section. This is
+ * important to avoid graph changes between the recursive queuing here and
+ * bdrv_reopen_multiple(). */
+ assert(bs->quiesce_counter > 0);
+
if (bs_queue == NULL) {
bs_queue = g_new0(BlockReopenQueue, 1);
QSIMPLEQ_INIT(bs_queue);
@@ -2905,6 +2911,8 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue
*bs_queue,
* If all devices prepare successfully, then the changes are committed
* to all devices.
*
+ * All affected nodes must be drained between bdrv_reopen_queue() and
+ * bdrv_reopen_multiple().
*/
int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error
**errp)
{
@@ -2914,11 +2922,8 @@ int bdrv_reopen_multiple(AioContext *ctx,
BlockReopenQueue *bs_queue, Error **er
assert(bs_queue != NULL);
- aio_context_release(ctx);
- bdrv_drain_all_begin();
- aio_context_acquire(ctx);
-
QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
+ assert(bs_entry->state.bs->quiesce_counter > 0);
if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, &local_err)) {
error_propagate(errp, local_err);
goto cleanup;
@@ -2947,8 +2952,6 @@ cleanup:
}
g_free(bs_queue);
- bdrv_drain_all_end();
-
return ret;
}
@@ -2958,12 +2961,18 @@ int bdrv_reopen(BlockDriverState *bs, int bdrv_flags,
Error **errp)
{
int ret = -1;
Error *local_err = NULL;
- BlockReopenQueue *queue = bdrv_reopen_queue(NULL, bs, NULL, bdrv_flags);
+ BlockReopenQueue *queue;
+ bdrv_subtree_drained_begin(bs);
+
+ queue = bdrv_reopen_queue(NULL, bs, NULL, bdrv_flags);
ret = bdrv_reopen_multiple(bdrv_get_aio_context(bs), queue, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
}
+
+ bdrv_subtree_drained_end(bs);
+
return ret;
}
diff --git a/block/replication.c b/block/replication.c
index e41e293d2b..b1ea3caa4b 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -394,6 +394,9 @@ static void reopen_backing_file(BlockDriverState *bs, bool
writable,
new_secondary_flags = s->orig_secondary_flags;
}
+ bdrv_subtree_drained_begin(s->hidden_disk->bs);
+ bdrv_subtree_drained_begin(s->secondary_disk->bs);
+
if (orig_hidden_flags != new_hidden_flags) {
reopen_queue = bdrv_reopen_queue(reopen_queue, s->hidden_disk->bs,
NULL,
new_hidden_flags);
@@ -409,6 +412,9 @@ static void reopen_backing_file(BlockDriverState *bs, bool
writable,
reopen_queue, &local_err);
error_propagate(errp, local_err);
}
+
+ bdrv_subtree_drained_end(s->hidden_disk->bs);
+ bdrv_subtree_drained_end(s->secondary_disk->bs);
}
static void backup_job_cleanup(BlockDriverState *bs)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index de8e3de726..a6a70fc3dc 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2013,8 +2013,11 @@ static int reopen_f(BlockBackend *blk, int argc, char
**argv)
opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
qemu_opts_reset(&reopen_opts);
+ bdrv_subtree_drained_begin(bs);
brq = bdrv_reopen_queue(NULL, bs, opts, flags);
bdrv_reopen_multiple(bdrv_get_aio_context(bs), brq, &local_err);
+ bdrv_subtree_drained_end(bs);
+
if (local_err) {
error_report_err(local_err);
} else {
--
2.13.6
- [Qemu-devel] [PULL v3 21/35] test-bdrv-drain: Test bs->quiesce_counter, (continued)
- [Qemu-devel] [PULL v3 21/35] test-bdrv-drain: Test bs->quiesce_counter, Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 20/35] test-bdrv-drain: Test callback for bdrv_drain, Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 18/35] block: Assert drain_all is only called from main AioContext, Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 26/35] test-bdrv-drain: Test nested drain sections, Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 27/35] block: Don't notify parents in drain call chain, Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 28/35] block: Add bdrv_subtree_drained_begin/end(), Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 30/35] test-bdrv-drain: Test behaviour in coroutine context, Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 29/35] test-bdrv-drain: Tests for bdrv_subtree_drain, Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 31/35] test-bdrv-drain: Recursive draining with multiple parents, Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 33/35] test-bdrv-drain: Test graph changes in drained section, Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 35/35] block: Keep nodes drained between reopen_queue/multiple,
Kevin Wolf <=
- [Qemu-devel] [PULL v3 32/35] block: Allow graph changes in subtree drained section, Kevin Wolf, 2017/12/22
- [Qemu-devel] [PULL v3 34/35] commit: Simplify reopen of base, Kevin Wolf, 2017/12/22
- Re: [Qemu-devel] [PULL v3 00/35] Block layer patches, no-reply, 2017/12/22