All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alberto Garcia <berto@igalia.com>
To: qemu-devel@nongnu.org
Cc: qemu-block@nongnu.org, Kevin Wolf <kwolf@redhat.com>,
	Eric Blake <eblake@redhat.com>,
	Markus Armbruster <armbru@redhat.com>,
	Max Reitz <mreitz@redhat.com>, Alberto Garcia <berto@igalia.com>
Subject: [Qemu-devel] [PATCH v13 02/19] block: Pause all jobs during bdrv_reopen_multiple()
Date: Fri, 28 Oct 2016 10:08:03 +0300	[thread overview]
Message-ID: <bc48b09229263f43651e04be0b7e0243815dfd9a.1477638050.git.berto@igalia.com> (raw)
In-Reply-To: <cover.1477638050.git.berto@igalia.com>
In-Reply-To: <cover.1477638050.git.berto@igalia.com>

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 <berto@igalia.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 7f3e7bc..adbecd0 100644
--- a/block.c
+++ b/block.c
@@ -2090,7 +2090,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
 
     assert(bs_queue != NULL);
 
-    bdrv_drain_all();
+    bdrv_drain_all_begin();
 
     QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
         if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, &local_err)) {
@@ -2120,6 +2120,9 @@ cleanup:
         g_free(bs_entry);
     }
     g_free(bs_queue);
+
+    bdrv_drain_all_end();
+
     return ret;
 }
 
-- 
2.9.3

  parent reply	other threads:[~2016-10-28  7:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-28  7:08 [Qemu-devel] [PATCH v13 00/19] Support streaming to an intermediate layer Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 01/19] block: Add bdrv_drain_all_{begin, end}() Alberto Garcia
2016-10-28  7:08 ` Alberto Garcia [this message]
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 03/19] block: Add block_job_add_bdrv() Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 04/19] block: Use block_job_add_bdrv() in mirror_start_job() Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 05/19] block: Use block_job_add_bdrv() in backup_start() Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 06/19] block: Check blockers in all nodes involved in a block-commit job Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 07/19] block: Block all nodes involved in the block-commit operation Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 08/19] block: Block all intermediate nodes in commit_active_start() Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 09/19] block: Support streaming to an intermediate layer Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 10/19] block: Add QMP support for " Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 11/19] docs: Document how to stream " Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 12/19] qemu-iotests: Test streaming " Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 13/19] qemu-iotests: Test block-stream operations in parallel Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 14/19] qemu-iotests: Test overlapping stream and commit operations Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 15/19] qemu-iotests: Test block-stream and block-commit in parallel Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 16/19] qemu-iotests: Add iotests.supports_quorum() Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 17/19] qemu-iotests: Test streaming to a Quorum child Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 18/19] block: Add 'base-node' parameter to the 'block-stream' command Alberto Garcia
2016-10-28  7:08 ` [Qemu-devel] [PATCH v13 19/19] qemu-iotests: Test the 'base-node' parameter of 'block-stream' Alberto Garcia
2016-10-28 10:50 ` [Qemu-devel] [PATCH v13 00/19] Support streaming to an intermediate layer Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bc48b09229263f43651e04be0b7e0243815dfd9a.1477638050.git.berto@igalia.com \
    --to=berto@igalia.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.