[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH] block: temporarily hold the new AioContext of bs_top in bdrv_app
From: |
Stefano Garzarella |
Subject: |
[PATCH] block: temporarily hold the new AioContext of bs_top in bdrv_append() |
Date: |
Tue, 14 Feb 2023 11:51:56 +0100 |
bdrv_append() is called with bs_top AioContext held, but
bdrv_attach_child_noperm() could change the AioContext of bs_top.
bdrv_replace_node_noperm() calls bdrv_drained_begin() starting from
commit 2398747128 ("block: Don't poll in bdrv_replace_child_noperm()").
bdrv_drained_begin() can call BDRV_POLL_WHILE that assumes the new lock
is taken, so let's temporarily hold the new AioContext to prevent QEMU
from failing in BDRV_POLL_WHILE when it tries to release the wrong
AioContext.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2168209
Reported-by: Aihua Liang <aliang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
I'm not sure whether to use the following Fixes tag. That commit added the
calls to bdrv_drained_begin() in bdrv_replace_node_noperm(), but maybe the
problem was pre-existing.
Fixes: 2398747128 ("block: Don't poll in bdrv_replace_child_noperm()")
Note: a local reproducer is attached in the BZ, it is based on the Aihua Liang
report and it hits the issue with a 20% ratio.
---
block.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/block.c b/block.c
index aa9062f2c1..0e2bc11e0b 100644
--- a/block.c
+++ b/block.c
@@ -5266,6 +5266,8 @@ int bdrv_drop_filter(BlockDriverState *bs, Error **errp)
* child.
*
* This function does not create any image files.
+ *
+ * The caller must hold the AioContext lock for @bs_top.
*/
int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
Error **errp)
@@ -5273,11 +5275,14 @@ int bdrv_append(BlockDriverState *bs_new,
BlockDriverState *bs_top,
int ret;
BdrvChild *child;
Transaction *tran = tran_new();
+ AioContext *old_context, *new_context;
GLOBAL_STATE_CODE();
assert(!bs_new->backing);
+ old_context = bdrv_get_aio_context(bs_top);
+
child = bdrv_attach_child_noperm(bs_new, bs_top, "backing",
&child_of_bds, bdrv_backing_role(bs_new),
tran, errp);
@@ -5286,11 +5291,29 @@ int bdrv_append(BlockDriverState *bs_new,
BlockDriverState *bs_top,
goto out;
}
+ /*
+ * bdrv_attach_child_noperm could change the AioContext of bs_top.
+ * bdrv_replace_node_noperm calls bdrv_drained_begin, so let's temporarily
+ * hold the new AioContext, since bdrv_drained_begin calls BDRV_POLL_WHILE
+ * that assumes the new lock is taken.
+ */
+ new_context = bdrv_get_aio_context(bs_top);
+
+ if (old_context != new_context) {
+ aio_context_release(old_context);
+ aio_context_acquire(new_context);
+ }
+
ret = bdrv_replace_node_noperm(bs_top, bs_new, true, tran, errp);
if (ret < 0) {
goto out;
}
+ if (old_context != new_context) {
+ aio_context_release(new_context);
+ aio_context_acquire(old_context);
+ }
+
ret = bdrv_refresh_perms(bs_new, tran, errp);
out:
tran_finalize(tran, ret);
--
2.39.1
- [PATCH] block: temporarily hold the new AioContext of bs_top in bdrv_append(),
Stefano Garzarella <=