From: Andrey Drobyshev <[email protected]>
The block layer's graph drain series [0] moved many drain operations outside
of graph locks, but introduced a regression: some function now unconditionally
use bdrv_graph_wrlock_drained(), which drains all BDS in the system and
asserts no I/O is in flight.
When 'blockdev-add' creates a new node, it creates a temporary
BlockBackend which is only used for probing, only to delete it later.
Before deletion, it's also attaching child 'file' BDS to that same
BlockBackend in bdrv_open_child_common().
Both blk_remove_bs() and bdrv_open_child_common() call
bdrv_graph_wrlock_drained() unconditionally. If we have an incoming IO
at the same time, it might lead to assertion failure:
qemu-system-x86_64: ../block/io.c:441: bdrv_drain_assert_idle: \
Assertion `qatomic_read(&bs->in_flight) == 0' failed.
The global drain is unnecessary for temporary nodes created during
'blockdev-add'. Only live device roots require full graph draining.
[0]
https://lore.kernel.org/qemu-devel/[email protected]/
Signed-off-by: Andrey Drobyshev <[email protected]>
---
block.c | 8 +++++++-
block/block-backend.c | 11 ++++++++++-
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index cf08e64add..26dfc624b6 100644
--- a/block.c
+++ b/block.c
@@ -3796,10 +3796,16 @@ bdrv_open_child_common(const char *filename, QDict
*options,
return NULL;
}
- bdrv_graph_wrlock_drained();
+
+ /* We only need this parent's subtree to be drained, not the entire graph.
+ * So we avoid global drain here.
+ */
+ bdrv_drained_begin(parent);
+ bdrv_graph_wrlock();
child = bdrv_attach_child(parent, bs, bdref_key, child_class, child_role,
errp);
bdrv_graph_wrunlock();
+ bdrv_drained_end(parent);
return child;
}
diff --git a/block/block-backend.c b/block/block-backend.c
index f8d6ba65c1..d8110cbaff 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -889,7 +889,16 @@ void blk_remove_bs(BlockBackend *blk)
root = blk->root;
blk->root = NULL;
- bdrv_graph_wrlock_drained();
+ if (blk->dev) {
+ bdrv_graph_wrlock_drained();
+ } else {
+ /* We suppose that blk with an unattached dev is temporary (e.g.
+ * used for probing in bdrv_open_inherit()) and therefore global
+ * drain is unnecessary. Draining of this blk's subtree is done above
+ * in blk_drain().
+ */
+ bdrv_graph_wrlock();
+ }
bdrv_root_unref_child(root);
bdrv_graph_wrunlock();
}
--
2.43.5
