diff mbox series

[v2,6/8] block: do not acquire AioContext in check_to_replace_node

Message ID 20210419085541.22310-7-eesposit@redhat.com
State New
Headers show
Series Block layer thread-safety, continued | expand

Commit Message

Emanuele Giuseppe Esposito April 19, 2021, 8:55 a.m. UTC 
Make the (only) caller do it.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 block.c               | 22 +++++-----------------
 blockdev.c            |  7 ++++++-
 include/block/block.h |  1 +
 3 files changed, 12 insertions(+), 18 deletions(-)

Comments

Paolo Bonzini May 5, 2021, 10:10 a.m. UTC | #1 
On 19/04/21 10:55, Emanuele Giuseppe Esposito wrote:
> + *
> + * Called with AioContext lock held.


... for @to_replace_bs.

>           aio_context_acquire(replace_aio_context);
> +        if (!check_to_replace_node(bs, to_replace_bs, replaces, errp)) {

A release is missing here.

Paolo

> +            return;
> +        }
>           replace_size = bdrv_getlength(to_replace_bs);
diff mbox series

Patch

diff --git a/block.c b/block.c
index f40fb63c75..fd12f88062 100644
--- a/block.c
+++ b/block.c
@@ -6776,24 +6776,15 @@  bool bdrv_recurse_can_replace(BlockDriverState *bs,
  *
  * The result (whether the node can be replaced or not) is only valid
  * for as long as no graph or permission changes occur.
+ *
+ * Called with AioContext lock held.
  */
 BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs,
+                                        BlockDriverState *to_replace_bs,
                                         const char *node_name, Error **errp)
 {
-    BlockDriverState *to_replace_bs = bdrv_find_node(node_name);
-    AioContext *aio_context;
-
-    if (!to_replace_bs) {
-        error_setg(errp, "Failed to find node with node-name='%s'", node_name);
-        return NULL;
-    }
-
-    aio_context = bdrv_get_aio_context(to_replace_bs);
-    aio_context_acquire(aio_context);
-
     if (bdrv_op_is_blocked(to_replace_bs, BLOCK_OP_TYPE_REPLACE, errp)) {
-        to_replace_bs = NULL;
-        goto out;
+        return NULL;
     }
 
     /* We don't want arbitrary node of the BDS chain to be replaced only the top
@@ -6806,12 +6797,9 @@  BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs,
                    "because it cannot be guaranteed that doing so would not "
                    "lead to an abrupt change of visible data",
                    node_name, parent_bs->node_name);
-        to_replace_bs = NULL;
-        goto out;
+        return NULL;
     }
 
-out:
-    aio_context_release(aio_context);
     return to_replace_bs;
 }
 
diff --git a/blockdev.c b/blockdev.c
index a57590aae4..e901107344 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3056,13 +3056,18 @@  static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,
             return;
         }
 
-        to_replace_bs = check_to_replace_node(bs, replaces, errp);
+        to_replace_bs = bdrv_find_node(replaces);
         if (!to_replace_bs) {
+            error_setg(errp, "Failed to find node with node-name='%s'",
+                       replaces);
             return;
         }
 
         replace_aio_context = bdrv_get_aio_context(to_replace_bs);
         aio_context_acquire(replace_aio_context);
+        if (!check_to_replace_node(bs, to_replace_bs, replaces, errp)) {
+            return;
+        }
         replace_size = bdrv_getlength(to_replace_bs);
         aio_context_release(replace_aio_context);
 
diff --git a/include/block/block.h b/include/block/block.h
index b3f6e509d4..f57c34a19a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -474,6 +474,7 @@  int bdrv_amend_options(BlockDriverState *bs_new, QemuOpts *opts,
 
 /* check if a named node can be replaced when doing drive-mirror */
 BlockDriverState *check_to_replace_node(BlockDriverState *parent_bs,
+                                        BlockDriverState *to_replace_bs,
                                         const char *node_name, Error **errp);
 
 /* async block I/O */