commit 430da832afb6a6eebb7c1726991c60fb06322d3e
author Kevin Wolf <kwolf@redhat.com> Fri Oct 27 17:53:16 2023 +0200
committer Kevin Wolf <kwolf@redhat.com> Tue Nov 07 19:14:19 2023 +0100
tree 0fbd2e3ecd95d23dec87312fecc84fef09b62efc
parent 372b69f503d47eb6619a98cac2ab5a6a569e3483

block: Mark bdrv_skip_implicit_filters() and callers GRAPH_RDLOCK

This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_skip_implicit_filters() need to hold a reader lock for the graph
because it calls bdrv_filter_child(), which accesses bs->file/backing.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20231027155333.420094-8-kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

block.c

diff --git a/block.c b/block.c
index a6060ed..7e8b397 100644
--- a/block.c
+++ b/block.c
@@ -4778,6 +4778,8 @@
         return 0;
     }
 
+    bdrv_graph_rdlock_main_loop();
+
     switch (qobject_type(value)) {
     case QTYPE_QNULL:
         assert(is_backing); /* The 'file' option does not allow a null value */
@@ -4787,17 +4789,16 @@
         str = qstring_get_str(qobject_to(QString, value));
         new_child_bs = bdrv_lookup_bs(NULL, str, errp);
         if (new_child_bs == NULL) {
-            return -EINVAL;
+            ret = -EINVAL;
+            goto out_rdlock;
         }
 
-        bdrv_graph_rdlock_main_loop();
         has_child = bdrv_recurse_has_child(new_child_bs, bs);
-        bdrv_graph_rdunlock_main_loop();
-
         if (has_child) {
             error_setg(errp, "Making '%s' a %s child of '%s' would create a "
                        "cycle", str, child_name, bs->node_name);
-            return -EINVAL;
+            ret = -EINVAL;
+            goto out_rdlock;
         }
         break;
     default:
@@ -4809,18 +4810,21 @@
     }
 
     if (old_child_bs == new_child_bs) {
-        return 0;
+        ret = 0;
+        goto out_rdlock;
     }
 
     if (old_child_bs) {
         if (bdrv_skip_implicit_filters(old_child_bs) == new_child_bs) {
-            return 0;
+            ret = 0;
+            goto out_rdlock;
         }
 
         if (old_child_bs->implicit) {
             error_setg(errp, "Cannot replace implicit %s child of %s",
                        child_name, bs->node_name);
-            return -EPERM;
+            ret = -EPERM;
+            goto out_rdlock;
         }
     }
 
@@ -4831,7 +4835,8 @@
          */
         error_setg(errp, "'%s' is a %s filter node that does not support a "
                    "%s child", bs->node_name, bs->drv->format_name, child_name);
-        return -EINVAL;
+        ret = -EINVAL;
+        goto out_rdlock;
     }
 
     if (is_backing) {
@@ -4852,6 +4857,7 @@
         aio_context_acquire(ctx);
     }
 
+    bdrv_graph_rdunlock_main_loop();
     bdrv_graph_wrlock(new_child_bs);
 
     ret = bdrv_set_file_or_backing_noperm(bs, new_child_bs, is_backing,
@@ -4870,6 +4876,10 @@
     }
 
     return ret;
+
+out_rdlock:
+    bdrv_graph_rdunlock_main_loop();
+    return ret;
 }
 
 /*