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 <eespo...@redhat.com> --- block.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index 29de2b62b5..fb5bc3077a 100644 --- a/block.c +++ b/block.c @@ -2879,8 +2879,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 @@ -2897,6 +2895,10 @@ static void bdrv_replace_child_noperm(BdrvChild **childp, if (child->klass->attach) { child->klass->attach(child); } + + assert_bdrv_graph_writable(new_bs); + QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent); + } /* -- 2.31.1