[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v2 05/10] block.c: bdrv_replace_child_noperm: first call ->attach
From: |
Emanuele Giuseppe Esposito |
Subject: |
[PATCH v2 05/10] block.c: bdrv_replace_child_noperm: first call ->attach(), and then add child |
Date: |
Mon, 14 Mar 2022 09:18:49 -0400 |
Doing the opposite can make adding the child node to a non-drained node,
as apply_subtree_drain is only done in ->attach() and thus make
assert_bdrv_graph_writable fail.
This can happen for example during a transaction rollback (test 245,
test_io_with_graph_changes):
1. a node is removed from the graph, thus it is undrained
2. then something happens, and we need to roll back the transactions
through tran_abort()
3. at this point, the current code would first attach the undrained node
to the graph via QLIST_INSERT_HEAD, and then call ->attach() that
will take care of restoring the drain with apply_subtree_drain(),
leaving the node undrained between the two operations.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index d870ba5393..c6a550f9c6 100644
--- a/block.c
+++ b/block.c
@@ -1434,6 +1434,11 @@ static void bdrv_inherited_options(BdrvChildRole role,
bool parent_is_format,
*child_flags = flags;
}
+/*
+ * Add the child node to child->opaque->children list,
+ * and then apply the drain to the whole child subtree,
+ * so that the drain count matches with the parent.
+ */
static void bdrv_child_cb_attach(BdrvChild *child)
{
BlockDriverState *bs = child->opaque;
@@ -2889,8 +2894,6 @@ static void bdrv_replace_child_noperm(BdrvChild **childp,
}
if (new_bs) {
- assert_bdrv_graph_writable(new_bs);
- QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent);
/*
* Detaching the old node may have led to the new node's
@@ -2901,12 +2904,19 @@ static void bdrv_replace_child_noperm(BdrvChild
**childp,
assert(new_bs->quiesce_counter <= new_bs_quiesce_counter);
drain_saldo += new_bs->quiesce_counter - new_bs_quiesce_counter;
- /* Attach only after starting new drained sections, so that recursive
- * drain sections coming from @child don't get an extra .drained_begin
- * callback. */
+ /*
+ * First call ->attach() cb.
+ * In child_of_bds case, add child to the parent
+ * (child->opaque) ->children list and if
+ * necessary add missing drains in the child subtree.
+ */
if (child->klass->attach) {
child->klass->attach(child);
}
+
+ /* Then add child to new_bs->parents list */
+ assert_bdrv_graph_writable(new_bs);
+ QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent);
}
/*
--
2.31.1
- [PATCH v2 00/10] block: bug fixes in preparation of AioContext removal, Emanuele Giuseppe Esposito, 2022/03/14
- [PATCH v2 03/10] block/io.c: fix bdrv_child_cb_drained_begin invocations from a coroutine, Emanuele Giuseppe Esposito, 2022/03/14
- [PATCH v2 07/10] test-bdrv-drain.c: remove test_detach_by_parent_cb(), Emanuele Giuseppe Esposito, 2022/03/14
- [PATCH v2 06/10] test-bdrv-drain.c: adapt test to support additional subtree drains, Emanuele Giuseppe Esposito, 2022/03/14
- [PATCH v2 05/10] block.c: bdrv_replace_child_noperm: first call ->attach(), and then add child,
Emanuele Giuseppe Esposito <=
- [PATCH v2 01/10] drains: create bh only when polling, Emanuele Giuseppe Esposito, 2022/03/14
- [PATCH v2 09/10] child_job_drained_poll: override polling condition only when in home thread, Emanuele Giuseppe Esposito, 2022/03/14
- [PATCH v2 10/10] tests/qemu-iotests/030: test_stream_parallel should use auto_finalize=False, Emanuele Giuseppe Esposito, 2022/03/14
- [PATCH v2 02/10] bdrv_parent_drained_begin_single: handle calls from coroutine context, Emanuele Giuseppe Esposito, 2022/03/14
- [PATCH v2 08/10] tests/unit/test-bdrv-drain.c: graph setup functions can't run in coroutines, Emanuele Giuseppe Esposito, 2022/03/14
- [PATCH v2 04/10] block.c: bdrv_replace_child_noperm: first remove the child, and then call ->detach(), Emanuele Giuseppe Esposito, 2022/03/14