Re: [PATCH 3/8] qcow2: Unlock the graph in qcow2_do_open() where necessary

[Eric Blake]

On Wed, May 10, 2023 at 10:35:56PM +0200, Kevin Wolf wrote:
> 
> qcow2_do_open() calls a few no_co_wrappers that wrap functions taking
> the graph lock internally as a writer. Therefore, it can't hold the
> reader lock across these calls, it causes deadlocks. Drop the lock
> temporarily around the calls.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block/qcow2.c | 6 ++
>  1 file changed, 6 insertions(+)

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[PATCH 3/8] qcow2: Unlock the graph in qcow2_do_open() where necessary

[Kevin Wolf]

qcow2_do_open() calls a few no_co_wrappers that wrap functions taking
the graph lock internally as a writer. Therefore, it can't hold the
reader lock across these calls, it causes deadlocks. Drop the lock
temporarily around the calls.

Signed-off-by: Kevin Wolf 
---
 block/qcow2.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 73f36447f9..b00b4e7575 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1619,9 +1619,11 @@ qcow2_do_open(BlockDriverState *bs, QDict *options, int 
flags,
 
 if (open_data_file) {
 /* Open external data file */
+bdrv_graph_co_rdunlock();
 s->data_file = bdrv_co_open_child(NULL, options, "data-file", bs,
   &child_of_bds, BDRV_CHILD_DATA,
   true, errp);
+bdrv_graph_co_rdlock();
 if (*errp) {
 ret = -EINVAL;
 goto fail;
@@ -1629,10 +1631,12 @@ qcow2_do_open(BlockDriverState *bs, QDict *options, int 
flags,
 
 if (s->incompatible_features & QCOW2_INCOMPAT_DATA_FILE) {
 if (!s->data_file && s->image_data_file) {
+bdrv_graph_co_rdunlock();
 s->data_file = bdrv_co_open_child(s->image_data_file, options,
   "data-file", bs,
   &child_of_bds,
   BDRV_CHILD_DATA, false, 
errp);
+bdrv_graph_co_rdlock();
 if (!s->data_file) {
 ret = -EINVAL;
 goto fail;
@@ -1857,7 +1861,9 @@ qcow2_do_open(BlockDriverState *bs, QDict *options, int 
flags,
  fail:
 g_free(s->image_data_file);
 if (open_data_file && has_data_file(bs)) {
+bdrv_graph_co_rdunlock();
 bdrv_unref_child(bs, s->data_file);
+bdrv_graph_co_rdlock();
 s->data_file = NULL;
 }
 g_free(s->unknown_header_fields);
-- 
2.40.1