[PATCH v2 00/26] block: fix coroutine_fn annotations

[PATCH v2 00/26] block: fix coroutine_fn annotations

[Paolo Bonzini]

This is the initial result of reviving Marc-André's series at
https://patchew.org/QEMU/20170704220346.29244-1-marcandre.lureau@redhat.com/.
A lot of the patches are similar to the ones that Marc-André wrote,
but due to the changes in the code it was easier to redo them.

For nbd, the patch is on top of "nbd: mark more coroutine_fns" that
I sent a few days ago and that (AIUI) Eric has already queued; only
one function was missing, much to my surprise.

Apart from this, I also identified the following functions that
can be called both in coroutine context and outside:

- qmp_dispatch
- schedule_next_request
- nvme_get_free_req
- bdrv_create
- bdrv_remove_persistent_dirty_bitmap
- bdrv_can_store_new_dirty_bitmap
- bdrv_do_drained_begin
- bdrv_do_drained_end
- bdrv_drain_all_begin
- qcow2_open
- qcow2_has_zero_init
- bdrv_qed_open
- qio_channel_readv_full_all_eof
- qio_channel_writev_full_all

besides, of course, everything that is generated by
scripts/block-coroutine-wrapper.py.

The patches are exactly the same as v1, but I have improved the commit
messages for the "remove incorrect coroutine_fn annotations" bits.

Marc-André Lureau (3):
  9p: add missing coroutine_fn annotations
  migration: add missing coroutine_fn annotations
  test-coroutine: add missing coroutine_fn annotations

Paolo Bonzini (23):
  block: remove incorrect coroutine_fn annotations
  qcow2: remove incorrect coroutine_fn annotations
  nbd: remove incorrect coroutine_fn annotations
  coroutine: remove incorrect coroutine_fn annotations
  blkdebug: add missing coroutine_fn annotations
  blkverify: add missing coroutine_fn annotations
  block: add missing coroutine_fn annotations
  file-posix: add missing coroutine_fn annotations
  iscsi: add missing coroutine_fn annotations
  nbd: add missing coroutine_fn annotations
  nfs: add missing coroutine_fn annotations
  nvme: add missing coroutine_fn annotations
  parallels: add missing coroutine_fn annotations
  qcow2: add missing coroutine_fn annotations
  copy-before-write: add missing coroutine_fn annotations
  curl: add missing coroutine_fn annotations
  qed: add missing coroutine_fn annotations
  quorum: add missing coroutine_fn annotations
  throttle: add missing coroutine_fn annotations
  vmdk: add missing coroutine_fn annotations
  job: add missing coroutine_fn annotations
  coroutine-lock: add missing coroutine_fn annotations
  raw-format: add missing coroutine_fn annotations

 block/blkdebug.c            | 14 +++++++-------
 block/blkverify.c           |  2 +-
 block/block-backend.c       | 26 +++++++++++++-------------
 block/copy-before-write.c   |  8 ++++----
 block/curl.c                |  2 +-
 block/file-posix.c          |  2 +-
 block/io.c                  | 24 ++++++++++++------------
 block/iscsi.c               |  2 +-
 block/nbd.c                 | 10 +++++-----
 block/nfs.c                 |  2 +-
 block/nvme.c                |  5 +++--
 block/parallels.c           |  5 +++--
 block/qcow2-cluster.c       | 18 +++++++++---------
 block/qcow2-refcount.c      |  6 +++---
 block/qcow2.c               |  4 ++--
 block/qcow2.h               | 18 +++++++++---------
 block/qed.c                 |  4 ++--
 block/quorum.c              | 35 ++++++++++++++++++-----------------
 block/raw-format.c          |  2 +-
 block/throttle.c            |  2 +-
 block/vmdk.c                | 20 ++++++++++----------
 hw/9pfs/9p.h                |  9 ++++++---
 include/block/nbd.h         |  2 +-
 include/qemu/coroutine.h    |  2 +-
 include/qemu/job.h          |  2 +-
 job.c                       |  2 +-
 migration/migration.c       |  3 ++-
 tests/unit/test-coroutine.c |  2 +-
 util/qemu-coroutine-lock.c  | 14 +++++++-------
 util/qemu-coroutine.c       |  2 +-
 30 files changed, 128 insertions(+), 121 deletions(-)

-- 
2.35.1

[PATCH v2 01/26] block: remove incorrect coroutine_fn annotations

[Paolo Bonzini]

This is incorrect because blk_pwritev_part() is called by
blk_pwrite_zeroes() and blk_pwrite(), neither of which has to be called
from a coroutine.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/block-backend.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index e0e1aff4b1..fedf2eca83 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1391,10 +1391,10 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
     return blk_co_pwritev_part(blk, offset, bytes, qiov, 0, flags);
 }
 
-static int coroutine_fn blk_pwritev_part(BlockBackend *blk, int64_t offset,
-                                         int64_t bytes,
-                                         QEMUIOVector *qiov, size_t qiov_offset,
-                                         BdrvRequestFlags flags)
+static int blk_pwritev_part(BlockBackend *blk, int64_t offset,
+                            int64_t bytes,
+                            QEMUIOVector *qiov, size_t qiov_offset,
+                            BdrvRequestFlags flags)
 {
     int ret;
 
-- 
2.35.1

Re: [PATCH v2 01/26] block: remove incorrect coroutine_fn annotations

[Eric Blake]

On Mon, May 09, 2022 at 12:29:54PM +0200, Paolo Bonzini wrote:
> This is incorrect because blk_pwritev_part() is called by
> blk_pwrite_zeroes() and blk_pwrite(), neither of which has to be called
> from a coroutine.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/block-backend.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index e0e1aff4b1..fedf2eca83 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1391,10 +1391,10 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
>      return blk_co_pwritev_part(blk, offset, bytes, qiov, 0, flags);
>  }
>  
> -static int coroutine_fn blk_pwritev_part(BlockBackend *blk, int64_t offset,
> -                                         int64_t bytes,
> -                                         QEMUIOVector *qiov, size_t qiov_offset,
> -                                         BdrvRequestFlags flags)
> +static int blk_pwritev_part(BlockBackend *blk, int64_t offset,
> +                            int64_t bytes,
> +                            QEMUIOVector *qiov, size_t qiov_offset,
> +                            BdrvRequestFlags flags)
>  {
>      int ret;
>  
> -- 
> 2.35.1
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 01/26] block: remove incorrect coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:46 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> This is incorrect because blk_pwritev_part() is called by
> blk_pwrite_zeroes() and blk_pwrite(), neither of which has to be called
> from a coroutine.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/block-backend.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Isn't this sort of justification the wrong way around?

Regardless:

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 02/26] qcow2: remove incorrect coroutine_fn annotations

[Paolo Bonzini]

This is incorrect because qcow2_mark_clean() calls qcow2_flush_caches().
qcow2_mark_clean() is called from non-coroutine context in
qcow2_inactivate() and qcow2_close().

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/qcow2-refcount.c | 4 ++--
 block/qcow2.h          | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index ed0ecfaa89..404d56e258 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1206,7 +1206,7 @@ void qcow2_free_any_cluster(BlockDriverState *bs, uint64_t l2_entry,
     }
 }
 
-int coroutine_fn qcow2_write_caches(BlockDriverState *bs)
+int qcow2_write_caches(BlockDriverState *bs)
 {
     BDRVQcow2State *s = bs->opaque;
     int ret;
@@ -1226,7 +1226,7 @@ int coroutine_fn qcow2_write_caches(BlockDriverState *bs)
     return 0;
 }
 
-int coroutine_fn qcow2_flush_caches(BlockDriverState *bs)
+int qcow2_flush_caches(BlockDriverState *bs)
 {
     int ret = qcow2_write_caches(bs);
     if (ret < 0) {
diff --git a/block/qcow2.h b/block/qcow2.h
index ba436a8d0d..c8d9e8ea79 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -874,8 +874,8 @@ void qcow2_free_any_cluster(BlockDriverState *bs, uint64_t l2_entry,
 int qcow2_update_snapshot_refcount(BlockDriverState *bs,
     int64_t l1_table_offset, int l1_size, int addend);
 
-int coroutine_fn qcow2_flush_caches(BlockDriverState *bs);
-int coroutine_fn qcow2_write_caches(BlockDriverState *bs);
+int qcow2_flush_caches(BlockDriverState *bs);
+int qcow2_write_caches(BlockDriverState *bs);
 int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
                           BdrvCheckMode fix);
 
-- 
2.35.1

Re: [PATCH v2 02/26] qcow2: remove incorrect coroutine_fn annotations

[Eric Blake]

On Mon, May 09, 2022 at 12:29:55PM +0200, Paolo Bonzini wrote:
> This is incorrect because qcow2_mark_clean() calls qcow2_flush_caches().
> qcow2_mark_clean() is called from non-coroutine context in
> qcow2_inactivate() and qcow2_close().
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/qcow2-refcount.c | 4 ++--
>  block/qcow2.h          | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 02/26] qcow2: remove incorrect coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:35 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> This is incorrect because qcow2_mark_clean() calls qcow2_flush_caches().
> qcow2_mark_clean() is called from non-coroutine context in
> qcow2_inactivate() and qcow2_close().
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/qcow2-refcount.c | 4 ++--
>  block/qcow2.h          | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 03/26] nbd: remove incorrect coroutine_fn annotations

[Paolo Bonzini]

nbd_co_establish_connection_cancel() cancels a coroutine but is not called
from coroutine context itself, for example in nbd_cancel_in_flight()
and in timer callbacks reconnect_delay_timer_cb() and open_timer_cb().

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/block/nbd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index a98eb665da..5c3710fa52 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -423,6 +423,6 @@ QIOChannel *coroutine_fn
 nbd_co_establish_connection(NBDClientConnection *conn, NBDExportInfo *info,
                             bool blocking, Error **errp);
 
-void coroutine_fn nbd_co_establish_connection_cancel(NBDClientConnection *conn);
+void nbd_co_establish_connection_cancel(NBDClientConnection *conn);
 
 #endif
-- 
2.35.1

Re: [PATCH v2 03/26] nbd: remove incorrect coroutine_fn annotations

[Eric Blake]

On Mon, May 09, 2022 at 12:29:56PM +0200, Paolo Bonzini wrote:
> nbd_co_establish_connection_cancel() cancels a coroutine but is not called
> from coroutine context itself, for example in nbd_cancel_in_flight()
> and in timer callbacks reconnect_delay_timer_cb() and open_timer_cb().
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/block/nbd.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index a98eb665da..5c3710fa52 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -423,6 +423,6 @@ QIOChannel *coroutine_fn
>  nbd_co_establish_connection(NBDClientConnection *conn, NBDExportInfo *info,
>                              bool blocking, Error **errp);
>  
> -void coroutine_fn nbd_co_establish_connection_cancel(NBDClientConnection *conn);
> +void nbd_co_establish_connection_cancel(NBDClientConnection *conn);
>  
>  #endif
> -- 
> 2.35.1
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 03/26] nbd: remove incorrect coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:39 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> nbd_co_establish_connection_cancel() cancels a coroutine but is not called
> from coroutine context itself, for example in nbd_cancel_in_flight()
> and in timer callbacks reconnect_delay_timer_cb() and open_timer_cb().
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/block/nbd.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 04/26] coroutine: remove incorrect coroutine_fn annotations

[Paolo Bonzini]

qemu_coroutine_get_aio_context inspects a coroutine, but it does
not have to be called from the coroutine itself (or from any
coroutine).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/coroutine.h | 2 +-
 util/qemu-coroutine.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 284571badb..2d9211faff 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -92,7 +92,7 @@ void coroutine_fn qemu_coroutine_yield(void);
 /**
  * Get the AioContext of the given coroutine
  */
-AioContext *coroutine_fn qemu_coroutine_get_aio_context(Coroutine *co);
+AioContext *qemu_coroutine_get_aio_context(Coroutine *co);
 
 /**
  * Get the currently executing coroutine
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index f3e8300c8d..32c7ae8f21 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -207,7 +207,7 @@ bool qemu_coroutine_entered(Coroutine *co)
     return co->caller;
 }
 
-AioContext *coroutine_fn qemu_coroutine_get_aio_context(Coroutine *co)
+AioContext *qemu_coroutine_get_aio_context(Coroutine *co)
 {
     return co->ctx;
 }
-- 
2.35.1

Re: [PATCH v2 04/26] coroutine: remove incorrect coroutine_fn annotations

[Eric Blake]

On Mon, May 09, 2022 at 12:29:57PM +0200, Paolo Bonzini wrote:
> qemu_coroutine_get_aio_context inspects a coroutine, but it does
> not have to be called from the coroutine itself (or from any
> coroutine).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/coroutine.h | 2 +-
>  util/qemu-coroutine.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 04/26] coroutine: remove incorrect coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:35 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> qemu_coroutine_get_aio_context inspects a coroutine, but it does
> not have to be called from the coroutine itself (or from any
> coroutine).
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/coroutine.h | 2 +-
>  util/qemu-coroutine.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 05/26] blkdebug: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/blkdebug.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index bbf2948703..a93ba61487 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -587,8 +587,8 @@ out:
     return ret;
 }
 
-static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                      BlkdebugIOType iotype)
+static int coroutine_fn rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+                                   BlkdebugIOType iotype)
 {
     BDRVBlkdebugState *s = bs->opaque;
     BlkdebugRule *rule = NULL;
@@ -672,7 +672,7 @@ blkdebug_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
     return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
 }
 
-static int blkdebug_co_flush(BlockDriverState *bs)
+static int coroutine_fn blkdebug_co_flush(BlockDriverState *bs)
 {
     int err = rule_check(bs, 0, 0, BLKDEBUG_IO_TYPE_FLUSH);
 
@@ -791,7 +791,7 @@ static void blkdebug_close(BlockDriverState *bs)
 }
 
 /* Called with lock held.  */
-static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
+static void coroutine_fn suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
 {
     BDRVBlkdebugState *s = bs->opaque;
     BlkdebugSuspendedReq *r;
@@ -810,8 +810,8 @@ static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
 }
 
 /* Called with lock held.  */
-static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
-                         int *action_count, int *new_state)
+static void coroutine_fn process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
+                                      int *action_count, int *new_state)
 {
     BDRVBlkdebugState *s = bs->opaque;
 
@@ -840,7 +840,7 @@ static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
     }
 }
 
-static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
+static void coroutine_fn blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
 {
     BDRVBlkdebugState *s = bs->opaque;
     struct BlkdebugRule *rule, *next;
-- 
2.35.1

Re: [PATCH v2 05/26] blkdebug: add missing coroutine_fn annotations

[Eric Blake]

On Mon, May 09, 2022 at 12:29:58PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Sparse commit message; from the followup email, this sentence would be helpful:

| The only rule is that callers of coroutine_fn must be coroutine_fn themselves,
| or the call must be within "if (qemu_in_coroutine())".

> ---
>  block/blkdebug.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index bbf2948703..a93ba61487 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -587,8 +587,8 @@ out:
>      return ret;
>  }
>  
> -static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> -                      BlkdebugIOType iotype)
> +static int coroutine_fn rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
> +                                   BlkdebugIOType iotype)

Following that guideline, rule_check() is reached from:
+ coroutine_fn blkdebug_co_preadv
+ coroutine_fn blkdebug_co_pwritev
+ blkdebug_co_flush (fixed to be coroutine_fn below)
+ coroutine_fn blkdebug_co_pwrite_zeroes
+ coroutine_fn blkdebug_co_pdiscard
+ coroutine_fn blkdebug_co_block_status

>  {
>      BDRVBlkdebugState *s = bs->opaque;
>      BlkdebugRule *rule = NULL;
> @@ -672,7 +672,7 @@ blkdebug_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
>      return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
>  }
>  
> -static int blkdebug_co_flush(BlockDriverState *bs)
> +static int coroutine_fn blkdebug_co_flush(BlockDriverState *bs)

blkdebug_co_flush() is only reached from:
+ .bdrv_co_flush_to_disk
  + from io.c coroutine_fn bdrv_co_flush

>  {
>      int err = rule_check(bs, 0, 0, BLKDEBUG_IO_TYPE_FLUSH);
>  
> @@ -791,7 +791,7 @@ static void blkdebug_close(BlockDriverState *bs)
>  }
>  
>  /* Called with lock held.  */
> -static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
> +static void coroutine_fn suspend_request(BlockDriverState *bs, BlkdebugRule *rule)

suspend_request() is only reached from:
+ process_rule (fixed to be coroutine_fn below)

>  {
>      BDRVBlkdebugState *s = bs->opaque;
>      BlkdebugSuspendedReq *r;
> @@ -810,8 +810,8 @@ static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
>  }
>  
>  /* Called with lock held.  */
> -static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
> -                         int *action_count, int *new_state)
> +static void coroutine_fn process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
> +                                      int *action_count, int *new_state)

process_rule() is only reached from:
+ blkdebug_debug_event (fixed to be coroutine_fn below)

>  {
>      BDRVBlkdebugState *s = bs->opaque;
>  
> @@ -840,7 +840,7 @@ static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
>      }
>  }
>  
> -static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
> +static void coroutine_fn blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)

Long line; could wrap the arguments.

blkdebug_debug_event() is only reached from:
+ .bdrv_debug_event
  + from io.c coroutine_fn bdrv_do_co_copy_on_readv
  + from io.c bdrv_padding_rmw_read (NOT marked coroutine_fn, no qemu_in_coroutine guard!)
  | + from io.c coroutine_fn bdrv_co_do_zero_pwritev
  | + from io.c coroutine_fn bdrv_co_pwritev_part
  + from io.c coroutine_fn bdrv_aligned_pwritev

So the patch is 95% correct; if you also mark io.c
bdrv_padding_rmw_read(), you can add:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[PATCH v2 06/26] blkverify: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/blkverify.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blkverify.c b/block/blkverify.c
index e4a37af3b2..020b1ae7b6 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -258,7 +258,7 @@ blkverify_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
     return blkverify_co_prwv(bs, &r, offset, bytes, qiov, qiov, flags, true);
 }
 
-static int blkverify_co_flush(BlockDriverState *bs)
+static int coroutine_fn blkverify_co_flush(BlockDriverState *bs)
 {
     BDRVBlkverifyState *s = bs->opaque;
 
-- 
2.35.1

Re: [PATCH v2 06/26] blkverify: add missing coroutine_fn annotations

[Eric Blake]

On Mon, May 09, 2022 at 12:29:59PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/blkverify.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/blkverify.c b/block/blkverify.c
> index e4a37af3b2..020b1ae7b6 100644
> --- a/block/blkverify.c
> +++ b/block/blkverify.c
> @@ -258,7 +258,7 @@ blkverify_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
>      return blkverify_co_prwv(bs, &r, offset, bytes, qiov, qiov, flags, true);
>  }
>  
> -static int blkverify_co_flush(BlockDriverState *bs)
> +static int coroutine_fn blkverify_co_flush(BlockDriverState *bs)

Called by:
+ .bdrv_co_flush
  + from io.c coroutine_fn bdrv_co_flush

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 06/26] blkverify: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:46 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/blkverify.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 07/26] block: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/block-backend.c | 18 +++++++++---------
 block/io.c            | 24 ++++++++++++------------
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index fedf2eca83..52009b8949 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1413,8 +1413,8 @@ typedef struct BlkRwCo {
     BdrvRequestFlags flags;
 } BlkRwCo;
 
-int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
-                      int64_t bytes, BdrvRequestFlags flags)
+int coroutine_fn blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
+                                   int64_t bytes, BdrvRequestFlags flags)
 {
     IO_OR_GS_CODE();
     return blk_pwritev_part(blk, offset, bytes, NULL, 0,
@@ -1534,7 +1534,7 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset,
     return &acb->common;
 }
 
-static void blk_aio_read_entry(void *opaque)
+static void coroutine_fn blk_aio_read_entry(void *opaque)
 {
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
@@ -1546,7 +1546,7 @@ static void blk_aio_read_entry(void *opaque)
     blk_aio_complete(acb);
 }
 
-static void blk_aio_write_entry(void *opaque)
+static void coroutine_fn blk_aio_write_entry(void *opaque)
 {
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
@@ -1580,8 +1580,8 @@ int blk_pread(BlockBackend *blk, int64_t offset, void *buf, int bytes)
     return ret < 0 ? ret : bytes;
 }
 
-int blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int bytes,
-               BdrvRequestFlags flags)
+int coroutine_fn blk_pwrite(BlockBackend *blk, int64_t offset, const void *buf, int bytes,
+                            BdrvRequestFlags flags)
 {
     int ret;
     QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes);
@@ -1681,7 +1681,7 @@ int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
     return ret;
 }
 
-static void blk_aio_ioctl_entry(void *opaque)
+static void coroutine_fn blk_aio_ioctl_entry(void *opaque)
 {
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
@@ -1715,7 +1715,7 @@ blk_co_do_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes)
     return bdrv_co_pdiscard(blk->root, offset, bytes);
 }
 
-static void blk_aio_pdiscard_entry(void *opaque)
+static void coroutine_fn blk_aio_pdiscard_entry(void *opaque)
 {
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
@@ -1771,7 +1771,7 @@ int coroutine_fn blk_co_do_flush(BlockBackend *blk)
     return bdrv_co_flush(blk_bs(blk));
 }
 
-static void blk_aio_flush_entry(void *opaque)
+static void coroutine_fn blk_aio_flush_entry(void *opaque)
 {
     BlkAioEmAIOCB *acb = opaque;
     BlkRwCo *rwco = &acb->rwco;
diff --git a/block/io.c b/block/io.c
index 9769ec53b0..7db9be3c03 100644
--- a/block/io.c
+++ b/block/io.c
@@ -751,7 +751,7 @@ void bdrv_drain_all(void)
  *
  * This function should be called when a tracked request is completing.
  */
-static void tracked_request_end(BdrvTrackedRequest *req)
+static void coroutine_fn tracked_request_end(BdrvTrackedRequest *req)
 {
     if (req->serialising) {
         qatomic_dec(&req->bs->serialising_in_flight);
@@ -766,11 +766,11 @@ static void tracked_request_end(BdrvTrackedRequest *req)
 /**
  * Add an active request to the tracked requests list
  */
-static void tracked_request_begin(BdrvTrackedRequest *req,
-                                  BlockDriverState *bs,
-                                  int64_t offset,
-                                  int64_t bytes,
-                                  enum BdrvTrackedRequestType type)
+static void coroutine_fn tracked_request_begin(BdrvTrackedRequest *req,
+                                               BlockDriverState *bs,
+                                               int64_t offset,
+                                               int64_t bytes,
+                                               enum BdrvTrackedRequestType type)
 {
     bdrv_check_request(offset, bytes, &error_abort);
 
@@ -809,7 +809,7 @@ static bool tracked_request_overlaps(BdrvTrackedRequest *req,
 }
 
 /* Called with self->bs->reqs_lock held */
-static BdrvTrackedRequest *
+static coroutine_fn BdrvTrackedRequest *
 bdrv_find_conflicting_request(BdrvTrackedRequest *self)
 {
     BdrvTrackedRequest *req;
@@ -1704,10 +1704,10 @@ static bool bdrv_init_padding(BlockDriverState *bs,
     return true;
 }
 
-static int bdrv_padding_rmw_read(BdrvChild *child,
-                                 BdrvTrackedRequest *req,
-                                 BdrvRequestPadding *pad,
-                                 bool zero_middle)
+static coroutine_fn int bdrv_padding_rmw_read(BdrvChild *child,
+                                              BdrvTrackedRequest *req,
+                                              BdrvRequestPadding *pad,
+                                              bool zero_middle)
 {
     QEMUIOVector local_qiov;
     BlockDriverState *bs = child->bs;
@@ -3228,7 +3228,7 @@ out:
     return ret;
 }
 
-int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf)
+int coroutine_fn bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf)
 {
     BlockDriver *drv = bs->drv;
     CoroutineIOCompletion co = {
-- 
2.35.1

Re: [PATCH v2 07/26] block: add missing coroutine_fn annotations

[Eric Blake]

On Mon, May 09, 2022 at 12:30:00PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Resuming more of my review...

> ---
>  block/block-backend.c | 18 +++++++++---------
>  block/io.c            | 24 ++++++++++++------------
>  2 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/block/block-backend.c b/block/block-backend.c
> index fedf2eca83..52009b8949 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1413,8 +1413,8 @@ typedef struct BlkRwCo {
>      BdrvRequestFlags flags;
>  } BlkRwCo;
>  
> -int blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
> -                      int64_t bytes, BdrvRequestFlags flags)
> +int coroutine_fn blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
> +                                   int64_t bytes, BdrvRequestFlags flags)

Tracking down all callers of blk_pwrite_zeroes is not as trivial as in
the previous patches.  But the very first one I checked:
 block.c: create_file_fallback_zero_first_sector()

is neither marked coroutine_fn, nor does it have the "if
(qemu_in_coroutine())" guard.  And block.c is not touched in this
patch series, per the diffstat in 0/26.  Am I missing something?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 07/26] block: add missing coroutine_fn annotations

[Paolo Bonzini]

On 5/13/22 23:26, Eric Blake wrote:
>> +int coroutine_fn blk_pwrite_zeroes(BlockBackend *blk, int64_t offset,
>> +                                   int64_t bytes, BdrvRequestFlags flags)
> Tracking down all callers of blk_pwrite_zeroes is not as trivial as in
> the previous patches.  But the very first one I checked:
>   block.c: create_file_fallback_zero_first_sector()
> 
> is neither marked coroutine_fn, nor does it have the "if
> (qemu_in_coroutine())" guard.

That one in particular _should_ be coroutine_fn; 
bdrv_co_create_opts_simple is the only caller.  This is probably a 
limitation of the thread-safety annotations that I used to generate this 
patchset: the compiler does limited inter-procedural analysis which for 
us means missing some potential coroutine_fn additions.

But I agree that blk_pwrite_zeros shouldn't be a coroutine_fn, because 
of fuse_fallocate and block_load.  Alberto, this function is another 
candidate for adding a blk_co_pwrite_zeros + a matching 
generated_co_wrapper.

Paolo

>  And block.c is not touched in this
> patch series, per the diffstat in 0/26.  Am I missing something?

Re: [PATCH v2 07/26] block: add missing coroutine_fn annotations

[Alberto Faria]

On Sat, May 14, 2022 at 10:54 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> But I agree that blk_pwrite_zeros shouldn't be a coroutine_fn, because
> of fuse_fallocate and block_load.  Alberto, this function is another
> candidate for adding a blk_co_pwrite_zeros + a matching
> generated_co_wrapper.

Thanks for the heads up. In fact, looking at block-backend-io.h,
there's quite a few functions that (AFAICT) we could implement using
generated_co_wrapper:

    - blk_pread, blk_preadv, blk_preadv_part
    - blk_pwrite, blk_pwritev, blk_pwritev_part
    - blk_pwrite_compressed, blk_pwrite_zeroes
    - blk_pdiscard
    - blk_flush
    - blk_truncate, blk_ioctl

Would this make sense?

Re: [PATCH v2 07/26] block: add missing coroutine_fn annotations

[Paolo Bonzini]

On 5/16/22 20:49, Alberto Faria wrote:
> Thanks for the heads up. In fact, looking at block-backend-io.h,
> there's quite a few functions that (AFAICT) we could implement using
> generated_co_wrapper:
> 
>      - blk_pread, blk_preadv, blk_preadv_part
>      - blk_pwrite, blk_pwritev, blk_pwritev_part
>      - blk_pwrite_compressed, blk_pwrite_zeroes
>      - blk_pdiscard
>      - blk_flush
>      - blk_truncate, blk_ioctl
> 
> Would this make sense?

Yes, I think so.

Paolo

[PATCH v2 08/26] file-posix: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/file-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 48cd096624..76eea8d350 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2158,7 +2158,7 @@ static void raw_aio_unplug(BlockDriverState *bs)
 #endif
 }
 
-static int raw_co_flush_to_disk(BlockDriverState *bs)
+static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
     RawPosixAIOData acb;
-- 
2.35.1

Re: [PATCH v2 08/26] file-posix: add missing coroutine_fn annotations

[Eric Blake]

On Mon, May 09, 2022 at 12:30:01PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/file-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Sparse commit message.

> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 48cd096624..76eea8d350 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2158,7 +2158,7 @@ static void raw_aio_unplug(BlockDriverState *bs)
>  #endif
>  }
>  
> -static int raw_co_flush_to_disk(BlockDriverState *bs)
> +static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)

Only reached by:
+ .bdrv_co_flush_to_disk
  + io.c: coroutine_fn bdrv_co_flush

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 08/26] file-posix: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:54 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/file-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 09/26] iscsi: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/iscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index d707d0b354..b33eeec794 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -290,7 +290,7 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
     }
 }
 
-static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask)
+static void coroutine_fn iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask)
 {
     *iTask = (struct IscsiTask) {
         .co         = qemu_coroutine_self(),
-- 
2.35.1

Re: [PATCH v2 09/26] iscsi: add missing coroutine_fn annotations

[Eric Blake]

On Mon, May 09, 2022 at 12:30:02PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/iscsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index d707d0b354..b33eeec794 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -290,7 +290,7 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
>      }
>  }
>  
> -static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask)
> +static void coroutine_fn iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask)

Might be worth wrapping the long line.

All callers are already coroutine_fn (iscsi_co_writev,
iscsi_co_block_status, iscsi_co_readv, iscsi_co_flush,
iscsi_co_pdiscard, iscsi_co_pwrite_zeroes, iscsi_co_copy_range_to), so
safe.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 09/26] iscsi: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:39 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/iscsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 10/26] nbd: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/nbd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 6085ab1d2c..fe913a6db4 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -983,11 +983,11 @@ static void nbd_iter_request_error(NBDReplyChunkIter *iter, int ret)
  * nbd_reply_chunk_iter_receive
  * The pointer stored in @payload requires g_free() to free it.
  */
-static bool nbd_reply_chunk_iter_receive(BDRVNBDState *s,
-                                         NBDReplyChunkIter *iter,
-                                         uint64_t handle,
-                                         QEMUIOVector *qiov, NBDReply *reply,
-                                         void **payload)
+static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,
+                                                      NBDReplyChunkIter *iter,
+                                                      uint64_t handle,
+                                                      QEMUIOVector *qiov, NBDReply *reply,
+                                                      void **payload)
 {
     int ret, request_ret;
     NBDReply local_reply;
-- 
2.35.1

Re: [PATCH v2 10/26] nbd: add missing coroutine_fn annotations

[Eric Blake]

On Mon, May 09, 2022 at 12:30:03PM +0200, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/nbd.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index 6085ab1d2c..fe913a6db4 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -983,11 +983,11 @@ static void nbd_iter_request_error(NBDReplyChunkIter *iter, int ret)
>   * nbd_reply_chunk_iter_receive
>   * The pointer stored in @payload requires g_free() to free it.
>   */
> -static bool nbd_reply_chunk_iter_receive(BDRVNBDState *s,
> -                                         NBDReplyChunkIter *iter,
> -                                         uint64_t handle,
> -                                         QEMUIOVector *qiov, NBDReply *reply,
> -                                         void **payload)
> +static bool coroutine_fn nbd_reply_chunk_iter_receive(BDRVNBDState *s,
> +                                                      NBDReplyChunkIter *iter,
> +                                                      uint64_t handle,
> +                                                      QEMUIOVector *qiov, NBDReply *reply,

Might be worth wrapping the now-longer line.

> +                                                      void **payload)

Used only by the macro NBD_FOREACH_REPLY_CHUNK(), which in turn is
used by nbd_co_receive_return_code, nbd_co_receive_cmdread_reply,
nbd_co_receive_blockstatus_reply, which all got recently marked in
0c43c6fc.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v2 10/26] nbd: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:54 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/nbd.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 11/26] nfs: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/nfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/nfs.c b/block/nfs.c
index 444c40b458..596ebe98cb 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -223,7 +223,7 @@ static void nfs_process_write(void *arg)
     qemu_mutex_unlock(&client->mutex);
 }
 
-static void nfs_co_init_task(BlockDriverState *bs, NFSRPC *task)
+static void coroutine_fn nfs_co_init_task(BlockDriverState *bs, NFSRPC *task)
 {
     *task = (NFSRPC) {
         .co             = qemu_coroutine_self(),
-- 
2.35.1

Re: [PATCH v2 11/26] nfs: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:46 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/nfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 12/26] nvme: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/nvme.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 01fb28aa63..6519697e40 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -1234,8 +1234,9 @@ static inline bool nvme_qiov_aligned(BlockDriverState *bs,
     return true;
 }
 
-static int nvme_co_prw(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-                       QEMUIOVector *qiov, bool is_write, int flags)
+static coroutine_fn int nvme_co_prw(BlockDriverState *bs,
+				    uint64_t offset, uint64_t bytes,
+				    QEMUIOVector *qiov, bool is_write, int flags)
 {
     BDRVNVMeState *s = bs->opaque;
     int r;
-- 
2.35.1

Re: [PATCH v2 12/26] nvme: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:02 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/nvme.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 13/26] parallels: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/parallels.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 8879b7027a..bee2ff023d 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -165,8 +165,9 @@ static int64_t block_status(BDRVParallelsState *s, int64_t sector_num,
     return start_off;
 }
 
-static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
-                                 int nb_sectors, int *pnum)
+static coroutine_fn int64_t allocate_clusters(BlockDriverState *bs,
+					      int64_t sector_num,
+					      int nb_sectors, int *pnum)
 {
     int ret = 0;
     BDRVParallelsState *s = bs->opaque;
-- 
2.35.1

Re: [PATCH v2 13/26] parallels: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:46 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/parallels.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 14/26] qcow2: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/qcow2-cluster.c  | 18 +++++++++---------
 block/qcow2-refcount.c |  2 +-
 block/qcow2.c          |  4 ++--
 block/qcow2.h          | 14 +++++++-------
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 20a16ba6ee..37fc7b905a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -884,7 +884,7 @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
     return 0;
 }
 
-static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
+static int coroutine_fn perform_cow(BlockDriverState *bs, QCowL2Meta *m)
 {
     BDRVQcow2State *s = bs->opaque;
     Qcow2COWRegion *start = &m->cow_start;
@@ -1024,7 +1024,7 @@ fail:
     return ret;
 }
 
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
+int coroutine_fn qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
 {
     BDRVQcow2State *s = bs->opaque;
     int i, j = 0, l2_index, ret;
@@ -1397,8 +1397,8 @@ static int count_single_write_clusters(BlockDriverState *bs, int nb_clusters,
  *           information on cluster allocation may be invalid now. The caller
  *           must start over anyway, so consider *cur_bytes undefined.
  */
-static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
-    uint64_t *cur_bytes, QCowL2Meta **m)
+static int coroutine_fn handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
+                                            uint64_t *cur_bytes, QCowL2Meta **m)
 {
     BDRVQcow2State *s = bs->opaque;
     QCowL2Meta *old_alloc;
@@ -1772,9 +1772,9 @@ out:
  *
  * Return 0 on success and -errno in error cases
  */
-int qcow2_alloc_host_offset(BlockDriverState *bs, uint64_t offset,
-                            unsigned int *bytes, uint64_t *host_offset,
-                            QCowL2Meta **m)
+int coroutine_fn qcow2_alloc_host_offset(BlockDriverState *bs, uint64_t offset,
+                                         unsigned int *bytes, uint64_t *host_offset,
+                                         QCowL2Meta **m)
 {
     BDRVQcow2State *s = bs->opaque;
     uint64_t start, remaining;
@@ -2105,8 +2105,8 @@ out:
     return ret;
 }
 
-int qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
-                             uint64_t bytes, int flags)
+int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
+                                          uint64_t bytes, int flags)
 {
     BDRVQcow2State *s = bs->opaque;
     uint64_t end_offset = offset + bytes;
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 404d56e258..17be4425f2 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -3704,7 +3704,7 @@ int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size)
     return -EIO;
 }
 
-int qcow2_detect_metadata_preallocation(BlockDriverState *bs)
+int coroutine_fn qcow2_detect_metadata_preallocation(BlockDriverState *bs)
 {
     BDRVQcow2State *s = bs->opaque;
     int64_t i, end_cluster, cluster_count = 0, threshold;
diff --git a/block/qcow2.c b/block/qcow2.c
index 4f5e6440fb..62cb153987 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2438,7 +2438,7 @@ static bool merge_cow(uint64_t offset, unsigned bytes,
  * Return 1 if the COW regions read as zeroes, 0 if not, < 0 on error.
  * Note that returning 0 does not guarantee non-zero data.
  */
-static int is_zero_cow(BlockDriverState *bs, QCowL2Meta *m)
+static int coroutine_fn is_zero_cow(BlockDriverState *bs, QCowL2Meta *m)
 {
     /*
      * This check is designed for optimization shortcut so it must be
@@ -2456,7 +2456,7 @@ static int is_zero_cow(BlockDriverState *bs, QCowL2Meta *m)
                                 m->cow_end.nb_bytes);
 }
 
-static int handle_alloc_space(BlockDriverState *bs, QCowL2Meta *l2meta)
+static int coroutine_fn handle_alloc_space(BlockDriverState *bs, QCowL2Meta *l2meta)
 {
     BDRVQcow2State *s = bs->opaque;
     QCowL2Meta *m;
diff --git a/block/qcow2.h b/block/qcow2.h
index c8d9e8ea79..36495d9051 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -895,7 +895,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order,
                                 void *cb_opaque, Error **errp);
 int qcow2_shrink_reftable(BlockDriverState *bs);
 int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size);
-int qcow2_detect_metadata_preallocation(BlockDriverState *bs);
+int coroutine_fn qcow2_detect_metadata_preallocation(BlockDriverState *bs);
 
 /* qcow2-cluster.c functions */
 int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
@@ -908,9 +908,9 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
 int qcow2_get_host_offset(BlockDriverState *bs, uint64_t offset,
                           unsigned int *bytes, uint64_t *host_offset,
                           QCow2SubclusterType *subcluster_type);
-int qcow2_alloc_host_offset(BlockDriverState *bs, uint64_t offset,
-                            unsigned int *bytes, uint64_t *host_offset,
-                            QCowL2Meta **m);
+int coroutine_fn qcow2_alloc_host_offset(BlockDriverState *bs, uint64_t offset,
+                                         unsigned int *bytes, uint64_t *host_offset,
+                                         QCowL2Meta **m);
 int qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
                                           uint64_t offset,
                                           int compressed_size,
@@ -918,13 +918,13 @@ int qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
 void qcow2_parse_compressed_l2_entry(BlockDriverState *bs, uint64_t l2_entry,
                                      uint64_t *coffset, int *csize);
 
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
+int coroutine_fn qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
 void qcow2_alloc_cluster_abort(BlockDriverState *bs, QCowL2Meta *m);
 int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
                           uint64_t bytes, enum qcow2_discard_type type,
                           bool full_discard);
-int qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
-                             uint64_t bytes, int flags);
+int coroutine_fn qcow2_subcluster_zeroize(BlockDriverState *bs, uint64_t offset,
+                                          uint64_t bytes, int flags);
 
 int qcow2_expand_zero_clusters(BlockDriverState *bs,
                                BlockDriverAmendStatusCB *status_cb,
-- 
2.35.1

Re: [PATCH v2 14/26] qcow2: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:01 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/qcow2-cluster.c  | 18 +++++++++---------
>  block/qcow2-refcount.c |  2 +-
>  block/qcow2.c          |  4 ++--
>  block/qcow2.h          | 14 +++++++-------
>  4 files changed, 19 insertions(+), 19 deletions(-)

Some overly long lines.

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 15/26] copy-before-write: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/copy-before-write.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/copy-before-write.c b/block/copy-before-write.c
index a8a06fdc09..5ad9693b13 100644
--- a/block/copy-before-write.c
+++ b/block/copy-before-write.c
@@ -165,9 +165,9 @@ static int coroutine_fn cbw_co_flush(BlockDriverState *bs)
  * It's guaranteed that guest writes will not interact in the region until
  * cbw_snapshot_read_unlock() called.
  */
-static BlockReq *cbw_snapshot_read_lock(BlockDriverState *bs,
-                                        int64_t offset, int64_t bytes,
-                                        int64_t *pnum, BdrvChild **file)
+static coroutine_fn BlockReq *cbw_snapshot_read_lock(BlockDriverState *bs,
+                                                     int64_t offset, int64_t bytes,
+                                                     int64_t *pnum, BdrvChild **file)
 {
     BDRVCopyBeforeWriteState *s = bs->opaque;
     BlockReq *req = g_new(BlockReq, 1);
@@ -197,7 +197,7 @@ static BlockReq *cbw_snapshot_read_lock(BlockDriverState *bs,
     return req;
 }
 
-static void cbw_snapshot_read_unlock(BlockDriverState *bs, BlockReq *req)
+static coroutine_fn void cbw_snapshot_read_unlock(BlockDriverState *bs, BlockReq *req)
 {
     BDRVCopyBeforeWriteState *s = bs->opaque;
 
-- 
2.35.1

Re: [PATCH v2 15/26] copy-before-write: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:57 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/copy-before-write.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Some overly long lines.

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 16/26] curl: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/curl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index 1e0f609579..cba4c4cac7 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -855,7 +855,7 @@ out_noclean:
     return -EINVAL;
 }
 
-static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
+static void coroutine_fn curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
 {
     CURLState *state;
     int running;
-- 
2.35.1

Re: [PATCH v2 16/26] curl: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:08 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/curl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 17/26] qed: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/qed.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index f34d9a3ac1..208128d679 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -259,7 +259,7 @@ static CachedL2Table *qed_new_l2_table(BDRVQEDState *s)
     return l2_table;
 }
 
-static bool qed_plug_allocating_write_reqs(BDRVQEDState *s)
+static bool coroutine_fn qed_plug_allocating_write_reqs(BDRVQEDState *s)
 {
     qemu_co_mutex_lock(&s->table_lock);
 
@@ -278,7 +278,7 @@ static bool qed_plug_allocating_write_reqs(BDRVQEDState *s)
     return true;
 }
 
-static void qed_unplug_allocating_write_reqs(BDRVQEDState *s)
+static void coroutine_fn qed_unplug_allocating_write_reqs(BDRVQEDState *s)
 {
     qemu_co_mutex_lock(&s->table_lock);
     assert(s->allocating_write_reqs_plugged);
-- 
2.35.1

Re: [PATCH v2 17/26] qed: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:05 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/qed.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 18/26] quorum: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/quorum.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index f33f30d36b..5ff69d7443 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -161,11 +161,10 @@ static bool quorum_64bits_compare(QuorumVoteValue *a, QuorumVoteValue *b)
     return a->l == b->l;
 }
 
-static QuorumAIOCB *quorum_aio_get(BlockDriverState *bs,
-                                   QEMUIOVector *qiov,
-                                   uint64_t offset,
-                                   uint64_t bytes,
-                                   int flags)
+static QuorumAIOCB *coroutine_fn quorum_aio_get(BlockDriverState *bs,
+                                                QEMUIOVector *qiov,
+                                                uint64_t offset, uint64_t bytes,
+                                                int flags)
 {
     BDRVQuorumState *s = bs->opaque;
     QuorumAIOCB *acb = g_new(QuorumAIOCB, 1);
@@ -273,7 +272,7 @@ static void quorum_report_bad_versions(BDRVQuorumState *s,
     }
 }
 
-static void quorum_rewrite_entry(void *opaque)
+static void coroutine_fn quorum_rewrite_entry(void *opaque)
 {
     QuorumCo *co = opaque;
     QuorumAIOCB *acb = co->acb;
@@ -574,7 +573,7 @@ free_exit:
     quorum_free_vote_list(&acb->votes);
 }
 
-static void read_quorum_children_entry(void *opaque)
+static void coroutine_fn read_quorum_children_entry(void *opaque)
 {
     QuorumCo *co = opaque;
     QuorumAIOCB *acb = co->acb;
@@ -602,7 +601,7 @@ static void read_quorum_children_entry(void *opaque)
     }
 }
 
-static int read_quorum_children(QuorumAIOCB *acb)
+static int coroutine_fn read_quorum_children(QuorumAIOCB *acb)
 {
     BDRVQuorumState *s = acb->bs->opaque;
     int i;
@@ -643,7 +642,7 @@ static int read_quorum_children(QuorumAIOCB *acb)
     return acb->vote_ret;
 }
 
-static int read_fifo_child(QuorumAIOCB *acb)
+static int coroutine_fn read_fifo_child(QuorumAIOCB *acb)
 {
     BDRVQuorumState *s = acb->bs->opaque;
     int n, ret;
@@ -664,8 +663,9 @@ static int read_fifo_child(QuorumAIOCB *acb)
     return ret;
 }
 
-static int quorum_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
-                            QEMUIOVector *qiov, BdrvRequestFlags flags)
+static int coroutine_fn quorum_co_preadv(BlockDriverState *bs,
+                                         int64_t offset, int64_t bytes,
+                                         QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     BDRVQuorumState *s = bs->opaque;
     QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
@@ -684,7 +684,7 @@ static int quorum_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
     return ret;
 }
 
-static void write_quorum_entry(void *opaque)
+static void coroutine_fn write_quorum_entry(void *opaque)
 {
     QuorumCo *co = opaque;
     QuorumAIOCB *acb = co->acb;
@@ -715,9 +715,9 @@ static void write_quorum_entry(void *opaque)
     }
 }
 
-static int quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
-                             int64_t bytes, QEMUIOVector *qiov,
-                             BdrvRequestFlags flags)
+static int coroutine_fn quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
+                                          int64_t bytes, QEMUIOVector *qiov,
+                                          BdrvRequestFlags flags)
 {
     BDRVQuorumState *s = bs->opaque;
     QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
@@ -746,8 +746,9 @@ static int quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
     return ret;
 }
 
-static int quorum_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
-                                   int64_t bytes, BdrvRequestFlags flags)
+static int coroutine_fn quorum_co_pwrite_zeroes(BlockDriverState *bs,
+                                                int64_t offset, int64_t bytes,
+                                                BdrvRequestFlags flags)
 
 {
     return quorum_co_pwritev(bs, offset, bytes, NULL,
-- 
2.35.1

Re: [PATCH v2 18/26] quorum: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 11:54 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/quorum.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index f33f30d36b..5ff69d7443 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -161,11 +161,10 @@ static bool quorum_64bits_compare(QuorumVoteValue *a, QuorumVoteValue *b)
>      return a->l == b->l;
>  }
>
> -static QuorumAIOCB *quorum_aio_get(BlockDriverState *bs,
> -                                   QEMUIOVector *qiov,
> -                                   uint64_t offset,
> -                                   uint64_t bytes,
> -                                   int flags)
> +static QuorumAIOCB *coroutine_fn quorum_aio_get(BlockDriverState *bs,
> +                                                QEMUIOVector *qiov,
> +                                                uint64_t offset, uint64_t bytes,
> +                                                int flags)
>  {
>      BDRVQuorumState *s = bs->opaque;
>      QuorumAIOCB *acb = g_new(QuorumAIOCB, 1);
> @@ -273,7 +272,7 @@ static void quorum_report_bad_versions(BDRVQuorumState *s,
>      }
>  }
>
> -static void quorum_rewrite_entry(void *opaque)
> +static void coroutine_fn quorum_rewrite_entry(void *opaque)
>  {
>      QuorumCo *co = opaque;
>      QuorumAIOCB *acb = co->acb;
> @@ -574,7 +573,7 @@ free_exit:
>      quorum_free_vote_list(&acb->votes);
>  }
>
> -static void read_quorum_children_entry(void *opaque)
> +static void coroutine_fn read_quorum_children_entry(void *opaque)
>  {
>      QuorumCo *co = opaque;
>      QuorumAIOCB *acb = co->acb;
> @@ -602,7 +601,7 @@ static void read_quorum_children_entry(void *opaque)
>      }
>  }
>
> -static int read_quorum_children(QuorumAIOCB *acb)
> +static int coroutine_fn read_quorum_children(QuorumAIOCB *acb)
>  {
>      BDRVQuorumState *s = acb->bs->opaque;
>      int i;
> @@ -643,7 +642,7 @@ static int read_quorum_children(QuorumAIOCB *acb)
>      return acb->vote_ret;
>  }
>
> -static int read_fifo_child(QuorumAIOCB *acb)
> +static int coroutine_fn read_fifo_child(QuorumAIOCB *acb)

There's a prototype further above that may be worth marking as coroutine_fn too.

>  {
>      BDRVQuorumState *s = acb->bs->opaque;
>      int n, ret;
> @@ -664,8 +663,9 @@ static int read_fifo_child(QuorumAIOCB *acb)
>      return ret;
>  }
>
> -static int quorum_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
> -                            QEMUIOVector *qiov, BdrvRequestFlags flags)
> +static int coroutine_fn quorum_co_preadv(BlockDriverState *bs,
> +                                         int64_t offset, int64_t bytes,
> +                                         QEMUIOVector *qiov, BdrvRequestFlags flags)

Overly long line.

>  {
>      BDRVQuorumState *s = bs->opaque;
>      QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
> @@ -684,7 +684,7 @@ static int quorum_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
>      return ret;
>  }
>
> -static void write_quorum_entry(void *opaque)
> +static void coroutine_fn write_quorum_entry(void *opaque)
>  {
>      QuorumCo *co = opaque;
>      QuorumAIOCB *acb = co->acb;
> @@ -715,9 +715,9 @@ static void write_quorum_entry(void *opaque)
>      }
>  }
>
> -static int quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
> -                             int64_t bytes, QEMUIOVector *qiov,
> -                             BdrvRequestFlags flags)
> +static int coroutine_fn quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
> +                                          int64_t bytes, QEMUIOVector *qiov,
> +                                          BdrvRequestFlags flags)
>  {
>      BDRVQuorumState *s = bs->opaque;
>      QuorumAIOCB *acb = quorum_aio_get(bs, qiov, offset, bytes, flags);
> @@ -746,8 +746,9 @@ static int quorum_co_pwritev(BlockDriverState *bs, int64_t offset,
>      return ret;
>  }
>
> -static int quorum_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
> -                                   int64_t bytes, BdrvRequestFlags flags)
> +static int coroutine_fn quorum_co_pwrite_zeroes(BlockDriverState *bs,
> +                                                int64_t offset, int64_t bytes,
> +                                                BdrvRequestFlags flags)
>
>  {
>      return quorum_co_pwritev(bs, offset, bytes, NULL,
> --
> 2.35.1

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 19/26] throttle: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/throttle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/throttle.c b/block/throttle.c
index 6e8d52fa24..ddd450593a 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -162,7 +162,7 @@ static int coroutine_fn throttle_co_pwritev_compressed(BlockDriverState *bs,
                                BDRV_REQ_WRITE_COMPRESSED);
 }
 
-static int throttle_co_flush(BlockDriverState *bs)
+static int coroutine_fn throttle_co_flush(BlockDriverState *bs)
 {
     return bdrv_co_flush(bs->file->bs);
 }
-- 
2.35.1

Re: [PATCH v2 19/26] throttle: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:13 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/throttle.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 20/26] vmdk: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/vmdk.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 38e5ab3806..2c7f1858f9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1793,10 +1793,10 @@ static int coroutine_fn vmdk_co_block_status(BlockDriverState *bs,
     return ret;
 }
 
-static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
-                            int64_t offset_in_cluster, QEMUIOVector *qiov,
-                            uint64_t qiov_offset, uint64_t n_bytes,
-                            uint64_t offset)
+static int coroutine_fn vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
+                                         int64_t offset_in_cluster, QEMUIOVector *qiov,
+					  uint64_t qiov_offset, uint64_t n_bytes,
+					  uint64_t offset)
 {
     int ret;
     VmdkGrainMarker *data = NULL;
@@ -1874,9 +1874,9 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
     return ret;
 }
 
-static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
-                            int64_t offset_in_cluster, QEMUIOVector *qiov,
-                            int bytes)
+static int coroutine_fn vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
+                                        int64_t offset_in_cluster, QEMUIOVector *qiov,
+					 int bytes)
 {
     int ret;
     int cluster_bytes, buf_bytes;
@@ -2023,9 +2023,9 @@ fail:
  *
  * Returns: error code with 0 for success.
  */
-static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
-                       uint64_t bytes, QEMUIOVector *qiov,
-                       bool zeroed, bool zero_dry_run)
+static int coroutine_fn vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
+				     uint64_t bytes, QEMUIOVector *qiov,
+				     bool zeroed, bool zero_dry_run)
 {
     BDRVVmdkState *s = bs->opaque;
     VmdkExtent *extent = NULL;
-- 
2.35.1

Re: [PATCH v2 20/26] vmdk: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:18 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/vmdk.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Some overly long lines.

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 21/26] job: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/job.h | 2 +-
 job.c              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qemu/job.h b/include/qemu/job.h
index c105b31076..397ac39608 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -436,7 +436,7 @@ void coroutine_fn job_pause_point(Job *job);
  *
  * Yield the job coroutine.
  */
-void job_yield(Job *job);
+void coroutine_fn job_yield(Job *job);
 
 /**
  * @job: The job that calls the function.
diff --git a/job.c b/job.c
index 075c6f3a20..20f0d8b2cd 100644
--- a/job.c
+++ b/job.c
@@ -525,7 +525,7 @@ void coroutine_fn job_pause_point(Job *job)
     }
 }
 
-void job_yield(Job *job)
+void coroutine_fn job_yield(Job *job)
 {
     assert(job->busy);
 
-- 
2.35.1

Re: [PATCH v2 21/26] job: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:02 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/job.h | 2 +-
>  job.c              | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 22/26] coroutine-lock: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/qemu-coroutine-lock.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index 2669403839..ec55490b52 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -144,7 +144,7 @@ typedef struct CoWaitRecord {
     QSLIST_ENTRY(CoWaitRecord) next;
 } CoWaitRecord;
 
-static void push_waiter(CoMutex *mutex, CoWaitRecord *w)
+static void coroutine_fn push_waiter(CoMutex *mutex, CoWaitRecord *w)
 {
     w->co = qemu_coroutine_self();
     QSLIST_INSERT_HEAD_ATOMIC(&mutex->from_push, w, next);
@@ -341,7 +341,7 @@ void qemu_co_rwlock_init(CoRwlock *lock)
 }
 
 /* Releases the internal CoMutex.  */
-static void qemu_co_rwlock_maybe_wake_one(CoRwlock *lock)
+static void coroutine_fn qemu_co_rwlock_maybe_wake_one(CoRwlock *lock)
 {
     CoRwTicket *tkt = QSIMPLEQ_FIRST(&lock->tickets);
     Coroutine *co = NULL;
@@ -374,7 +374,7 @@ static void qemu_co_rwlock_maybe_wake_one(CoRwlock *lock)
     }
 }
 
-void qemu_co_rwlock_rdlock(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_rdlock(CoRwlock *lock)
 {
     Coroutine *self = qemu_coroutine_self();
 
@@ -399,7 +399,7 @@ void qemu_co_rwlock_rdlock(CoRwlock *lock)
     self->locks_held++;
 }
 
-void qemu_co_rwlock_unlock(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_unlock(CoRwlock *lock)
 {
     Coroutine *self = qemu_coroutine_self();
 
@@ -417,7 +417,7 @@ void qemu_co_rwlock_unlock(CoRwlock *lock)
     qemu_co_rwlock_maybe_wake_one(lock);
 }
 
-void qemu_co_rwlock_downgrade(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_downgrade(CoRwlock *lock)
 {
     qemu_co_mutex_lock(&lock->mutex);
     assert(lock->owners == -1);
@@ -427,7 +427,7 @@ void qemu_co_rwlock_downgrade(CoRwlock *lock)
     qemu_co_rwlock_maybe_wake_one(lock);
 }
 
-void qemu_co_rwlock_wrlock(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_wrlock(CoRwlock *lock)
 {
     Coroutine *self = qemu_coroutine_self();
 
@@ -447,7 +447,7 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock)
     self->locks_held++;
 }
 
-void qemu_co_rwlock_upgrade(CoRwlock *lock)
+void coroutine_fn qemu_co_rwlock_upgrade(CoRwlock *lock)
 {
     qemu_co_mutex_lock(&lock->mutex);
     assert(lock->owners > 0);
-- 
2.35.1

Re: [PATCH v2 22/26] coroutine-lock: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:13 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  util/qemu-coroutine-lock.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 23/26] raw-format: add missing coroutine_fn annotations

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/raw-format.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/raw-format.c b/block/raw-format.c
index 69fd650eaf..45440345b6 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -411,7 +411,7 @@ static void raw_lock_medium(BlockDriverState *bs, bool locked)
     bdrv_lock_medium(bs->file->bs, locked);
 }
 
-static int raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
+static int coroutine_fn raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
 {
     BDRVRawState *s = bs->opaque;
     if (s->offset || s->has_size) {
-- 
2.35.1

Re: [PATCH v2 23/26] raw-format: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:08 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/raw-format.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/raw-format.c b/block/raw-format.c
> index 69fd650eaf..45440345b6 100644
> --- a/block/raw-format.c
> +++ b/block/raw-format.c
> @@ -411,7 +411,7 @@ static void raw_lock_medium(BlockDriverState *bs, bool locked)
>      bdrv_lock_medium(bs->file->bs, locked);
>  }
>
> -static int raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
> +static int coroutine_fn raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)

Overly long line.

>  {
>      BDRVRawState *s = bs->opaque;
>      if (s->offset || s->has_size) {
> --
> 2.35.1

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 24/26] 9p: add missing coroutine_fn annotations

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/9pfs/9p.h | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 994f952600..a523ac34a9 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -424,21 +424,24 @@ typedef struct V9fsGetlock
 extern int open_fd_hw;
 extern int total_open_fd;
 
-static inline void v9fs_path_write_lock(V9fsState *s)
+static inline void coroutine_fn
+v9fs_path_write_lock(V9fsState *s)
 {
     if (s->ctx.export_flags & V9FS_PATHNAME_FSCONTEXT) {
         qemu_co_rwlock_wrlock(&s->rename_lock);
     }
 }
 
-static inline void v9fs_path_read_lock(V9fsState *s)
+static inline void coroutine_fn
+v9fs_path_read_lock(V9fsState *s)
 {
     if (s->ctx.export_flags & V9FS_PATHNAME_FSCONTEXT) {
         qemu_co_rwlock_rdlock(&s->rename_lock);
     }
 }
 
-static inline void v9fs_path_unlock(V9fsState *s)
+static inline void coroutine_fn
+v9fs_path_unlock(V9fsState *s)
 {
     if (s->ctx.export_flags & V9FS_PATHNAME_FSCONTEXT) {
         qemu_co_rwlock_unlock(&s->rename_lock);
-- 
2.35.1

Re: [PATCH v2 24/26] 9p: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:09 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Acked-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/9pfs/9p.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 25/26] migration: add missing coroutine_fn annotations

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 migration/migration.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 5a31b23bd6..e4ccfb9496 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -565,7 +565,8 @@ static void process_incoming_migration_bh(void *opaque)
     migration_incoming_state_destroy();
 }
 
-static void process_incoming_migration_co(void *opaque)
+static void coroutine_fn
+process_incoming_migration_co(void *opaque)
 {
     MigrationIncomingState *mis = migration_incoming_get_current();
     PostcopyState ps;
-- 
2.35.1

Re: [PATCH v2 25/26] migration: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:13 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  migration/migration.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

[PATCH v2 26/26] test-coroutine: add missing coroutine_fn annotations

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20170704220346.29244-4-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/unit/test-coroutine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/test-coroutine.c b/tests/unit/test-coroutine.c
index aa77a3bcb3..e16b80c245 100644
--- a/tests/unit/test-coroutine.c
+++ b/tests/unit/test-coroutine.c
@@ -610,7 +610,7 @@ static void perf_baseline(void)
     g_test_message("Function call %u iterations: %f s", maxcycles, duration);
 }
 
-static __attribute__((noinline)) void perf_cost_func(void *opaque)
+static __attribute__((noinline)) void coroutine_fn perf_cost_func(void *opaque)
 {
     qemu_coroutine_yield();
 }
-- 
2.35.1

Re: [PATCH v2 26/26] test-coroutine: add missing coroutine_fn annotations

[Alberto Faria]

On Mon, May 9, 2022 at 12:08 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Message-Id: <20170704220346.29244-4-marcandre.lureau@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/unit/test-coroutine.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Alberto Faria <afaria@redhat.com>

Re: [PATCH v2 00/26] block: fix coroutine_fn annotations

[Daniel P. Berrangé]

On Mon, May 09, 2022 at 12:29:53PM +0200, Paolo Bonzini wrote:
> This is the initial result of reviving Marc-André's series at
> https://patchew.org/QEMU/20170704220346.29244-1-marcandre.lureau@redhat.com/.
> A lot of the patches are similar to the ones that Marc-André wrote,
> but due to the changes in the code it was easier to redo them.
> 
> For nbd, the patch is on top of "nbd: mark more coroutine_fns" that
> I sent a few days ago and that (AIUI) Eric has already queued; only
> one function was missing, much to my surprise.
> 
> Apart from this, I also identified the following functions that
> can be called both in coroutine context and outside:

snip

> - qio_channel_readv_full_all_eof
> - qio_channel_writev_full_all

I'm trying to understand what criteria makes those two functions
liable for the annotation, but not others in the I/O code ?

What is the actual rule for when to apply 'coroutine_fn' annotation
to a function, and does it apply transitively to up and/or down the
call stack ? 


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 00/26] block: fix coroutine_fn annotations

[Paolo Bonzini]

On 5/9/22 13:53, Daniel P. Berrangé wrote:
>> Apart from this, I also identified the following functions that
>> can be called both in coroutine context and outside:
> snip
> 
>> - qio_channel_readv_full_all_eof
>> - qio_channel_writev_full_all
>
> I'm trying to understand what criteria makes those two functions
> liable for the annotation, but not others in the I/O code ?

These two are _not_ annotated as coroutine_fn: this is a list of 
functions that can be called both in coroutine context and outside, 
while 'coroutine_fn' functions can be called only within coroutines.

The only 'coroutine_fn' function in the I/O code is qio_channel_yield.

> What is the actual rule for when to apply 'coroutine_fn' annotation
> to a function, and does it apply transitively to up and/or down the
> call stack ?

The only rule is that callers of coroutine_fn must be coroutine_fn 
themselves, or the call must be within "if (qemu_in_coroutine())".  For 
example:

- qio_channel_readv_full_all_eof() calls qio_channel_yield() within such 
an "if", therefore it need not be coroutine_fn.

- qio_channel_yield() unconditionally calls qemu_coroutine_yield() which 
is coroutine_fn, and therefore must be coroutine_fn as well.

After this series, the only exception to the rule is that 
qemu_coroutine_self() is occasionally called from tracepoints.

Paolo