On Wed, 12/20 11:34, Kevin Wolf wrote: > 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 <[email protected]> > --- > 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 37f8639fe9..cfd39bea10 100644 > --- a/block.c > +++ b/block.c > @@ -2774,6 +2774,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, > @@ -2789,6 +2790,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); > @@ -2913,6 +2919,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) > { > @@ -2922,11 +2930,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; > @@ -2955,8 +2960,6 @@ cleanup: > } > g_free(bs_queue); > > - bdrv_drain_all_end(); > - > return ret; > } > > @@ -2966,12 +2969,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 >
Reviewed-by: Fam Zheng <[email protected]>
