From de4fed6f4ef7f2f1a1bce50a2cbbb7fdd397b7ec Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 29 Sep 2023 16:51:50 +0200 Subject: [PATCH] qcow2: Mark qcow2_inactivate() and callers GRAPH_RDLOCK This adds GRAPH_RDLOCK annotations to declare that callers of qcow2_inactivate() need to hold a reader lock for the graph because it calls bdrv_get_device_or_node_name(), which accesses the parents list of a node. qcow2_do_close() is a bit strange because it is called from different contexts. If close_data_file = true, we know that we were called from non-coroutine main loop context (more specifically, we're coming from qcow2_close()) and can safely drop the reader lock temporarily with bdrv_graph_rdunlock_main_loop() and acquire the writer lock. Signed-off-by: Kevin Wolf Message-ID: <20230929145157.45443-16-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/qcow2.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 137c036ea4..aa01d9e7b5 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2737,14 +2737,12 @@ fail_nometa: return ret; } -static int qcow2_inactivate(BlockDriverState *bs) +static int GRAPH_RDLOCK qcow2_inactivate(BlockDriverState *bs) { BDRVQcow2State *s = bs->opaque; int ret, result = 0; Error *local_err = NULL; - assume_graph_lock(); /* FIXME */ - qcow2_store_persistent_dirty_bitmaps(bs, true, &local_err); if (local_err != NULL) { result = -EINVAL; @@ -2774,7 +2772,8 @@ static int qcow2_inactivate(BlockDriverState *bs) return result; } -static void qcow2_do_close(BlockDriverState *bs, bool close_data_file) +static void coroutine_mixed_fn GRAPH_RDLOCK +qcow2_do_close(BlockDriverState *bs, bool close_data_file) { BDRVQcow2State *s = bs->opaque; qemu_vfree(s->l1_table); @@ -2801,18 +2800,24 @@ static void qcow2_do_close(BlockDriverState *bs, bool close_data_file) g_free(s->image_backing_format); if (close_data_file && has_data_file(bs)) { + GLOBAL_STATE_CODE(); + bdrv_graph_rdunlock_main_loop(); bdrv_graph_wrlock(NULL); bdrv_unref_child(bs, s->data_file); bdrv_graph_wrunlock(); s->data_file = NULL; + bdrv_graph_rdlock_main_loop(); } qcow2_refcount_close(bs); qcow2_free_snapshots(bs); } -static void qcow2_close(BlockDriverState *bs) +static void GRAPH_UNLOCKED qcow2_close(BlockDriverState *bs) { + GLOBAL_STATE_CODE(); + GRAPH_RDLOCK_GUARD_MAINLOOP(); + qcow2_do_close(bs, true); }