From a2aabf88958119ec7d3022287eff6bcc924c90a8 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Wed, 28 Apr 2021 18:17:57 +0300 Subject: [PATCH] block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare During reopen we may add backing bs from other aio context, which may lead to changing original context of top bs. We are going to move graph modification to prepare stage. So, it will be possible that bdrv_flush() in bdrv_reopen_prepare called on bs in non-original aio context, which we didn't aquire which leads to crash. To avoid this problem move bdrv_flush() to be a separate reopen stage before bdrv_reopen_prepare(). This doesn't seem correct to acquire only one aio context and not all contexts participating in reopen. But it's not obvious how to do it correctly, keeping in mind: 1. rules of bdrv_set_aio_context_ignore() that requires new_context lock not being held 2. possible deadlocks because of holding all (or several?) AioContext locks Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Kevin Wolf Message-Id: <20210428151804.439460-30-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf --- block.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/block.c b/block.c index bdfe59d94d..357ec1be2c 100644 --- a/block.c +++ b/block.c @@ -4274,6 +4274,14 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) assert(bs_queue != NULL); + QTAILQ_FOREACH(bs_entry, bs_queue, entry) { + ret = bdrv_flush(bs_entry->state.bs); + if (ret < 0) { + error_setg_errno(errp, -ret, "Error flushing drive"); + goto cleanup; + } + } + QTAILQ_FOREACH(bs_entry, bs_queue, entry) { assert(bs_entry->state.bs->quiesce_counter > 0); if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, errp)) { @@ -4669,12 +4677,6 @@ static int bdrv_reopen_prepare(BDRVReopenState *reopen_state, bdrv_reopen_perm(queue, reopen_state->bs, &reopen_state->perm, &reopen_state->shared_perm); - ret = bdrv_flush(reopen_state->bs); - if (ret) { - error_setg_errno(errp, -ret, "Error flushing drive"); - goto error; - } - if (drv->bdrv_reopen_prepare) { /* * If a driver-specific option is missing, it means that we