By using the appropriate variants bdrv_set_backing_hd_drained() and
bdrv_try_change_aio_context_locked(), there only needs to be a single
drained and write-locked section in external_snapshot_abort().
Signed-off-by: Fiona Ebner <f.eb...@proxmox.com>
---
The assumption in the added code comment about the reference is AFAIU
it. Is this correct?
And unrelated, but I'm wondering, isn't this dead code? It's only
executed if state->overlay_appended is set, which happens at the very
end of external_snapshot_action(). How can the transaction still be
aborted after that?
blockdev.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index e625534925..3c53472a23 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1580,11 +1580,19 @@ static void external_snapshot_abort(void *opaque)
AioContext *tmp_context;
int ret;
+ bdrv_graph_wrlock_drained();
+
aio_context = bdrv_get_aio_context(state->old_bs);
- bdrv_ref(state->old_bs); /* we can't let bdrv_set_backind_hd()
- close state->old_bs; we need it */
- bdrv_set_backing_hd(state->new_bs, NULL, &error_abort);
+ /*
+ * Note that state->old_bs would not disappear during the
+ * write-locked section, because the unref from
+ * bdrv_set_backing_hd_drained() only happens at the end of the
+ * write-locked section. However, just be explicit about keeping a
+ * reference and don't rely on that implicit detail.
+ */
+ bdrv_ref(state->old_bs);
+ bdrv_set_backing_hd_drained(state->new_bs, NULL, &error_abort);
/*
* The call to bdrv_set_backing_hd() above returns state->old_bs to
@@ -1593,16 +1601,14 @@ static void external_snapshot_abort(void *opaque)
*/
tmp_context = bdrv_get_aio_context(state->old_bs);
if (aio_context != tmp_context) {
- ret = bdrv_try_change_aio_context(state->old_bs,
- aio_context, NULL, NULL);
+ ret = bdrv_try_change_aio_context_locked(state->old_bs,
+ aio_context, NULL,
+ NULL);
assert(ret == 0);
}
- bdrv_drained_begin(state->new_bs);
- bdrv_graph_wrlock();
bdrv_replace_node(state->new_bs, state->old_bs, &error_abort);
bdrv_graph_wrunlock();
- bdrv_drained_end(state->new_bs);
bdrv_unref(state->old_bs); /* bdrv_replace_node() ref'ed old_bs */
}
--
2.39.5
