All of lore.kernel.org
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: qemu-block@nongnu.org
Cc: qemu-devel@nongnu.org, Max Reitz <mreitz@redhat.com>,
	Fam Zheng <famz@redhat.com>, Kevin Wolf <kwolf@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	John Snow <jsnow@redhat.com>
Subject: [Qemu-devel] [PATCH 02/18] block: BDS deletion during bdrv_drain_recurse
Date: Wed, 13 Sep 2017 20:18:54 +0200	[thread overview]
Message-ID: <20170913181910.29688-3-mreitz@redhat.com> (raw)
In-Reply-To: <20170913181910.29688-1-mreitz@redhat.com>

Drainined a BDS child may lead to both the original BDS and/or its other
children being deleted (e.g. if the original BDS represents a block
job).  We should prepare for this in both bdrv_drain_recurse() and
bdrv_drained_begin() by monitoring whether the BDS we are about to drain
still exists at all.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/io.c | 72 +++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 20 deletions(-)

diff --git a/block/io.c b/block/io.c
index 4378ae4c7d..8ec1a564ad 100644
--- a/block/io.c
+++ b/block/io.c
@@ -182,33 +182,57 @@ static void bdrv_drain_invoke(BlockDriverState *bs)
 
 static bool bdrv_drain_recurse(BlockDriverState *bs)
 {
-    BdrvChild *child, *tmp;
+    BdrvChild *child;
     bool waited;
+    struct BDSToDrain {
+        BlockDriverState *bs;
+        BdrvDeletedStatus del_stat;
+        QLIST_ENTRY(BDSToDrain) next;
+    };
+    QLIST_HEAD(, BDSToDrain) bs_list = QLIST_HEAD_INITIALIZER(bs_list);
+    bool in_main_loop =
+        qemu_get_current_aio_context() == qemu_get_aio_context();
 
     waited = BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0);
 
     /* Ensure any pending metadata writes are submitted to bs->file.  */
     bdrv_drain_invoke(bs);
 
-    QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) {
-        BlockDriverState *bs = child->bs;
-        bool in_main_loop =
-            qemu_get_current_aio_context() == qemu_get_aio_context();
-        assert(bs->refcnt > 0);
-        if (in_main_loop) {
-            /* In case the recursive bdrv_drain_recurse processes a
-             * block_job_defer_to_main_loop BH and modifies the graph,
-             * let's hold a reference to bs until we are done.
-             *
-             * IOThread doesn't have such a BH, and it is not safe to call
-             * bdrv_unref without BQL, so skip doing it there.
-             */
-            bdrv_ref(bs);
-        }
-        waited |= bdrv_drain_recurse(bs);
-        if (in_main_loop) {
-            bdrv_unref(bs);
+    /* Draining children may result in other children being removed and maybe
+     * even deleted, so copy the children list first */
+    QLIST_FOREACH(child, &bs->children, next) {
+        struct BDSToDrain *bs2d = g_new0(struct BDSToDrain, 1);
+
+        bs2d->bs = child->bs;
+        QLIST_INSERT_HEAD(&bs->deleted_status, &bs2d->del_stat, next);
+
+        QLIST_INSERT_HEAD(&bs_list, bs2d, next);
+    }
+
+    while (!QLIST_EMPTY(&bs_list)) {
+        struct BDSToDrain *bs2d = QLIST_FIRST(&bs_list);
+        QLIST_REMOVE(bs2d, next);
+
+        if (!bs2d->del_stat.deleted) {
+            QLIST_REMOVE(&bs2d->del_stat, next);
+
+            if (in_main_loop) {
+                /* In case the recursive bdrv_drain_recurse processes a
+                 * block_job_defer_to_main_loop BH and modifies the graph,
+                 * let's hold a reference to the BDS until we are done.
+                 *
+                 * IOThread doesn't have such a BH, and it is not safe to call
+                 * bdrv_unref without BQL, so skip doing it there.
+                 */
+                bdrv_ref(bs2d->bs);
+            }
+            waited |= bdrv_drain_recurse(bs2d->bs);
+            if (in_main_loop) {
+                bdrv_unref(bs2d->bs);
+            }
         }
+
+        g_free(bs2d);
     }
 
     return waited;
@@ -252,17 +276,25 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
 
 void bdrv_drained_begin(BlockDriverState *bs)
 {
+    BdrvDeletedStatus del_stat = { .deleted = false };
+
     if (qemu_in_coroutine()) {
         bdrv_co_yield_to_drain(bs);
         return;
     }
 
+    QLIST_INSERT_HEAD(&bs->deleted_status, &del_stat, next);
+
     if (atomic_fetch_inc(&bs->quiesce_counter) == 0) {
         aio_disable_external(bdrv_get_aio_context(bs));
         bdrv_parent_drained_begin(bs);
     }
 
-    bdrv_drain_recurse(bs);
+    if (!del_stat.deleted) {
+        QLIST_REMOVE(&del_stat, next);
+
+        bdrv_drain_recurse(bs);
+    }
 }
 
 void bdrv_drained_end(BlockDriverState *bs)
-- 
2.13.5

  parent reply	other threads:[~2017-09-13 18:19 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-13 18:18 [Qemu-devel] [PATCH 00/18] block/mirror: Add active-sync mirroring Max Reitz
2017-09-13 18:18 ` [Qemu-devel] [PATCH 01/18] block: Add BdrvDeletedStatus Max Reitz
2017-09-13 18:18 ` Max Reitz [this message]
2017-09-18  3:44   ` [Qemu-devel] [PATCH 02/18] block: BDS deletion during bdrv_drain_recurse Fam Zheng
2017-09-18 16:13     ` Max Reitz
2017-10-09 18:30       ` Max Reitz
2017-10-10  8:36   ` Kevin Wolf
2017-10-11 11:41     ` Max Reitz
2017-09-13 18:18 ` [Qemu-devel] [PATCH 03/18] blockjob: Make drained_{begin, end} public Max Reitz
2017-09-18  3:46   ` Fam Zheng
2017-09-13 18:18 ` [Qemu-devel] [PATCH 04/18] block/mirror: Pull out mirror_perform() Max Reitz
2017-09-18  3:48   ` Fam Zheng
2017-09-25  9:38   ` Vladimir Sementsov-Ogievskiy
2017-09-13 18:18 ` [Qemu-devel] [PATCH 05/18] block/mirror: Convert to coroutines Max Reitz
2017-09-18  6:02   ` Fam Zheng
2017-09-18 16:41     ` Max Reitz
2017-10-10  9:14   ` Kevin Wolf
2017-10-11 11:43     ` Max Reitz
2017-09-13 18:18 ` [Qemu-devel] [PATCH 06/18] block/mirror: Use CoQueue to wait on in-flight ops Max Reitz
2017-09-13 18:18 ` [Qemu-devel] [PATCH 07/18] block/mirror: Wait for in-flight op conflicts Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 08/18] block/mirror: Use source as a BdrvChild Max Reitz
2017-10-10  9:27   ` Kevin Wolf
2017-10-11 11:46     ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 09/18] block: Generalize should_update_child() rule Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 10/18] block/mirror: Make source the file child Max Reitz
2017-10-10  9:47   ` Kevin Wolf
2017-10-11 12:02     ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 11/18] hbitmap: Add @advance param to hbitmap_iter_next() Max Reitz
2017-09-25 15:38   ` Vladimir Sementsov-Ogievskiy
2017-09-25 20:40     ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 12/18] block/dirty-bitmap: Add bdrv_dirty_iter_next_area Max Reitz
2017-09-25 15:49   ` Vladimir Sementsov-Ogievskiy
2017-09-25 20:43     ` Max Reitz
2017-10-02 13:32     ` Vladimir Sementsov-Ogievskiy
2017-09-13 18:19 ` [Qemu-devel] [PATCH 13/18] block/mirror: Keep write perm for pending writes Max Reitz
2017-10-10  9:58   ` Kevin Wolf
2017-10-11 12:20     ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 14/18] block/mirror: Distinguish active from passive ops Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 15/18] block/mirror: Add active mirroring Max Reitz
2017-09-14 15:57   ` Stefan Hajnoczi
2017-09-16 13:58     ` Max Reitz
2017-09-18 10:06       ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-09-18 16:26         ` Max Reitz
2017-09-19  9:44           ` Stefan Hajnoczi
2017-09-19  9:57             ` Daniel P. Berrange
2017-09-20 14:56               ` Stefan Hajnoczi
2017-10-10 10:16           ` Kevin Wolf
2017-10-11 12:33             ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 16/18] block/mirror: Add copy mode QAPI interface Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 17/18] qemu-io: Add background write Max Reitz
2017-09-18  6:46   ` Fam Zheng
2017-09-18 17:53     ` Max Reitz
2017-09-19  8:03       ` Fam Zheng
2017-09-21 14:40         ` Max Reitz
2017-09-21 14:59           ` Fam Zheng
2017-09-21 15:03             ` Max Reitz
2017-09-13 18:19 ` [Qemu-devel] [PATCH 18/18] iotests: Add test for active mirroring Max Reitz
2017-09-18  6:45   ` Fam Zheng
2017-09-18 16:53     ` Max Reitz
2017-09-19  8:08       ` Fam Zheng
2017-09-14 15:42 ` [Qemu-devel] [PATCH 00/18] block/mirror: Add active-sync mirroring Stefan Hajnoczi
2017-09-16 14:02   ` Max Reitz
2017-09-18 10:02     ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-09-18 15:42       ` Max Reitz

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=20170913181910.29688-3-mreitz@redhat.com \
    --to=mreitz@redhat.com \
    --cc=famz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /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.