From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34136) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1GLi-0006yv-2c for qemu-devel@nongnu.org; Mon, 31 Oct 2016 13:26:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1GLh-0008R6-6X for qemu-devel@nongnu.org; Mon, 31 Oct 2016 13:26:18 -0400 From: Kevin Wolf Date: Mon, 31 Oct 2016 18:25:36 +0100 Message-Id: <1477934758-29332-8-git-send-email-kwolf@redhat.com> In-Reply-To: <1477934758-29332-1-git-send-email-kwolf@redhat.com> References: <1477934758-29332-1-git-send-email-kwolf@redhat.com> Subject: [Qemu-devel] [PULL 07/29] block: Pause all jobs during bdrv_reopen_multiple() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-block@nongnu.org Cc: kwolf@redhat.com, qemu-devel@nongnu.org From: Alberto Garcia When a BlockDriverState is about to be reopened it can trigger certain operations that need to write to disk. During this process a different block job can be woken up. If that block job completes and also needs to call bdrv_reopen() it can happen that it needs to do it on the same BlockDriverState that is still in the process of being reopened. This can have fatal consequences, like in this example: 1) Block job A starts and sleeps after a while. 2) Block job B starts and tries to reopen node1 (a qcow2 file). 3) Reopening node1 means flushing and replacing its qcow2 cache. 4) While the qcow2 cache is being flushed, job A wakes up. 5) Job A completes and reopens node1, replacing its cache. 6) Job B resumes, but the cache that was being flushed no longer exists. This patch splits the bdrv_drain_all() call to keep all block jobs paused during bdrv_reopen_multiple(), so that step 4 can never happen and the operation is safe. Note that this scenario can only happen if both bdrv_reopen() calls are made by block jobs on the same backing chain. Otherwise there's no chance that the same BlockDriverState appears in both reopen queues. Signed-off-by: Alberto Garcia Reviewed-by: Paolo Bonzini Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index a17baab..2c87186 100644 --- a/block.c +++ b/block.c @@ -2091,7 +2091,7 @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er assert(bs_queue != NULL); aio_context_release(ctx); - bdrv_drain_all(); + bdrv_drain_all_begin(); aio_context_acquire(ctx); QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) { @@ -2122,6 +2122,9 @@ cleanup: g_free(bs_entry); } g_free(bs_queue); + + bdrv_drain_all_end(); + return ret; } -- 1.8.3.1