qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, stefanha@redhat.com, eesposit@redhat.com,
	eblake@redhat.com, pbonzini@redhat.com,
	vsementsov@yandex-team.ru, qemu-devel@nongnu.org
Subject: [PATCH 17/22] block: Mark bdrv_op_is_blocked() and callers GRAPH_RDLOCK
Date: Fri, 29 Sep 2023 16:51:52 +0200	[thread overview]
Message-ID: <20230929145157.45443-18-kwolf@redhat.com> (raw)
In-Reply-To: <20230929145157.45443-1-kwolf@redhat.com>

This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_op_is_blocked() 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.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/block-global-state.h |  4 +++-
 block.c                            |  2 --
 block/block-backend.c              |  1 +
 block/commit.c                     |  1 +
 block/monitor/block-hmp-cmds.c     |  3 +++
 block/qapi-sysemu.c                |  9 +++++++--
 blockdev.c                         | 15 +++++++++++++++
 7 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h
index 794ef34ded..6bfafe781d 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -246,7 +246,9 @@ bdrv_attach_child(BlockDriverState *parent_bs,
                   BdrvChildRole child_role,
                   Error **errp);
 
-bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
+bool GRAPH_RDLOCK
+bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
+
 void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason);
 void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason);
 void bdrv_op_block_all(BlockDriverState *bs, Error *reason);
diff --git a/block.c b/block.c
index 39c28a8dde..0814b0704e 100644
--- a/block.c
+++ b/block.c
@@ -7250,8 +7250,6 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
     BdrvOpBlocker *blocker;
     GLOBAL_STATE_CODE();
 
-    assume_graph_lock(); /* FIXME */
-
     assert((int) op >= 0 && op < BLOCK_OP_TYPE_MAX);
     if (!QLIST_EMPTY(&bs->op_blockers[op])) {
         blocker = QLIST_FIRST(&bs->op_blockers[op]);
diff --git a/block/block-backend.c b/block/block-backend.c
index 0fd1b4a437..a7057400d7 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2388,6 +2388,7 @@ bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp)
 {
     BlockDriverState *bs = blk_bs(blk);
     GLOBAL_STATE_CODE();
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     if (!bs) {
         return false;
diff --git a/block/commit.c b/block/commit.c
index aa45beb0f0..43d1de7577 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -434,6 +434,7 @@ int bdrv_commit(BlockDriverState *bs)
     Error *local_err = NULL;
 
     GLOBAL_STATE_CODE();
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     if (!drv)
         return -ENOMEDIUM;
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 6b7d7dd072..7645c7e5fb 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -144,6 +144,9 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict)
     AioContext *aio_context;
     Error *local_err = NULL;
 
+    GLOBAL_STATE_CODE();
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
+
     bs = bdrv_find_node(id);
     if (bs) {
         qmp_blockdev_del(id, &local_err);
diff --git a/block/qapi-sysemu.c b/block/qapi-sysemu.c
index e885a64c32..3f614cbc04 100644
--- a/block/qapi-sysemu.c
+++ b/block/qapi-sysemu.c
@@ -169,14 +169,16 @@ void qmp_blockdev_close_tray(const char *device,
     }
 }
 
-static void blockdev_remove_medium(const char *device, const char *id,
-                                   Error **errp)
+static void GRAPH_UNLOCKED
+blockdev_remove_medium(const char *device, const char *id, Error **errp)
 {
     BlockBackend *blk;
     BlockDriverState *bs;
     AioContext *aio_context;
     bool has_attached_device;
 
+    GLOBAL_STATE_CODE();
+
     blk = qmp_get_blk(device, id, errp);
     if (!blk) {
         return;
@@ -205,9 +207,12 @@ static void blockdev_remove_medium(const char *device, const char *id,
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
 
+    bdrv_graph_rdlock_main_loop();
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_EJECT, errp)) {
+        bdrv_graph_rdunlock_main_loop();
         goto out;
     }
+    bdrv_graph_rdunlock_main_loop();
 
     blk_remove_bs(blk);
 
diff --git a/blockdev.c b/blockdev.c
index 51c58dd432..a01c62596b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1696,9 +1696,12 @@ static void drive_backup_action(DriveBackup *backup,
     }
 
     /* Early check to avoid creating target */
+    bdrv_graph_rdlock_main_loop();
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
+        bdrv_graph_rdunlock_main_loop();
         goto out;
     }
+    bdrv_graph_rdunlock_main_loop();
 
     flags = bs->open_flags | BDRV_O_RDWR;
 
@@ -2360,10 +2363,13 @@ void coroutine_fn qmp_block_resize(const char *device, const char *node_name,
         return;
     }
 
+    bdrv_graph_co_rdlock();
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {
         error_setg(errp, QERR_DEVICE_IN_USE, device);
+        bdrv_graph_co_rdunlock();
         return;
     }
+    bdrv_graph_co_rdunlock();
 
     blk = blk_co_new_with_bs(bs, BLK_PERM_RESIZE, BLK_PERM_ALL, errp);
     if (!blk) {
@@ -2487,13 +2493,16 @@ void qmp_block_stream(const char *job_id, const char *device,
      * Check for op blockers in the whole chain between bs and base (or bottom)
      */
     iter_end = bottom ? bdrv_filter_or_cow_bs(bottom_bs) : base_bs;
+    bdrv_graph_rdlock_main_loop();
     for (iter = bs; iter && iter != iter_end;
          iter = bdrv_filter_or_cow_bs(iter))
     {
         if (bdrv_op_is_blocked(iter, BLOCK_OP_TYPE_STREAM, errp)) {
+            bdrv_graph_rdunlock_main_loop();
             goto out;
         }
     }
+    bdrv_graph_rdunlock_main_loop();
 
     /* if we are streaming the entire chain, the result will have no backing
      * file, and specifying one is therefore an error */
@@ -3021,9 +3030,12 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
     }
 
     /* Early check to avoid creating target */
+    bdrv_graph_rdlock_main_loop();
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_MIRROR_SOURCE, errp)) {
+        bdrv_graph_rdunlock_main_loop();
         return;
     }
+    bdrv_graph_rdunlock_main_loop();
 
     aio_context = bdrv_get_aio_context(bs);
     aio_context_acquire(aio_context);
@@ -3409,9 +3421,12 @@ void qmp_change_backing_file(const char *device,
 
     /* even though we are not necessarily operating on bs, we need it to
      * determine if block ops are currently prohibited on the chain */
+    bdrv_graph_rdlock_main_loop();
     if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_CHANGE, errp)) {
+        bdrv_graph_rdunlock_main_loop();
         goto out;
     }
+    bdrv_graph_rdunlock_main_loop();
 
     /* final sanity check */
     if (!bdrv_chain_contains(bs, image_bs)) {
-- 
2.41.0



  parent reply	other threads:[~2023-09-29 14:53 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-29 14:51 [PATCH 00/22] block: Graph locking part 5 (protect children/parent links) Kevin Wolf
2023-09-29 14:51 ` [PATCH 01/22] test-bdrv-drain: Don't call bdrv_graph_wrlock() in coroutine context Kevin Wolf
2023-09-29 14:51 ` [PATCH 02/22] block-coroutine-wrapper: Add no_co_wrapper_bdrv_rdlock functions Kevin Wolf
2023-09-29 14:51 ` [PATCH 03/22] block: Take graph rdlock in bdrv_inactivate_all() Kevin Wolf
2023-09-29 14:51 ` [PATCH 04/22] block: Mark bdrv_first_blk() and bdrv_is_root_node() GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 05/22] block: Mark drain related functions GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 06/22] block: Mark bdrv_parent_cb_resize() and callers GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 07/22] block: Mark bdrv_snapshot_fallback() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 08/22] block: Take graph rdlock in parts of reopen Kevin Wolf
2023-09-29 14:51 ` [PATCH 09/22] block: Mark bdrv_get_xdbg_block_graph() and callers GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 10/22] block: Mark bdrv_refresh_filename() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 11/22] block: Mark bdrv_primary_child() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 12/22] block: Mark bdrv_get_parent_name() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 13/22] block: Mark bdrv_amend_options() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 14/22] qcow2: Mark qcow2_signal_corruption() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 15/22] qcow2: Mark qcow2_inactivate() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 16/22] qcow2: Mark check_constraints_on_bitmap() GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` Kevin Wolf [this message]
2023-09-29 14:51 ` [PATCH 18/22] block: Mark bdrv_apply_auto_read_only() and callers GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 19/22] block: Mark bdrv_get_specific_info() " Kevin Wolf
2023-09-29 14:51 ` [PATCH 20/22] block: Protect bs->parents with graph_lock Kevin Wolf
2023-09-29 14:51 ` [PATCH 21/22] block: Protect bs->children " Kevin Wolf
2023-09-29 14:51 ` [PATCH 22/22] block: Add assertion for bdrv_graph_wrlock() Kevin Wolf
2023-10-10 20:48 ` [PATCH 00/22] block: Graph locking part 5 (protect children/parent links) Stefan Hajnoczi
2023-10-11 11:05   ` Kevin Wolf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230929145157.45443-18-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=eblake@redhat.com \
    --cc=eesposit@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@yandex-team.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).