All of lore.kernel.org
 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 12/22] block: Mark bdrv_get_parent_name() and callers GRAPH_RDLOCK
Date: Fri, 29 Sep 2023 16:51:47 +0200	[thread overview]
Message-ID: <20230929145157.45443-13-kwolf@redhat.com> (raw)
In-Reply-To: <20230929145157.45443-1-kwolf@redhat.com>

This adds GRAPH_RDLOCK annotations to declare that callers of
bdrv_get_parent_name() need to hold a reader lock for the graph
because it accesses the parents list of a node.

For some places, we know that they will hold the lock, but we don't have
the GRAPH_RDLOCK annotations yet. In this case, add assume_graph_lock()
with a FIXME comment. These places will be removed once everything is
properly annotated.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.h                |  7 +++----
 include/block/block-io.h     |  8 ++++++--
 include/block/block_int-io.h |  2 +-
 include/block/qapi.h         |  7 ++++---
 block.c                      | 14 ++++++++++++--
 block/backup.c               |  1 +
 block/parallels.c            |  3 +++
 block/qcow.c                 |  3 +++
 block/qcow2-bitmap.c         |  2 ++
 block/qcow2.c                |  4 ++++
 block/quorum.c               |  4 ++--
 block/rbd.c                  |  2 ++
 block/snapshot.c             |  3 +++
 block/vdi.c                  |  3 +++
 block/vpc.c                  |  3 +++
 block/vvfat.c                |  2 ++
 blockjob.c                   |  1 +
 17 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index f789ce3ae0..359bfca4aa 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -1003,10 +1003,9 @@ int coroutine_fn qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp
 bool qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs,
                                           bool release_stored, Error **errp);
 int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp);
-bool coroutine_fn qcow2_co_can_store_new_dirty_bitmap(BlockDriverState *bs,
-                                                      const char *name,
-                                                      uint32_t granularity,
-                                                      Error **errp);
+bool coroutine_fn GRAPH_RDLOCK
+qcow2_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
+                                    uint32_t granularity, Error **errp);
 int coroutine_fn qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs,
                                                          const char *name,
                                                          Error **errp);
diff --git a/include/block/block-io.h b/include/block/block-io.h
index cf72e39717..5b1a1d5998 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -176,8 +176,12 @@ const char *bdrv_get_format_name(BlockDriverState *bs);
 
 bool bdrv_supports_compressed_writes(BlockDriverState *bs);
 const char *bdrv_get_node_name(const BlockDriverState *bs);
-const char *bdrv_get_device_name(const BlockDriverState *bs);
-const char *bdrv_get_device_or_node_name(const BlockDriverState *bs);
+
+const char * GRAPH_RDLOCK
+bdrv_get_device_name(const BlockDriverState *bs);
+
+const char * GRAPH_RDLOCK
+bdrv_get_device_or_node_name(const BlockDriverState *bs);
 
 int coroutine_fn GRAPH_RDLOCK
 bdrv_co_get_info(BlockDriverState *bs, BlockDriverInfo *bdi);
diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h
index 2b6004ab93..34eac72d7a 100644
--- a/include/block/block_int-io.h
+++ b/include/block/block_int-io.h
@@ -99,7 +99,7 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
  */
 void bdrv_wakeup(BlockDriverState *bs);
 
-const char *bdrv_get_parent_name(const BlockDriverState *bs);
+const char * GRAPH_RDLOCK bdrv_get_parent_name(const BlockDriverState *bs);
 bool blk_dev_has_tray(BlockBackend *blk);
 bool blk_dev_is_tray_open(BlockBackend *blk);
 
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 887235653a..54c48de26a 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -33,9 +33,10 @@ BlockDeviceInfo * GRAPH_RDLOCK
 bdrv_block_device_info(BlockBackend *blk, BlockDriverState *bs,
                        bool flat, Error **errp);
 
-int bdrv_query_snapshot_info_list(BlockDriverState *bs,
-                                  SnapshotInfoList **p_list,
-                                  Error **errp);
+int GRAPH_RDLOCK
+bdrv_query_snapshot_info_list(BlockDriverState *bs,
+                              SnapshotInfoList **p_list,
+                              Error **errp);
 void GRAPH_RDLOCK
 bdrv_query_image_info(BlockDriverState *bs, ImageInfo **p_info, bool flat,
                       bool skip_implicit_filters, Error **errp);
diff --git a/block.c b/block.c
index 133efa47e2..9f6cd5bb8e 100644
--- a/block.c
+++ b/block.c
@@ -279,8 +279,9 @@ bool bdrv_is_read_only(BlockDriverState *bs)
     return !(bs->open_flags & BDRV_O_RDWR);
 }
 
-static int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only,
-                                  bool ignore_allow_rdw, Error **errp)
+static int GRAPH_RDLOCK
+bdrv_can_set_read_only(BlockDriverState *bs, bool read_only,
+                       bool ignore_allow_rdw, Error **errp)
 {
     IO_CODE();
 
@@ -319,6 +320,8 @@ int bdrv_apply_auto_read_only(BlockDriverState *bs, const char *errmsg,
     int ret = 0;
     IO_CODE();
 
+    assume_graph_lock(); /* FIXME */
+
     if (!(bs->open_flags & BDRV_O_RDWR)) {
         return 0;
     }
@@ -4949,7 +4952,10 @@ bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
      * to r/w. Attempting to set to r/w may fail if either BDRV_O_ALLOW_RDWR is
      * not set, or if the BDS still has copy_on_read enabled */
     read_only = !(reopen_state->flags & BDRV_O_RDWR);
+
+    bdrv_graph_rdlock_main_loop();
     ret = bdrv_can_set_read_only(reopen_state->bs, read_only, true, &local_err);
+    bdrv_graph_rdunlock_main_loop();
     if (local_err) {
         error_propagate(errp, local_err);
         goto error;
@@ -4983,9 +4989,11 @@ bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
     } else {
         /* It is currently mandatory to have a bdrv_reopen_prepare()
          * handler for each supported drv. */
+        bdrv_graph_rdlock_main_loop();
         error_setg(errp, "Block format '%s' used by node '%s' "
                    "does not support reopening files", drv->format_name,
                    bdrv_get_device_or_node_name(reopen_state->bs));
+        bdrv_graph_rdunlock_main_loop();
         ret = -1;
         goto error;
     }
@@ -7241,6 +7249,8 @@ bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp)
 {
     BdrvOpBlocker *blocker;
     GLOBAL_STATE_CODE();
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
+
     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/backup.c b/block/backup.c
index db3791f4d1..9a3c4bdc82 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -374,6 +374,7 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
     assert(bs);
     assert(target);
     GLOBAL_STATE_CODE();
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     /* QMP interface protects us from these cases */
     assert(sync_mode != MIRROR_SYNC_MODE_INCREMENTAL);
diff --git a/block/parallels.c b/block/parallels.c
index 48c32d6821..39d332ed49 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -1170,9 +1170,12 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         bitmap_new(DIV_ROUND_UP(s->header_size, s->bat_dirty_block));
 
     /* Disable migration until bdrv_activate method is added */
+    bdrv_graph_rdlock_main_loop();
     error_setg(&s->migration_blocker, "The Parallels format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
+    bdrv_graph_rdunlock_main_loop();
+
     ret = migrate_add_blocker(s->migration_blocker, errp);
     if (ret < 0) {
         error_setg(errp, "Migration blocker error");
diff --git a/block/qcow.c b/block/qcow.c
index d56d24ab6d..38a16253b8 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -301,9 +301,12 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* Disable migration when qcow images are used */
+    bdrv_graph_rdlock_main_loop();
     error_setg(&s->migration_blocker, "The qcow format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
+    bdrv_graph_rdunlock_main_loop();
+
     ret = migrate_add_blocker(s->migration_blocker, errp);
     if (ret < 0) {
         error_free(s->migration_blocker);
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 037fa2d435..4e20a28cc7 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -166,6 +166,8 @@ static int check_constraints_on_bitmap(BlockDriverState *bs,
     int64_t len = bdrv_getlength(bs);
     int64_t bitmap_bytes;
 
+    assume_graph_lock(); /* FIXME */
+
     assert(granularity > 0);
     assert((granularity & (granularity - 1)) == 0);
 
diff --git a/block/qcow2.c b/block/qcow2.c
index af43d59d76..db5fe86c5a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2737,6 +2737,8 @@ static int qcow2_inactivate(BlockDriverState *bs)
     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;
@@ -5944,6 +5946,8 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset,
     char *message;
     va_list ap;
 
+    assume_graph_lock(); /* FIXME */
+
     fatal = fatal && bdrv_is_writable(bs);
 
     if (s->signaled_corruption &&
diff --git a/block/quorum.c b/block/quorum.c
index 05220cab7f..d3ffc2ee33 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -206,7 +206,7 @@ static void quorum_report_bad(QuorumOpType type, uint64_t offset,
                                       end_sector - start_sector);
 }
 
-static void quorum_report_failure(QuorumAIOCB *acb)
+static void GRAPH_RDLOCK quorum_report_failure(QuorumAIOCB *acb)
 {
     const char *reference = bdrv_get_device_or_node_name(acb->bs);
     int64_t start_sector = acb->offset / BDRV_SECTOR_SIZE;
@@ -219,7 +219,7 @@ static void quorum_report_failure(QuorumAIOCB *acb)
 
 static int quorum_vote_error(QuorumAIOCB *acb);
 
-static bool quorum_has_too_much_io_failed(QuorumAIOCB *acb)
+static bool GRAPH_RDLOCK quorum_has_too_much_io_failed(QuorumAIOCB *acb)
 {
     BDRVQuorumState *s = acb->bs->opaque;
 
diff --git a/block/rbd.c b/block/rbd.c
index 978671411e..f2defad9da 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1208,6 +1208,8 @@ static int qemu_rbd_reopen_prepare(BDRVReopenState *state,
     BDRVRBDState *s = state->bs->opaque;
     int ret = 0;
 
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
+
     if (s->snap && state->flags & BDRV_O_RDWR) {
         error_setg(errp,
                    "Cannot change node '%s' to r/w when using RBD snapshot",
diff --git a/block/snapshot.c b/block/snapshot.c
index ad2bf6e068..6e16eb803a 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -432,6 +432,7 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
     BlockDriver *drv = bs->drv;
 
     GLOBAL_STATE_CODE();
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     if (!drv) {
         error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
@@ -641,8 +642,10 @@ int bdrv_all_goto_snapshot(const char *name,
         }
         aio_context_release(ctx);
         if (ret < 0) {
+            bdrv_graph_rdlock_main_loop();
             error_prepend(errp, "Could not load snapshot '%s' on '%s': ",
                           name, bdrv_get_device_or_node_name(bs));
+            bdrv_graph_rdunlock_main_loop();
             return -1;
         }
 
diff --git a/block/vdi.c b/block/vdi.c
index 6c35309e04..85b4382612 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -492,9 +492,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* Disable migration when vdi images are used */
+    bdrv_graph_rdlock_main_loop();
     error_setg(&s->migration_blocker, "The vdi format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
+    bdrv_graph_rdunlock_main_loop();
+
     ret = migrate_add_blocker(s->migration_blocker, errp);
     if (ret < 0) {
         error_free(s->migration_blocker);
diff --git a/block/vpc.c b/block/vpc.c
index ceb87dd3d8..945847fe4a 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -446,9 +446,12 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* Disable migration when VHD images are used */
+    bdrv_graph_rdlock_main_loop();
     error_setg(&s->migration_blocker, "The vpc format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
+    bdrv_graph_rdunlock_main_loop();
+
     ret = migrate_add_blocker(s->migration_blocker, errp);
     if (ret < 0) {
         error_free(s->migration_blocker);
diff --git a/block/vvfat.c b/block/vvfat.c
index 0ddc91fc09..7d770c5672 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1145,6 +1145,8 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
     QemuOpts *opts;
     int ret;
 
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
+
 #ifdef DEBUG
     vvv = s;
 #endif
diff --git a/blockjob.c b/blockjob.c
index 58c5d64539..807f992b59 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -485,6 +485,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
     BlockJob *job;
     int ret;
     GLOBAL_STATE_CODE();
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     if (job_id == NULL && !(flags & JOB_INTERNAL)) {
         job_id = bdrv_get_device_name(bs);
-- 
2.41.0



  parent reply	other threads:[~2023-09-29 14:54 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 ` Kevin Wolf [this message]
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 ` [PATCH 17/22] block: Mark bdrv_op_is_blocked() and callers GRAPH_RDLOCK Kevin Wolf
2023-09-29 14:51 ` [PATCH 18/22] block: Mark bdrv_apply_auto_read_only() " 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-13-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.