[16/20] block: Mark BlockDriver callbacks for amend job GRAPH_RDLOCK

Commit Message

Kevin Wolf April 25, 2023, 5:31 p.m. UTC

From: Emanuele Giuseppe Esposito <[e-mail]>

This adds GRAPH_RDLOCK annotations to declare that callers of amend
callbacks in BlockDriver need to hold a reader lock for the graph.

Signed-off-by: Emanuele Giuseppe Esposito <[e-mail]>
Signed-off-by: Kevin Wolf <[e-mail]>
---
 include/block/block_int-common.h | 12 ++++++------
 block/amend.c                    |  8 +++++++-
 2 files changed, 13 insertions(+), 7 deletions(-)

Comments

Eric Blake April 25, 2023, 9:20 p.m. UTC | #1

On Tue, Apr 25, 2023 at 07:31:54PM +0200, Kevin Wolf wrote:
> From: Emanuele Giuseppe Esposito <[e-mail]>
> 
> This adds GRAPH_RDLOCK annotations to declare that callers of amend
> callbacks in BlockDriver need to hold a reader lock for the graph.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <[e-mail]>
> Signed-off-by: Kevin Wolf <[e-mail]>
> ---
>  include/block/block_int-common.h | 12 ++++++------
>  block/amend.c                    |  8 +++++++-
>  2 files changed, 13 insertions(+), 7 deletions(-)

Reviewed-by: Eric Blake <[e-mail]>

Stefan Hajnoczi May 1, 2023, 7:07 p.m. UTC | #2

On Tue, Apr 25, 2023 at 07:31:54PM +0200, Kevin Wolf wrote:
> From: Emanuele Giuseppe Esposito <[e-mail]>
> 
> This adds GRAPH_RDLOCK annotations to declare that callers of amend
> callbacks in BlockDriver need to hold a reader lock for the graph.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <[e-mail]>
> Signed-off-by: Kevin Wolf <[e-mail]>
> ---
>  include/block/block_int-common.h | 12 ++++++------
>  block/amend.c                    |  8 +++++++-
>  2 files changed, 13 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi <[e-mail]>

Patch

diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 88ce7f9d9e..37d094796e 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -204,12 +204,13 @@  struct BlockDriver {
      * to allow driver-specific initialization code that requires
      * the BQL, like setting up specific permission flags.
      */
-    int (*bdrv_amend_pre_run)(BlockDriverState *bs, Error **errp);
+    int GRAPH_RDLOCK_PTR (*bdrv_amend_pre_run)(
+        BlockDriverState *bs, Error **errp);
     /*
      * This function is invoked under BQL after .bdrv_co_amend()
      * to allow cleaning up what was done in .bdrv_amend_pre_run().
      */
-    void (*bdrv_amend_clean)(BlockDriverState *bs);
+    void GRAPH_RDLOCK_PTR (*bdrv_amend_clean)(BlockDriverState *bs);
 
     /*
      * Return true if @to_replace can be replaced by a BDS with the
@@ -463,10 +464,9 @@  struct BlockDriver {
 
     int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
 
-    int coroutine_fn (*bdrv_co_amend)(BlockDriverState *bs,
-                                      BlockdevAmendOptions *opts,
-                                      bool force,
-                                      Error **errp);
+    int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_amend)(
+        BlockDriverState *bs, BlockdevAmendOptions *opts, bool force,
+        Error **errp);
 
     /* aio */
     BlockAIOCB * GRAPH_RDLOCK_PTR (*bdrv_aio_preadv)(BlockDriverState *bs,
diff --git a/block/amend.c b/block/amend.c
index bc4bb7b416..53a410247c 100644
--- a/block/amend.c
+++ b/block/amend.c
@@ -46,6 +46,7 @@  static int coroutine_fn blockdev_amend_run(Job *job, Error **errp)
 {
     BlockdevAmendJob *s = container_of(job, BlockdevAmendJob, common);
     int ret;
+    GRAPH_RDLOCK_GUARD();
 
     job_progress_set_remaining(&s->common, 1);
     ret = s->bs->drv->bdrv_co_amend(s->bs, s->opts, s->force, errp);
@@ -54,7 +55,8 @@  static int coroutine_fn blockdev_amend_run(Job *job, Error **errp)
     return ret;
 }
 
-static int blockdev_amend_pre_run(BlockdevAmendJob *s, Error **errp)
+static int GRAPH_RDLOCK
+blockdev_amend_pre_run(BlockdevAmendJob *s, Error **errp)
 {
     if (s->bs->drv->bdrv_amend_pre_run) {
         return s->bs->drv->bdrv_amend_pre_run(s->bs, errp);
@@ -67,9 +69,11 @@  static void blockdev_amend_free(Job *job)
 {
     BlockdevAmendJob *s = container_of(job, BlockdevAmendJob, common);
 
+    bdrv_graph_rdlock_main_loop();
     if (s->bs->drv->bdrv_amend_clean) {
         s->bs->drv->bdrv_amend_clean(s->bs);
     }
+    bdrv_graph_rdunlock_main_loop();
 
     bdrv_unref(s->bs);
 }
@@ -93,6 +97,8 @@  void qmp_x_blockdev_amend(const char *job_id,
     BlockDriver *drv = bdrv_find_format(fmt);
     BlockDriverState *bs;
 
+    GRAPH_RDLOCK_GUARD_MAINLOOP();
+
     bs = bdrv_lookup_bs(NULL, node_name, errp);
     if (!bs) {
         return;