151 lines
5.3 KiB
Diff
151 lines
5.3 KiB
Diff
From b51c41bfbf422dd3e15b946faa1695571bcbe2df Mon Sep 17 00:00:00 2001
|
|
From: Kevin Wolf <kwolf@redhat.com>
|
|
Date: Wed, 10 Oct 2018 20:21:50 +0100
|
|
Subject: [PATCH 24/49] block: Poll after drain on attaching a node
|
|
|
|
RH-Author: Kevin Wolf <kwolf@redhat.com>
|
|
Message-id: <20181010202213.7372-12-kwolf@redhat.com>
|
|
Patchwork-id: 82601
|
|
O-Subject: [RHEL-8 qemu-kvm PATCH 21/44] block: Poll after drain on attaching a node
|
|
Bugzilla: 1637976
|
|
RH-Acked-by: Max Reitz <mreitz@redhat.com>
|
|
RH-Acked-by: John Snow <jsnow@redhat.com>
|
|
RH-Acked-by: Thomas Huth <thuth@redhat.com>
|
|
|
|
Commit dcf94a23b1 ('block: Don't poll in parent drain callbacks')
|
|
removed polling in bdrv_child_cb_drained_begin() on the grounds that the
|
|
original bdrv_drain() already will poll and BdrvChildRole.drained_begin
|
|
calls must not cause graph changes (and therefore must not call
|
|
aio_poll() or the recursion through the graph will break.
|
|
|
|
This reasoning is correct for calls through bdrv_do_drained_begin().
|
|
However, BdrvChildRole.drained_begin is also called when a node that is
|
|
already in a drained section (i.e. bdrv_do_drained_begin() has already
|
|
returned and therefore can't poll any more) is attached to a new parent.
|
|
In this case, we must explicitly poll to have all requests completed
|
|
before the drained new child can be attached to the parent.
|
|
|
|
In bdrv_replace_child_noperm(), we know that we're not inside the
|
|
recursion of bdrv_do_drained_begin() because graph changes are not
|
|
allowed there, and bdrv_replace_child_noperm() is a graph change. The
|
|
call of BdrvChildRole.drained_begin() must therefore be followed by a
|
|
BDRV_POLL_WHILE() that waits for the completion of requests.
|
|
|
|
Reported-by: Max Reitz <mreitz@redhat.com>
|
|
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
(cherry picked from commit 4be6a6d118123340f16fb8b3bf45220d0f8ed6d4)
|
|
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
|
|
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
|
|
---
|
|
block.c | 2 +-
|
|
block/io.c | 26 ++++++++++++++++++++------
|
|
include/block/block.h | 8 ++++++++
|
|
include/block/block_int.h | 3 +++
|
|
4 files changed, 32 insertions(+), 7 deletions(-)
|
|
|
|
diff --git a/block.c b/block.c
|
|
index eea9c6f..e89b5e3 100644
|
|
--- a/block.c
|
|
+++ b/block.c
|
|
@@ -2072,7 +2072,7 @@ static void bdrv_replace_child_noperm(BdrvChild *child,
|
|
}
|
|
assert(num >= 0);
|
|
for (i = 0; i < num; i++) {
|
|
- child->role->drained_begin(child);
|
|
+ bdrv_parent_drained_begin_single(child, true);
|
|
}
|
|
}
|
|
|
|
diff --git a/block/io.c b/block/io.c
|
|
index 38ae299..d404088 100644
|
|
--- a/block/io.c
|
|
+++ b/block/io.c
|
|
@@ -52,9 +52,7 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore,
|
|
if (c == ignore || (ignore_bds_parents && c->role->parent_is_bds)) {
|
|
continue;
|
|
}
|
|
- if (c->role->drained_begin) {
|
|
- c->role->drained_begin(c);
|
|
- }
|
|
+ bdrv_parent_drained_begin_single(c, false);
|
|
}
|
|
}
|
|
|
|
@@ -73,6 +71,14 @@ void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore,
|
|
}
|
|
}
|
|
|
|
+static bool bdrv_parent_drained_poll_single(BdrvChild *c)
|
|
+{
|
|
+ if (c->role->drained_poll) {
|
|
+ return c->role->drained_poll(c);
|
|
+ }
|
|
+ return false;
|
|
+}
|
|
+
|
|
static bool bdrv_parent_drained_poll(BlockDriverState *bs, BdrvChild *ignore,
|
|
bool ignore_bds_parents)
|
|
{
|
|
@@ -83,14 +89,22 @@ static bool bdrv_parent_drained_poll(BlockDriverState *bs, BdrvChild *ignore,
|
|
if (c == ignore || (ignore_bds_parents && c->role->parent_is_bds)) {
|
|
continue;
|
|
}
|
|
- if (c->role->drained_poll) {
|
|
- busy |= c->role->drained_poll(c);
|
|
- }
|
|
+ busy |= bdrv_parent_drained_poll_single(c);
|
|
}
|
|
|
|
return busy;
|
|
}
|
|
|
|
+void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll)
|
|
+{
|
|
+ if (c->role->drained_begin) {
|
|
+ c->role->drained_begin(c);
|
|
+ }
|
|
+ if (poll) {
|
|
+ BDRV_POLL_WHILE(c->bs, bdrv_parent_drained_poll_single(c));
|
|
+ }
|
|
+}
|
|
+
|
|
static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src)
|
|
{
|
|
dst->opt_transfer = MAX(dst->opt_transfer, src->opt_transfer);
|
|
diff --git a/include/block/block.h b/include/block/block.h
|
|
index f9079ac..356712c 100644
|
|
--- a/include/block/block.h
|
|
+++ b/include/block/block.h
|
|
@@ -590,6 +590,14 @@ void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore,
|
|
bool ignore_bds_parents);
|
|
|
|
/**
|
|
+ * bdrv_parent_drained_begin_single:
|
|
+ *
|
|
+ * Begin a quiesced section for the parent of @c. If @poll is true, wait for
|
|
+ * any pending activity to cease.
|
|
+ */
|
|
+void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll);
|
|
+
|
|
+/**
|
|
* bdrv_parent_drained_end:
|
|
*
|
|
* End a quiesced section of all users of @bs. This is part of
|
|
diff --git a/include/block/block_int.h b/include/block/block_int.h
|
|
index 9757d5e..b7806e3 100644
|
|
--- a/include/block/block_int.h
|
|
+++ b/include/block/block_int.h
|
|
@@ -610,6 +610,9 @@ struct BdrvChildRole {
|
|
* requests after returning from .drained_begin() until .drained_end() is
|
|
* called.
|
|
*
|
|
+ * These functions must not change the graph (and therefore also must not
|
|
+ * call aio_poll(), which could change the graph indirectly).
|
|
+ *
|
|
* Note that this can be nested. If drained_begin() was called twice, new
|
|
* I/O is allowed only after drained_end() was called twice, too.
|
|
*/
|
|
--
|
|
1.8.3.1
|
|
|