[Qemu-devel] [PATCH 00/35] RFC: coroutine annotations & clang check

[Qemu-devel] [PATCH 00/35] RFC: coroutine annotations & clang check

[Marc-André Lureau]

Hi,

After investigating a bit using clang-tidy to do some coroutine checks
(and hitting a wall as there are no pre-processor info in the AST), it
was suggested to me on the clang mailing list to try to use
-Wthread-safety. I had to modify clang a bit to make it work on qemu
code base (annotations on function typedef etc,
https://github.com/elmarco/clang qemu-ta branch - very hackish state).

The analysis simply checks that coroutine_fn are called from a
coroutine "context" (or "role"). I couldn't find any misuse in qemu
code base, however, a number of coroutine_fn annotations are missing.

(I think it would make sense to squash all the "mark coroutine_fn"
commits if we apply them, I tried to split them by domains/maintainer
to ease review)

Marc-André Lureau (35):
  WIP: coroutine: annotate coroutine with clang thread safety attributes
  WIP: coroutine: manually tag the fast-paths
  test-coroutine: fix coroutine attribute
  coroutine: remove coroutine_fn from qemu_coroutine_self()
  coroutine: remove coroutine_fn from qemu_co_queue_run_restart()
  coroutine: mark CoRwLock coroutine_fn
  blockjob: mark coroutine_fn
  block: all bdrv_aio callbacks are coroutine_fn
  block: bdrv_create() and bdrv_debug_event() are coroutine_fn
  vmdk: mark coroutine_fn
  qcow2: mark coroutine_fn
  raw: mark coroutine_fn
  nbd: mark coroutine_fn
  migration: mark coroutine_fn
  backup: mark coroutine_fn
  crypto: mark coroutine_fn
  curl: mark coroutine_fn
  gluster: mark coroutine_fn
  nfs: mark coroutine_fn
  quorum: mark coroutine_fn
  rbd: mark coroutine_fn
  sheepdog: mark coroutine_fn
  ssh: mark coroutine_fn
  null: mark coroutine_fn
  mirror: mark coroutine_fn
  iscsi: mark coroutine_fn
  file-posix: mark coroutine_fn
  9p: mark coroutine_fn
  block: mark coroutine_fn
  block-backend: mark coroutine_fn
  parallels: mark coroutine_fn
  qed: mark coroutine_fn
  vdi: mark coroutine_fn
  vhdx: mark coroutine_fn
  vpc: mark coroutine_fn

 block/nbd-client.h             | 10 +++++-----
 block/qcow2.h                  |  6 ++++--
 hw/9pfs/9p.h                   |  9 ++++++---
 include/block/block_backup.h   |  4 ++--
 include/block/block_int.h      | 14 +++++++-------
 include/block/blockjob_int.h   |  4 ++--
 include/qemu/coroutine.h       | 39 ++++++++++++++++++++++++++++++++++-----
 include/qemu/coroutine_int.h   |  2 +-
 include/sysemu/block-backend.h |  4 ++--
 block.c                        |  2 ++
 block/backup.c                 |  9 ++++++---
 block/blkdebug.c               | 15 ++++++++++-----
 block/blkverify.c              |  3 ++-
 block/block-backend.c          | 38 ++++++++++++++++++++++++++------------
 block/crypto.c                 |  3 ++-
 block/curl.c                   |  3 ++-
 block/file-posix.c             | 15 ++++++++++-----
 block/gluster.c                |  3 ++-
 block/io.c                     | 25 +++++++++++++++++++++----
 block/iscsi.c                  |  6 ++++--
 block/mirror.c                 | 15 ++++++++++-----
 block/nbd-client.c             | 24 ++++++++++++++++--------
 block/nbd.c                    |  3 ++-
 block/nfs.c                    |  3 ++-
 block/null.c                   |  9 ++++++---
 block/parallels.c              |  3 ++-
 block/qcow.c                   |  4 +++-
 block/qcow2-cluster.c          | 11 +++++++----
 block/qcow2.c                  | 15 ++++++++++-----
 block/qed.c                    |  3 ++-
 block/quorum.c                 | 25 ++++++++++++++++---------
 block/raw-format.c             |  6 ++++--
 block/rbd.c                    | 15 ++++++++++-----
 block/sheepdog.c               | 20 ++++++++++++++------
 block/ssh.c                    |  6 ++++--
 block/throttle-groups.c        | 10 ++++++++--
 block/vdi.c                    |  3 ++-
 block/vhdx.c                   |  3 ++-
 block/vmdk.c                   | 12 ++++++++----
 block/vpc.c                    |  3 ++-
 blockjob.c                     |  6 ++++--
 migration/migration.c          |  3 ++-
 migration/rdma.c               |  2 ++
 nbd/server.c                   |  3 ++-
 tests/test-coroutine.c         |  2 +-
 util/coroutine-sigaltstack.c   |  2 ++
 util/coroutine-ucontext.c      |  2 ++
 util/coroutine-win32.c         |  2 ++
 util/qemu-coroutine.c          |  2 ++
 49 files changed, 299 insertions(+), 132 deletions(-)

-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 01/35] WIP: coroutine: annotate coroutine with clang thread safety attributes

[Marc-André Lureau]

It is possible to use clang -Wthread-safety to do some basic coroutine
checks:
http://lists.llvm.org/pipermail/cfe-dev/2017-June/054372.html
https://clang.llvm.org/docs/ThreadSafetyAnalysis.html

This will basically check that you don't call accidentally a coroutine
function from a non-coroutine, as this may crash at run time if the
coroutine function yields.

I had to modify clang to support annotations on typedef and function
pointers, and check some function assignments/arguments. The end
result is quire far from ready for upstream review, but could serve as
basis for more checks or work. (https://github.com/elmarco/clang
qemu-ta branch)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/coroutine.h     | 31 ++++++++++++++++++++++++++++++-
 util/coroutine-sigaltstack.c |  2 ++
 util/coroutine-ucontext.c    |  2 ++
 util/coroutine-win32.c       |  2 ++
 util/qemu-coroutine.c        |  2 ++
 5 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index a4509bd977..35ff394f51 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -28,6 +28,34 @@
  * These functions are re-entrant and may be used outside the global mutex.
  */
 
+/* clang thread-safety attributes, used for static analysis of the CFG */
+#if defined(__clang__) && (!defined(SWIG))
+#define THREAD_ANNOTATION_ATTRIBUTE__(x)   __attribute__((x))
+#else
+#define THREAD_ANNOTATION_ATTRIBUTE__(x)
+#endif
+
+#define TAA_ROLE                                        \
+    THREAD_ANNOTATION_ATTRIBUTE__(capability("role"))
+
+#define TAA_REQUIRES(...)                                               \
+    THREAD_ANNOTATION_ATTRIBUTE__(requires_capability(__VA_ARGS__))
+
+#define TAA_ACQUIRE(R)                                          \
+    THREAD_ANNOTATION_ATTRIBUTE__(acquire_capability(R))
+
+#define TAA_RELEASE(R)                                          \
+    THREAD_ANNOTATION_ATTRIBUTE__(release_capability(R))
+
+#define TAA_NO_ANALYSYS                                         \
+    THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis)
+
+typedef int TAA_ROLE coroutine_role;
+extern coroutine_role _coroutine_fn;
+
+static inline void co_role_acquire(coroutine_role R) TAA_ACQUIRE(R) TAA_NO_ANALYSYS {}
+static inline void co_role_release(coroutine_role R) TAA_RELEASE(R) TAA_NO_ANALYSYS {}
+
 /**
  * Mark a function that executes in coroutine context
  *
@@ -42,7 +70,8 @@
  *       ....
  *   }
  */
-#define coroutine_fn
+
+#define coroutine_fn TAA_REQUIRES(_coroutine_fn)
 
 typedef struct Coroutine Coroutine;
 
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index f6fc49a0e5..05d1a378d1 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -98,7 +98,9 @@ static void coroutine_bootstrap(CoroutineSigAltStack *self, Coroutine *co)
     }
 
     while (true) {
+        co_role_acquire(_coroutine_fn);
         co->entry(co->entry_arg);
+        co_role_release(_coroutine_fn);
         qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
     }
 }
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 6621f3f692..010fbaedf1 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -76,7 +76,9 @@ static void coroutine_trampoline(int i0, int i1)
     }
 
     while (true) {
+        co_role_acquire(_coroutine_fn);
         co->entry(co->entry_arg);
+        co_role_release(_coroutine_fn);
         qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
     }
 }
diff --git a/util/coroutine-win32.c b/util/coroutine-win32.c
index de6bd4fd3e..75a3bed543 100644
--- a/util/coroutine-win32.c
+++ b/util/coroutine-win32.c
@@ -64,7 +64,9 @@ static void CALLBACK coroutine_trampoline(void *co_)
     Coroutine *co = co_;
 
     while (true) {
+        co_role_acquire(_coroutine_fn);
         co->entry(co->entry_arg);
+        co_role_release(_coroutine_fn);
         qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
     }
 }
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index d6095c1d5a..efa0f20e69 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -25,6 +25,8 @@ enum {
     POOL_BATCH_SIZE = 64,
 };
 
+coroutine_role _coroutine_fn;
+
 /** Free list to speed up creation */
 static QSLIST_HEAD(, Coroutine) release_pool = QSLIST_HEAD_INITIALIZER(pool);
 static unsigned int release_pool_size;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 02/35] WIP: coroutine: manually tag the fast-paths

[Marc-André Lureau]

Some functions are both regular and coroutine. They may call coroutine
functions in some cases, if it is known to be running in a coroutine.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block.c                 |  2 ++
 block/block-backend.c   |  2 ++
 block/io.c              | 16 +++++++++++++++-
 block/sheepdog.c        |  2 ++
 block/throttle-groups.c | 10 ++++++++--
 migration/rdma.c        |  2 ++
 6 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 694396281b..b08c006da4 100644
--- a/block.c
+++ b/block.c
@@ -443,7 +443,9 @@ int bdrv_create(BlockDriver *drv, const char* filename,
 
     if (qemu_in_coroutine()) {
         /* Fast-path if already in coroutine context */
+        co_role_acquire(_coroutine_fn);
         bdrv_create_co_entry(&cco);
+        co_role_release(_coroutine_fn);
     } else {
         co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
         qemu_coroutine_enter(co);
diff --git a/block/block-backend.c b/block/block-backend.c
index 0df3457a09..56fc0a4d1e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1072,7 +1072,9 @@ static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,
 
     if (qemu_in_coroutine()) {
         /* Fast-path if already in coroutine context */
+        co_role_acquire(_coroutine_fn);
         co_entry(&rwco);
+        co_role_release(_coroutine_fn);
     } else {
         Coroutine *co = qemu_coroutine_create(co_entry, &rwco);
         bdrv_coroutine_enter(blk_bs(blk), co);
diff --git a/block/io.c b/block/io.c
index 2de7c77983..14b88c8609 100644
--- a/block/io.c
+++ b/block/io.c
@@ -229,7 +229,9 @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs)
 void bdrv_drained_begin(BlockDriverState *bs)
 {
     if (qemu_in_coroutine()) {
+        co_role_acquire(_coroutine_fn);
         bdrv_co_yield_to_drain(bs);
+        co_role_release(_coroutine_fn);
         return;
     }
 
@@ -616,7 +618,9 @@ static int bdrv_prwv_co(BdrvChild *child, int64_t offset,
 
     if (qemu_in_coroutine()) {
         /* Fast-path if already in coroutine context */
+        co_role_acquire(_coroutine_fn);
         bdrv_rw_co_entry(&rwco);
+        co_role_release(_coroutine_fn);
     } else {
         co = qemu_coroutine_create(bdrv_rw_co_entry, &rwco);
         bdrv_coroutine_enter(child->bs, co);
@@ -1901,7 +1905,9 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
 
     if (qemu_in_coroutine()) {
         /* Fast-path if already in coroutine context */
+        co_role_acquire(_coroutine_fn);
         bdrv_get_block_status_above_co_entry(&data);
+        co_role_release(_coroutine_fn);
     } else {
         co = qemu_coroutine_create(bdrv_get_block_status_above_co_entry,
                                    &data);
@@ -2027,7 +2033,11 @@ bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
                 bool is_read)
 {
     if (qemu_in_coroutine()) {
-        return bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
+        int ret;
+        co_role_acquire(_coroutine_fn);
+        ret = bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
+        co_role_release(_coroutine_fn);
+        return ret;
     } else {
         BdrvVmstateCo data = {
             .bs         = bs,
@@ -2259,7 +2269,9 @@ int bdrv_flush(BlockDriverState *bs)
 
     if (qemu_in_coroutine()) {
         /* Fast-path if already in coroutine context */
+        co_role_acquire(_coroutine_fn);
         bdrv_flush_co_entry(&flush_co);
+        co_role_release(_coroutine_fn);
     } else {
         co = qemu_coroutine_create(bdrv_flush_co_entry, &flush_co);
         bdrv_coroutine_enter(bs, co);
@@ -2406,7 +2418,9 @@ int bdrv_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
 
     if (qemu_in_coroutine()) {
         /* Fast-path if already in coroutine context */
+        co_role_acquire(_coroutine_fn);
         bdrv_pdiscard_co_entry(&rwco);
+        co_role_release(_coroutine_fn);
     } else {
         co = qemu_coroutine_create(bdrv_pdiscard_co_entry, &rwco);
         bdrv_coroutine_enter(bs, co);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 08d7b11e9d..83bc43dde4 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -726,7 +726,9 @@ static int do_req(int sockfd, BlockDriverState *bs, SheepdogReq *hdr,
     };
 
     if (qemu_in_coroutine()) {
+        co_role_acquire(_coroutine_fn);
         do_co_req(&srco);
+        co_role_release(_coroutine_fn);
     } else {
         co = qemu_coroutine_create(do_co_req, &srco);
         if (bs) {
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index da2b490c38..8778f78965 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -304,9 +304,15 @@ static void schedule_next_request(BlockBackend *blk, bool is_write)
 
     /* If it doesn't have to wait, queue it for immediate execution */
     if (!must_wait) {
+        bool handled = false;
+
+        if (qemu_in_coroutine()) {
+            co_role_acquire(_coroutine_fn);
+            handled = throttle_group_co_restart_queue(blk, is_write);
+            co_role_release(_coroutine_fn);
+        }
         /* Give preference to requests from the current blk */
-        if (qemu_in_coroutine() &&
-            throttle_group_co_restart_queue(blk, is_write)) {
+        if (handled) {
             token = blk;
         } else {
             ThrottleTimers *tt = &blk_get_public(token)->throttle_timers;
diff --git a/migration/rdma.c b/migration/rdma.c
index c6bc607a03..8c00d4d74c 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1518,7 +1518,9 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma, int wrid_requested,
          * so don't yield unless we know we're running inside of a coroutine.
          */
         if (rdma->migration_started_on_destination) {
+            co_role_acquire(_coroutine_fn);
             yield_until_fd_readable(rdma->comp_channel->fd);
+            co_role_release(_coroutine_fn);
         }
 
         if (ibv_get_cq_event(rdma->comp_channel, &cq, &cq_ctx)) {
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 03/35] test-coroutine: fix coroutine attribute

[Marc-André Lureau]

  CC      tests/test-coroutine.o
qemu/tests/test-coroutine.c:390:5: warning: calling function 'qemu_coroutine_yield' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
    qemu_coroutine_yield();
    ^
qemu/tests/test-coroutine.c:403:14: warning: Unmached attributes [-Wthread-safety-analysis]
        co = qemu_coroutine_create(perf_cost_func, &i);
             ^
2 warnings generated.

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

diff --git a/tests/test-coroutine.c b/tests/test-coroutine.c
index abd97c23c1..f14277af83 100644
--- a/tests/test-coroutine.c
+++ b/tests/test-coroutine.c
@@ -385,7 +385,7 @@ static void perf_baseline(void)
         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.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 04/35] coroutine: remove coroutine_fn from qemu_coroutine_self()

[Marc-André Lureau]

The function may be safely called from non-coroutine context.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/coroutine.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 35ff394f51..ec55fe295c 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -121,7 +121,7 @@ void coroutine_fn qemu_coroutine_yield(void);
 /**
  * Get the currently executing coroutine
  */
-Coroutine *coroutine_fn qemu_coroutine_self(void);
+Coroutine *qemu_coroutine_self(void);
 
 /**
  * Return whether or not currently inside a coroutine
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 05/35] coroutine: remove coroutine_fn from qemu_co_queue_run_restart()

[Marc-André Lureau]

The function can be invoked from non-coroutine context.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/coroutine_int.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index cb98892bba..e9fc72c5b0 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -61,6 +61,6 @@ Coroutine *qemu_coroutine_new(void);
 void qemu_coroutine_delete(Coroutine *co);
 CoroutineAction qemu_coroutine_switch(Coroutine *from, Coroutine *to,
                                       CoroutineAction action);
-void coroutine_fn qemu_co_queue_run_restart(Coroutine *co);
+void qemu_co_queue_run_restart(Coroutine *co);
 
 #endif
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 06/35] coroutine: mark CoRwLock coroutine_fn

[Marc-André Lureau]

  CC      util/qemu-coroutine-lock.o
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:371:5: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
    qemu_co_mutex_lock(&lock->mutex);
    ^
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:371:5: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:374:9: warning: calling function 'qemu_co_queue_wait' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
        qemu_co_queue_wait(&lock->queue, &lock->mutex);
        ^
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:374:9: warning: calling function 'qemu_co_queue_wait' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:377:5: warning: calling function 'qemu_co_mutex_unlock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
    qemu_co_mutex_unlock(&lock->mutex);
    ^
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:377:5: warning: calling function 'qemu_co_mutex_unlock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:390:9: warning: calling function 'qemu_co_queue_restart_all' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
        qemu_co_queue_restart_all(&lock->queue);
        ^
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:390:9: warning: calling function 'qemu_co_queue_restart_all' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:394:9: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
        qemu_co_mutex_lock(&lock->mutex);
        ^
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:394:9: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:399:13: warning: calling function 'qemu_co_queue_next' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
            qemu_co_queue_next(&lock->queue);
            ^
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:399:13: warning: calling function 'qemu_co_queue_next' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:402:5: warning: calling function 'qemu_co_mutex_unlock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
    qemu_co_mutex_unlock(&lock->mutex);
    ^
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:402:5: warning: calling function 'qemu_co_mutex_unlock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:407:5: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
    qemu_co_mutex_lock(&lock->mutex);
    ^
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:407:5: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:410:9: warning: calling function 'qemu_co_queue_wait' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
        qemu_co_queue_wait(&lock->queue, &lock->mutex);
        ^
/home/elmarco/src/qemu/util/qemu-coroutine-lock.c:410:9: warning: calling function 'qemu_co_queue_wait' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/coroutine.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index ec55fe295c..5698414c14 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -255,20 +255,20 @@ void qemu_co_rwlock_init(CoRwlock *lock);
  * of a parallel writer, control is transferred to the caller of the current
  * coroutine.
  */
-void qemu_co_rwlock_rdlock(CoRwlock *lock);
+void coroutine_fn qemu_co_rwlock_rdlock(CoRwlock *lock);
 
 /**
  * Write Locks the mutex. If the lock cannot be taken immediately because
  * of a parallel reader, control is transferred to the caller of the current
  * coroutine.
  */
-void qemu_co_rwlock_wrlock(CoRwlock *lock);
+void coroutine_fn qemu_co_rwlock_wrlock(CoRwlock *lock);
 
 /**
  * Unlocks the read/write lock and schedules the next coroutine that was
  * waiting for this lock to be run.
  */
-void qemu_co_rwlock_unlock(CoRwlock *lock);
+void coroutine_fn qemu_co_rwlock_unlock(CoRwlock *lock);
 
 /**
  * Yield the coroutine for a given duration
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 07/35] blockjob: mark coroutine_fn

[Marc-André Lureau]

/home/elmarco/src/qemu/blockjob.c:820:9: error: calling function 'qemu_coroutine_yield' requires holding role '_coroutine_fn' exclusively [-Werror,-Wthread-safety-analysis]
        qemu_coroutine_yield();
        ^
/home/elmarco/src/qemu/blockjob.c:824:5: error: calling function 'block_job_pause_point' requires holding role '_coroutine_fn' exclusively [-Werror,-Wthread-safety-analysis]
    block_job_pause_point(job);
    ^
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/block/blockjob_int.h | 4 ++--
 blockjob.c                   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index f13ad05c0d..a3bc01fd51 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -145,7 +145,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
  * Put the job to sleep (assuming that it wasn't canceled) for @ns
  * nanoseconds.  Canceling the job will interrupt the wait immediately.
  */
-void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns);
+void coroutine_fn block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns);
 
 /**
  * block_job_yield:
@@ -153,7 +153,7 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns);
  *
  * Yield the block job coroutine.
  */
-void block_job_yield(BlockJob *job);
+void coroutine_fn block_job_yield(BlockJob *job);
 
 /**
  * block_job_pause_all:
diff --git a/blockjob.c b/blockjob.c
index 70a78188b7..96424323c1 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -788,7 +788,8 @@ bool block_job_is_cancelled(BlockJob *job)
     return job->cancelled;
 }
 
-void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
+void coroutine_fn
+block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
 {
     assert(job->busy);
 
@@ -806,7 +807,8 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
     block_job_pause_point(job);
 }
 
-void block_job_yield(BlockJob *job)
+void coroutine_fn
+block_job_yield(BlockJob *job)
 {
     assert(job->busy);
 
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/block/block_int.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 15fa602150..93eb2a9528 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -133,15 +133,15 @@ struct BlockDriver {
     void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
 
     /* aio */
-    BlockAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs,
+    BlockAIOCB * coroutine_fn (*bdrv_aio_readv)(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockCompletionFunc *cb, void *opaque);
-    BlockAIOCB *(*bdrv_aio_writev)(BlockDriverState *bs,
+    BlockAIOCB * coroutine_fn (*bdrv_aio_writev)(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockCompletionFunc *cb, void *opaque);
-    BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
+    BlockAIOCB * coroutine_fn (*bdrv_aio_flush)(BlockDriverState *bs,
         BlockCompletionFunc *cb, void *opaque);
-    BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
+    BlockAIOCB * coroutine_fn (*bdrv_aio_pdiscard)(BlockDriverState *bs,
         int64_t offset, int bytes,
         BlockCompletionFunc *cb, void *opaque);
 
@@ -247,7 +247,7 @@ struct BlockDriver {
     void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
 
     /* to control generic scsi devices */
-    BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs,
+    BlockAIOCB * coroutine_fn (*bdrv_aio_ioctl)(BlockDriverState *bs,
         unsigned long int req, void *buf,
         BlockCompletionFunc *cb, void *opaque);
     int coroutine_fn (*bdrv_co_ioctl)(BlockDriverState *bs,
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 09/35] block: bdrv_create() and bdrv_debug_event() are coroutine_fn

[Marc-André Lureau]

Called from coroutine.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/block/block_int.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 93eb2a9528..a183c72b7c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -126,7 +126,7 @@ struct BlockDriver {
     int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
                           Error **errp);
     void (*bdrv_close)(BlockDriverState *bs);
-    int (*bdrv_create)(const char *filename, QemuOpts *opts, Error **errp);
+    int coroutine_fn (*bdrv_create)(const char *filename, QemuOpts *opts, Error **errp);
     int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
     int (*bdrv_make_empty)(BlockDriverState *bs);
 
@@ -267,7 +267,7 @@ struct BlockDriver {
                               BlockDriverAmendStatusCB *status_cb,
                               void *cb_opaque);
 
-    void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
+    void coroutine_fn (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
 
     /* TODO Better pass a option string/QDict/QemuOpts to add any rule? */
     int (*bdrv_debug_breakpoint)(BlockDriverState *bs, const char *event,
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 10/35] vmdk: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/vmdk.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 55581b03fe..f8422e8971 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1334,7 +1334,8 @@ static int64_t coroutine_fn vmdk_co_get_block_status(BlockDriverState *bs,
     return ret;
 }
 
-static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_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)
@@ -1406,7 +1407,8 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
     return ret;
 }
 
-static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
+static int coroutine_fn
+vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
                             int64_t offset_in_cluster, QEMUIOVector *qiov,
                             int bytes)
 {
@@ -1551,7 +1553,8 @@ fail:
  *
  * Returns: error code with 0 for success.
  */
-static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
+static int coroutine_fn
+vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
                        uint64_t bytes, QEMUIOVector *qiov,
                        bool zeroed, bool zero_dry_run)
 {
@@ -1857,7 +1860,8 @@ static int filename_decompose(const char *filename, char *path, char *prefix,
     return VMDK_OK;
 }
 
-static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int idx = 0;
     BlockBackend *new_blk = NULL;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 11/35] qcow2: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/qcow2.h         |  6 ++++--
 block/qcow.c          |  4 +++-
 block/qcow2-cluster.c | 11 +++++++----
 block/qcow2.c         | 15 ++++++++++-----
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 87b15eb4aa..a32b47b7f6 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -550,14 +550,16 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num,
 
 int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
                              unsigned int *bytes, uint64_t *cluster_offset);
-int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
                                unsigned int *bytes, uint64_t *host_offset,
                                QCowL2Meta **m);
 uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
                                          uint64_t offset,
                                          int compressed_size);
 
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
+int coroutine_fn
+qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
 int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
                           uint64_t bytes, enum qcow2_discard_type type,
                           bool full_discard);
diff --git a/block/qcow.c b/block/qcow.c
index 7bd94dcd46..d84ae7fb74 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -796,7 +796,9 @@ static void qcow_close(BlockDriverState *bs)
     error_free(s->migration_blocker);
 }
 
-static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
+
+static int coroutine_fn
+qcow_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int header_size, backing_filename_len, l1_size, shift, i;
     QCowHeader header;
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 3d341fd9cb..964d23aee8 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -761,7 +761,8 @@ uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
     return cluster_offset;
 }
 
-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;
@@ -890,7 +891,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;
@@ -1014,7 +1015,8 @@ out:
  *           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,
+static int coroutine_fn
+handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
     uint64_t *cur_bytes, QCowL2Meta **m)
 {
     BDRVQcow2State *s = bs->opaque;
@@ -1413,7 +1415,8 @@ fail:
  *
  * Return 0 on success and -errno in error cases
  */
-int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
                                unsigned int *bytes, uint64_t *host_offset,
                                QCowL2Meta **m)
 {
diff --git a/block/qcow2.c b/block/qcow2.c
index 2f94f0326e..6ecf1489dc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2079,7 +2079,8 @@ static int qcow2_change_backing_file(BlockDriverState *bs,
     return qcow2_update_header(bs);
 }
 
-static int preallocate(BlockDriverState *bs)
+static int coroutine_fn
+preallocate(BlockDriverState *bs)
 {
     uint64_t bytes;
     uint64_t offset;
@@ -2140,7 +2141,8 @@ static int preallocate(BlockDriverState *bs)
     return 0;
 }
 
-static int qcow2_create2(const char *filename, int64_t total_size,
+static int coroutine_fn
+qcow2_create2(const char *filename, int64_t total_size,
                          const char *backing_file, const char *backing_format,
                          int flags, size_t cluster_size, PreallocMode prealloc,
                          QemuOpts *opts, int version, int refcount_order,
@@ -2390,7 +2392,8 @@ out:
     return ret;
 }
 
-static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     char *backing_file = NULL;
     char *backing_fmt = NULL;
@@ -3011,7 +3014,8 @@ static void dump_refcounts(BlockDriverState *bs)
 }
 #endif
 
-static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+static int coroutine_fn
+qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
                               int64_t pos)
 {
     BDRVQcow2State *s = bs->opaque;
@@ -3021,7 +3025,8 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
                                     qiov->size, qiov, 0);
 }
 
-static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+static int coroutine_fn
+qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
                               int64_t pos)
 {
     BDRVQcow2State *s = bs->opaque;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 12/35] raw: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/raw-format.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/raw-format.c b/block/raw-format.c
index 0d185fe41b..402d3b9fba 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -361,7 +361,8 @@ 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) {
@@ -375,7 +376,8 @@ static int raw_has_zero_init(BlockDriverState *bs)
     return bdrv_has_zero_init(bs->file->bs);
 }
 
-static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     return bdrv_create_file(filename, opts, errp);
 }
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 13/35] nbd: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/nbd-client.h | 10 +++++-----
 block/nbd-client.c | 24 ++++++++++++++++--------
 block/nbd.c        |  3 ++-
 nbd/server.c       |  3 ++-
 4 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/block/nbd-client.h b/block/nbd-client.h
index 49636bc621..473d1f88fd 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -42,13 +42,13 @@ int nbd_client_init(BlockDriverState *bs,
                     Error **errp);
 void nbd_client_close(BlockDriverState *bs);
 
-int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes);
-int nbd_client_co_flush(BlockDriverState *bs);
-int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes);
+int coroutine_fn nbd_client_co_flush(BlockDriverState *bs);
+int coroutine_fn nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
                           uint64_t bytes, QEMUIOVector *qiov, int flags);
-int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+int coroutine_fn nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
                                 int bytes, BdrvRequestFlags flags);
-int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
                          uint64_t bytes, QEMUIOVector *qiov, int flags);
 
 void nbd_client_detach_aio_context(BlockDriverState *bs);
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 02e928142e..63c0210c37 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -111,7 +111,8 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
     s->read_reply_co = NULL;
 }
 
-static int nbd_co_send_request(BlockDriverState *bs,
+static int coroutine_fn
+nbd_co_send_request(BlockDriverState *bs,
                                NBDRequest *request,
                                QEMUIOVector *qiov)
 {
@@ -158,7 +159,8 @@ static int nbd_co_send_request(BlockDriverState *bs,
     return rc;
 }
 
-static void nbd_co_receive_reply(NBDClientSession *s,
+static void coroutine_fn
+nbd_co_receive_reply(NBDClientSession *s,
                                  NBDRequest *request,
                                  NBDReply *reply,
                                  QEMUIOVector *qiov)
@@ -185,7 +187,8 @@ static void nbd_co_receive_reply(NBDClientSession *s,
     }
 }
 
-static void nbd_coroutine_end(BlockDriverState *bs,
+static void coroutine_fn
+nbd_coroutine_end(BlockDriverState *bs,
                               NBDRequest *request)
 {
     NBDClientSession *s = nbd_get_client_session(bs);
@@ -204,7 +207,8 @@ static void nbd_coroutine_end(BlockDriverState *bs,
     qemu_co_mutex_unlock(&s->send_mutex);
 }
 
-int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
                          uint64_t bytes, QEMUIOVector *qiov, int flags)
 {
     NBDClientSession *client = nbd_get_client_session(bs);
@@ -229,7 +233,8 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
     return -reply.error;
 }
 
-int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
                           uint64_t bytes, QEMUIOVector *qiov, int flags)
 {
     NBDClientSession *client = nbd_get_client_session(bs);
@@ -258,7 +263,8 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
     return -reply.error;
 }
 
-int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+int coroutine_fn
+nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
                                 int bytes, BdrvRequestFlags flags)
 {
     ssize_t ret;
@@ -292,7 +298,8 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
     return -reply.error;
 }
 
-int nbd_client_co_flush(BlockDriverState *bs)
+int coroutine_fn
+nbd_client_co_flush(BlockDriverState *bs)
 {
     NBDClientSession *client = nbd_get_client_session(bs);
     NBDRequest request = { .type = NBD_CMD_FLUSH };
@@ -316,7 +323,8 @@ int nbd_client_co_flush(BlockDriverState *bs)
     return -reply.error;
 }
 
-int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
+int coroutine_fn
+nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
 {
     NBDClientSession *client = nbd_get_client_session(bs);
     NBDRequest request = {
diff --git a/block/nbd.c b/block/nbd.c
index d529305330..8689b27d7d 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -465,7 +465,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
     return ret;
 }
 
-static int nbd_co_flush(BlockDriverState *bs)
+static int coroutine_fn
+nbd_co_flush(BlockDriverState *bs)
 {
     return nbd_client_co_flush(bs);
 }
diff --git a/nbd/server.c b/nbd/server.c
index 8a70c054a6..4112b4b184 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -960,7 +960,8 @@ void nbd_export_close_all(void)
     }
 }
 
-static int nbd_co_send_reply(NBDRequestData *req, NBDReply *reply, int len)
+static int coroutine_fn
+nbd_co_send_reply(NBDRequestData *req, NBDReply *reply, int len)
 {
     NBDClient *client = req->client;
     int ret;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 14/35] migration: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 migration/migration.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 51ccd1a4c5..3370482637 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -303,7 +303,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)
 {
     QEMUFile *f = opaque;
     MigrationIncomingState *mis = migration_incoming_get_current();
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 15/35] backup: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/block/block_backup.h | 4 ++--
 block/backup.c               | 9 ++++++---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/block/block_backup.h b/include/block/block_backup.h
index 8a759477a3..415cf8519d 100644
--- a/include/block/block_backup.h
+++ b/include/block/block_backup.h
@@ -27,12 +27,12 @@ typedef struct CowRequest {
     CoQueue wait_queue; /* coroutines blocked on this request */
 } CowRequest;
 
-void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
+void coroutine_fn backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
                                           int nb_sectors);
 void backup_cow_request_begin(CowRequest *req, BlockJob *job,
                               int64_t sector_num,
                               int nb_sectors);
-void backup_cow_request_end(CowRequest *req);
+void coroutine_fn backup_cow_request_end(CowRequest *req);
 
 void backup_do_checkpoint(BlockJob *job, Error **errp);
 
diff --git a/block/backup.c b/block/backup.c
index 5387fbd84e..58ddd80b3f 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -84,7 +84,8 @@ static void cow_request_begin(CowRequest *req, BackupBlockJob *job,
 }
 
 /* Forget about a completed request */
-static void cow_request_end(CowRequest *req)
+static void coroutine_fn
+cow_request_end(CowRequest *req)
 {
     QLIST_REMOVE(req, list);
     qemu_co_queue_restart_all(&req->wait_queue);
@@ -275,7 +276,8 @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
     bitmap_zero(backup_job->done_bitmap, len);
 }
 
-void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
+void coroutine_fn
+backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
                                           int nb_sectors)
 {
     BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common);
@@ -304,7 +306,8 @@ void backup_cow_request_begin(CowRequest *req, BlockJob *job,
     cow_request_begin(req, backup_job, start, end);
 }
 
-void backup_cow_request_end(CowRequest *req)
+void coroutine_fn
+backup_cow_request_end(CowRequest *req)
 {
     cow_request_end(req);
 }
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 16/35] crypto: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/crypto.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/crypto.c b/block/crypto.c
index 10e5ddccaa..0e30a4ea06 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -568,7 +568,8 @@ static int block_crypto_open_luks(BlockDriverState *bs,
                                      bs, options, flags, errp);
 }
 
-static int block_crypto_create_luks(const char *filename,
+static int coroutine_fn
+block_crypto_create_luks(const char *filename,
                                     QemuOpts *opts,
                                     Error **errp)
 {
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 17/35] curl: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/curl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index 2a244e2439..d3719dc086 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -855,7 +855,8 @@ 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.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 18/35] gluster: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/gluster.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/gluster.c b/block/gluster.c
index addceed6eb..dea8ab43a5 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -965,7 +965,8 @@ static coroutine_fn int qemu_gluster_co_pwrite_zeroes(BlockDriverState *bs,
 }
 #endif
 
-static int qemu_gluster_create(const char *filename,
+static int coroutine_fn
+qemu_gluster_create(const char *filename,
                                QemuOpts *opts, Error **errp)
 {
     BlockdevOptionsGluster *gconf;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 19/35] nfs: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/nfs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/nfs.c b/block/nfs.c
index c3c5de0113..3f393a95a4 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -679,7 +679,8 @@ static QemuOptsList nfs_create_opts = {
     }
 };
 
-static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
 {
     int ret = 0;
     int64_t total_size = 0;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 20/35] quorum: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/quorum.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 55ba916655..b086d70daa 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -264,7 +264,8 @@ 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;
@@ -282,7 +283,8 @@ static void quorum_rewrite_entry(void *opaque)
     }
 }
 
-static bool quorum_rewrite_bad_versions(QuorumAIOCB *acb,
+static bool coroutine_fn
+quorum_rewrite_bad_versions(QuorumAIOCB *acb,
                                         QuorumVoteValue *value)
 {
     QuorumVoteVersion *version;
@@ -497,7 +499,8 @@ static int quorum_vote_error(QuorumAIOCB *acb)
     return ret;
 }
 
-static void quorum_vote(QuorumAIOCB *acb)
+static void coroutine_fn
+quorum_vote(QuorumAIOCB *acb)
 {
     bool quorum = true;
     int i, j, ret;
@@ -577,7 +580,8 @@ 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;
@@ -605,7 +609,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, ret;
@@ -648,7 +652,8 @@ static int read_quorum_children(QuorumAIOCB *acb)
     return 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;
@@ -669,7 +674,8 @@ static int read_fifo_child(QuorumAIOCB *acb)
     return ret;
 }
 
-static int quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
+static int coroutine_fn
+quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
                             uint64_t bytes, QEMUIOVector *qiov, int flags)
 {
     BDRVQuorumState *s = bs->opaque;
@@ -689,7 +695,7 @@ static int quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
     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,7 +721,8 @@ static void write_quorum_entry(void *opaque)
     }
 }
 
-static int quorum_co_pwritev(BlockDriverState *bs, uint64_t offset,
+static int coroutine_fn
+quorum_co_pwritev(BlockDriverState *bs, uint64_t offset,
                              uint64_t bytes, QEMUIOVector *qiov, int flags)
 {
     BDRVQuorumState *s = bs->opaque;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 21/35] rbd: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/rbd.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 9da02cdceb..7b4d548cd2 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -348,7 +348,8 @@ static QemuOptsList runtime_opts = {
     },
 };
 
-static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     Error *local_err = NULL;
     int64_t bytes = 0;
@@ -861,7 +862,8 @@ failed:
     return NULL;
 }
 
-static BlockAIOCB *qemu_rbd_aio_readv(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_readv(BlockDriverState *bs,
                                       int64_t sector_num,
                                       QEMUIOVector *qiov,
                                       int nb_sectors,
@@ -873,7 +875,8 @@ static BlockAIOCB *qemu_rbd_aio_readv(BlockDriverState *bs,
                          RBD_AIO_READ);
 }
 
-static BlockAIOCB *qemu_rbd_aio_writev(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_writev(BlockDriverState *bs,
                                        int64_t sector_num,
                                        QEMUIOVector *qiov,
                                        int nb_sectors,
@@ -886,7 +889,8 @@ static BlockAIOCB *qemu_rbd_aio_writev(BlockDriverState *bs,
 }
 
 #ifdef LIBRBD_SUPPORTS_AIO_FLUSH
-static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_flush(BlockDriverState *bs,
                                       BlockCompletionFunc *cb,
                                       void *opaque)
 {
@@ -1063,7 +1067,8 @@ static int qemu_rbd_snap_list(BlockDriverState *bs,
 }
 
 #ifdef LIBRBD_SUPPORTS_DISCARD
-static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_pdiscard(BlockDriverState *bs,
                                          int64_t offset,
                                          int bytes,
                                          BlockCompletionFunc *cb,
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 22/35] sheepdog: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/sheepdog.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 83bc43dde4..64ff275db9 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -481,7 +481,8 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
     return aio_req;
 }
 
-static void wait_for_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *acb)
+static void coroutine_fn
+wait_for_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *acb)
 {
     SheepdogAIOCB *cb;
 
@@ -494,7 +495,8 @@ retry:
     }
 }
 
-static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
+static void coroutine_fn
+sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
                          QEMUIOVector *qiov, int64_t sector_num, int nb_sectors,
                          int type)
 {
@@ -1954,7 +1956,8 @@ static int parse_block_size_shift(BDRVSheepdogState *s, QemuOpts *opt)
     return 0;
 }
 
-static int sd_create(const char *filename, QemuOpts *opts,
+static int coroutine_fn
+sd_create(const char *filename, QemuOpts *opts,
                      Error **errp)
 {
     Error *err = NULL;
@@ -2431,7 +2434,8 @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB *acb)
     }
 }
 
-static void sd_aio_complete(SheepdogAIOCB *acb)
+static void coroutine_fn
+sd_aio_complete(SheepdogAIOCB *acb)
 {
     if (acb->aiocb_type == AIOCB_FLUSH_CACHE) {
         return;
@@ -2905,7 +2909,8 @@ cleanup:
     return ret;
 }
 
-static int sd_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+static int coroutine_fn
+sd_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
                            int64_t pos)
 {
     BDRVSheepdogState *s = bs->opaque;
@@ -2920,7 +2925,8 @@ static int sd_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
     return ret;
 }
 
-static int sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+static int coroutine_fn
+sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
                            int64_t pos)
 {
     BDRVSheepdogState *s = bs->opaque;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 23/35] ssh: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/ssh.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 52964416da..03a8ebe6f7 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -813,7 +813,8 @@ static QemuOptsList ssh_create_opts = {
     }
 };
 
-static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+ssh_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int r, ret;
     int64_t total_size = 0;
@@ -1029,7 +1030,8 @@ static coroutine_fn int ssh_co_readv(BlockDriverState *bs,
     return ret;
 }
 
-static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
+static int coroutine_fn
+ssh_write(BDRVSSHState *s, BlockDriverState *bs,
                      int64_t offset, size_t size,
                      QEMUIOVector *qiov)
 {
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 24/35] null: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/null.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/block/null.c b/block/null.c
index 876f90965b..4c8afe16d7 100644
--- a/block/null.c
+++ b/block/null.c
@@ -167,7 +167,8 @@ static inline BlockAIOCB *null_aio_common(BlockDriverState *bs,
     return &acb->common;
 }
 
-static BlockAIOCB *null_aio_readv(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+null_aio_readv(BlockDriverState *bs,
                                   int64_t sector_num, QEMUIOVector *qiov,
                                   int nb_sectors,
                                   BlockCompletionFunc *cb,
@@ -182,7 +183,8 @@ static BlockAIOCB *null_aio_readv(BlockDriverState *bs,
     return null_aio_common(bs, cb, opaque);
 }
 
-static BlockAIOCB *null_aio_writev(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+null_aio_writev(BlockDriverState *bs,
                                    int64_t sector_num, QEMUIOVector *qiov,
                                    int nb_sectors,
                                    BlockCompletionFunc *cb,
@@ -191,7 +193,8 @@ static BlockAIOCB *null_aio_writev(BlockDriverState *bs,
     return null_aio_common(bs, cb, opaque);
 }
 
-static BlockAIOCB *null_aio_flush(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+null_aio_flush(BlockDriverState *bs,
                                   BlockCompletionFunc *cb,
                                   void *opaque)
 {
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 25/35] mirror: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/mirror.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 68744a17e8..2f0a9946d9 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -224,7 +224,8 @@ static int mirror_cow_align(MirrorBlockJob *s,
     return ret;
 }
 
-static inline void mirror_wait_for_io(MirrorBlockJob *s)
+static inline void coroutine_fn
+mirror_wait_for_io(MirrorBlockJob *s)
 {
     assert(!s->waiting_for_io);
     s->waiting_for_io = true;
@@ -239,7 +240,8 @@ static inline void mirror_wait_for_io(MirrorBlockJob *s)
  *          (new_end - sector_num) if tail is rounded up or down due to
  *          alignment or buffer limit.
  */
-static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
+static int coroutine_fn
+mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
                           int nb_sectors)
 {
     BlockBackend *source = s->common.blk;
@@ -490,7 +492,8 @@ static void mirror_free_init(MirrorBlockJob *s)
  * mirror_resume() because mirror_run() will begin iterating again
  * when the job is resumed.
  */
-static void mirror_wait_for_all_io(MirrorBlockJob *s)
+static void coroutine_fn
+mirror_wait_for_all_io(MirrorBlockJob *s)
 {
     while (s->in_flight > 0) {
         mirror_wait_for_io(s);
@@ -605,7 +608,8 @@ static void mirror_exit(BlockJob *job, void *opaque)
     bdrv_unref(src);
 }
 
-static void mirror_throttle(MirrorBlockJob *s)
+static void coroutine_fn
+mirror_throttle(MirrorBlockJob *s)
 {
     int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 
@@ -984,7 +988,8 @@ static void mirror_complete(BlockJob *job, Error **errp)
     block_job_enter(&s->common);
 }
 
-static void mirror_pause(BlockJob *job)
+static void coroutine_fn
+mirror_pause(BlockJob *job)
 {
     MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
 
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 26/35] iscsi: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/iscsi.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 54067e2620..e16311cb4a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1005,7 +1005,8 @@ static void iscsi_ioctl_handle_emulated(IscsiAIOCB *acb, int req, void *buf)
     qemu_bh_schedule(acb->bh);
 }
 
-static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+iscsi_aio_ioctl(BlockDriverState *bs,
         unsigned long int req, void *buf,
         BlockCompletionFunc *cb, void *opaque)
 {
@@ -2107,7 +2108,8 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
     return 0;
 }
 
-static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int ret = 0;
     int64_t total_size = 0;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 27/35] file-posix: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/file-posix.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 3927fabf06..adafbbb6a0 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1483,7 +1483,8 @@ static int aio_worker(void *arg)
     return ret;
 }
 
-static int paio_submit_co(BlockDriverState *bs, int fd,
+static int coroutine_fn
+paio_submit_co(BlockDriverState *bs, int fd,
                           int64_t offset, QEMUIOVector *qiov,
                           int bytes, int type)
 {
@@ -1599,7 +1600,8 @@ static void raw_aio_unplug(BlockDriverState *bs)
 #endif
 }
 
-static BlockAIOCB *raw_aio_flush(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+raw_aio_flush(BlockDriverState *bs,
         BlockCompletionFunc *cb, void *opaque)
 {
     BDRVRawState *s = bs->opaque;
@@ -1835,7 +1837,8 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
     return (int64_t)st.st_blocks * 512;
 }
 
-static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+raw_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int fd;
     int result = 0;
@@ -2526,7 +2529,8 @@ hdev_open_Mac_error:
 
 #if defined(__linux__)
 
-static BlockAIOCB *hdev_aio_ioctl(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+hdev_aio_ioctl(BlockDriverState *bs,
         unsigned long int req, void *buf,
         BlockCompletionFunc *cb, void *opaque)
 {
@@ -2592,7 +2596,8 @@ static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs,
     return -ENOTSUP;
 }
 
-static int hdev_create(const char *filename, QemuOpts *opts,
+static int coroutine_fn
+hdev_create(const char *filename, QemuOpts *opts,
                        Error **errp)
 {
     int fd;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 28/35] 9p: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@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 d1cfeaf10e..935a6c9a3c 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -312,21 +312,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.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 29/35] block: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/blkdebug.c  | 15 ++++++++++-----
 block/blkverify.c |  3 ++-
 block/io.c        |  9 ++++++---
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index a1b24b9b0d..d55e2e69c8 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -483,7 +483,8 @@ out:
     return ret;
 }
 
-static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
+static int coroutine_fn
+rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
 {
     BDRVBlkdebugState *s = bs->opaque;
     BlkdebugRule *rule = NULL;
@@ -563,7 +564,8 @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_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);
 
@@ -656,7 +658,8 @@ static void blkdebug_close(BlockDriverState *bs)
     g_free(s->config_file);
 }
 
-static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
+static void coroutine_fn
+suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
 {
     BDRVBlkdebugState *s = bs->opaque;
     BlkdebugSuspendedReq r;
@@ -681,7 +684,8 @@ static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
     g_free(r.tag);
 }
 
-static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
+static bool coroutine_fn
+process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
     bool injected)
 {
     BDRVBlkdebugState *s = bs->opaque;
@@ -712,7 +716,8 @@ static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
     return injected;
 }
 
-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;
diff --git a/block/blkverify.c b/block/blkverify.c
index 06369f9eac..d0c946173a 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -255,7 +255,8 @@ blkverify_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_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;
 
diff --git a/block/io.c b/block/io.c
index 14b88c8609..a53a86df3e 100644
--- a/block/io.c
+++ b/block/io.c
@@ -366,7 +366,8 @@ 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) {
         atomic_dec(&req->bs->serialising_in_flight);
@@ -381,7 +382,8 @@ static void tracked_request_end(BdrvTrackedRequest *req)
 /**
  * Add an active request to the tracked requests list
  */
-static void tracked_request_begin(BdrvTrackedRequest *req,
+static void coroutine_fn
+tracked_request_begin(BdrvTrackedRequest *req,
                                   BlockDriverState *bs,
                                   int64_t offset,
                                   unsigned int bytes,
@@ -2430,7 +2432,8 @@ int bdrv_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
     return rwco.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.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 30/35] block-backend: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/sysemu/block-backend.h |  4 ++--
 block/block-backend.c          | 36 ++++++++++++++++++++++++------------
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 1e05281fff..2f967037af 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -165,8 +165,8 @@ int blk_co_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
 int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
 BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
                           BlockCompletionFunc *cb, void *opaque);
-int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
-int blk_co_flush(BlockBackend *blk);
+int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
+int coroutine_fn blk_co_flush(BlockBackend *blk);
 int blk_flush(BlockBackend *blk);
 int blk_commit_all(void);
 void blk_drain(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index 56fc0a4d1e..a48aa4f900 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1032,7 +1032,8 @@ typedef struct BlkRwCo {
     BdrvRequestFlags flags;
 } BlkRwCo;
 
-static void blk_read_entry(void *opaque)
+static void coroutine_fn
+blk_read_entry(void *opaque)
 {
     BlkRwCo *rwco = opaque;
 
@@ -1040,7 +1041,8 @@ static void blk_read_entry(void *opaque)
                               rwco->qiov, rwco->flags);
 }
 
-static void blk_write_entry(void *opaque)
+static void coroutine_fn
+blk_write_entry(void *opaque)
 {
     BlkRwCo *rwco = opaque;
 
@@ -1195,7 +1197,8 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes,
     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;
@@ -1206,7 +1209,8 @@ 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;
@@ -1288,7 +1292,8 @@ BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t offset,
                         blk_aio_write_entry, flags, cb, opaque);
 }
 
-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;
@@ -1303,7 +1308,8 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
     return blk_aio_prwv(blk, 0, 0, NULL, blk_aio_flush_entry, 0, cb, opaque);
 }
 
-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;
@@ -1339,7 +1345,8 @@ int blk_co_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
     return bdrv_co_ioctl(blk_bs(blk), req, buf);
 }
 
-static void blk_ioctl_entry(void *opaque)
+static void coroutine_fn
+blk_ioctl_entry(void *opaque)
 {
     BlkRwCo *rwco = opaque;
     rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
@@ -1351,7 +1358,8 @@ int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf)
     return blk_prw(blk, req, buf, 0, blk_ioctl_entry, 0);
 }
 
-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;
@@ -1376,7 +1384,8 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
     return blk_aio_prwv(blk, req, 0, &qiov, blk_aio_ioctl_entry, 0, cb, opaque);
 }
 
-int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
+int coroutine_fn
+blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
 {
     int ret = blk_check_byte_request(blk, offset, bytes);
     if (ret < 0) {
@@ -1386,7 +1395,8 @@ int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
     return bdrv_co_pdiscard(blk_bs(blk), offset, bytes);
 }
 
-int blk_co_flush(BlockBackend *blk)
+int coroutine_fn
+blk_co_flush(BlockBackend *blk)
 {
     if (!blk_is_available(blk)) {
         return -ENOMEDIUM;
@@ -1395,7 +1405,8 @@ int blk_co_flush(BlockBackend *blk)
     return bdrv_co_flush(blk_bs(blk));
 }
 
-static void blk_flush_entry(void *opaque)
+static void coroutine_fn
+blk_flush_entry(void *opaque)
 {
     BlkRwCo *rwco = opaque;
     rwco->ret = blk_co_flush(rwco->blk);
@@ -1785,7 +1796,8 @@ int blk_truncate(BlockBackend *blk, int64_t offset, Error **errp)
     return bdrv_truncate(blk->root, offset, errp);
 }
 
-static void blk_pdiscard_entry(void *opaque)
+static void coroutine_fn
+blk_pdiscard_entry(void *opaque)
 {
     BlkRwCo *rwco = opaque;
     rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, rwco->qiov->size);
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 31/35] parallels: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/parallels.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/parallels.c b/block/parallels.c
index 8be46a7d48..213e42b9d2 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -472,7 +472,8 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res,
 }
 
 
-static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+parallels_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int64_t total_size, cl_size;
     uint8_t tmp[BDRV_SECTOR_SIZE];
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 32/35] qed: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/qed.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/qed.c b/block/qed.c
index 385381a78a..dd2859a1c9 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -622,7 +622,8 @@ out:
     return ret;
 }
 
-static int bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     uint64_t image_size = 0;
     uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 33/35] vdi: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/vdi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/vdi.c b/block/vdi.c
index 79af47763b..53cd7f64d8 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -716,7 +716,8 @@ vdi_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
     return ret;
 }
 
-static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vdi_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int ret = 0;
     uint64_t bytes = 0;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 34/35] vhdx: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/vhdx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 8b270b57c9..56b54f3ed7 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1787,7 +1787,8 @@ exit:
  *    .---- ~ ----------- ~ ------------ ~ ---------------- ~ -----------.
  *   1MB
  */
-static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     int ret = 0;
     uint64_t image_size = (uint64_t) 2 * GiB;
-- 
2.13.1.395.gf7b71de06

[Qemu-devel] [PATCH 35/35] vpc: mark coroutine_fn

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 block/vpc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/vpc.c b/block/vpc.c
index 4240ba9d1c..1b4aba20bd 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -872,7 +872,8 @@ static int create_fixed_disk(BlockBackend *blk, uint8_t *buf,
     return ret;
 }
 
-static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vpc_create(const char *filename, QemuOpts *opts, Error **errp)
 {
     uint8_t buf[1024];
     VHDFooter *footer = (VHDFooter *) buf;
-- 
2.13.1.395.gf7b71de06

Re: [Qemu-devel] [PATCH 00/35] RFC: coroutine annotations & clang check

[Markus Armbruster]

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

> Hi,
>
> After investigating a bit using clang-tidy to do some coroutine checks
> (and hitting a wall as there are no pre-processor info in the AST), it
> was suggested to me on the clang mailing list to try to use
> -Wthread-safety. I had to modify clang a bit to make it work on qemu
> code base (annotations on function typedef etc,
> https://github.com/elmarco/clang qemu-ta branch - very hackish state).

Hardcore!

Are your clang modifications upstreamable once cleaned up?

> The analysis simply checks that coroutine_fn are called from a
> coroutine "context" (or "role"). I couldn't find any misuse in qemu
> code base, however, a number of coroutine_fn annotations are missing.
>
> (I think it would make sense to squash all the "mark coroutine_fn"
> commits if we apply them, I tried to split them by domains/maintainer
> to ease review)
[...]
>  49 files changed, 299 insertions(+), 132 deletions(-)

"A number of coroutine_fn annotations are missing" seems to be an
understatement :)

Re: [Qemu-devel] [PATCH 00/35] RFC: coroutine annotations & clang check

[Marc-André Lureau]

Hi

On Wed, Jul 5, 2017 at 7:26 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Hi,
> >
> > After investigating a bit using clang-tidy to do some coroutine checks
> > (and hitting a wall as there are no pre-processor info in the AST), it
> > was suggested to me on the clang mailing list to try to use
> > -Wthread-safety. I had to modify clang a bit to make it work on qemu
> > code base (annotations on function typedef etc,
> > https://github.com/elmarco/clang qemu-ta branch - very hackish state).
>
> Hardcore!

I think the most hardcore part is experimenting with clang code base,
modifying a line, and waiting 10 minutes for the linker (and restart
it if it crashed!). This slows down progress and kill motivation. (so
happy qemu is written in C :)

>
> Are your clang modifications upstreamable once cleaned up?

I am trying to get some help on clang mailing list, but it is very far
from this point without it. My changes are gross (but it seems to do
the job..).

I would be also curious to know how hard a similar (only for this
purpose) check could be implemented for gcc, perhaps with plugins.

>
> > The analysis simply checks that coroutine_fn are called from a
> > coroutine "context" (or "role"). I couldn't find any misuse in qemu
> > code base, however, a number of coroutine_fn annotations are missing.
> >
> > (I think it would make sense to squash all the "mark coroutine_fn"
> > commits if we apply them, I tried to split them by domains/maintainer
> > to ease review)
> [...]
> >  49 files changed, 299 insertions(+), 132 deletions(-)
>
> "A number of coroutine_fn annotations are missing" seems to be an
> understatement :)
>

Re: [Qemu-devel] [PATCH 28/35] 9p: mark coroutine_fn

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]

On Wed,  5 Jul 2017 00:03:39 +0200
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---

Acked-by: Greg Kurz <groug@kaod.org>

>  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 d1cfeaf10e..935a6c9a3c 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -312,21 +312,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);


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn

[Paolo Bonzini]

On 05/07/2017 00:03, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/block/block_int.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 15fa602150..93eb2a9528 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -133,15 +133,15 @@ struct BlockDriver {
>      void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
>  
>      /* aio */
> -    BlockAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs,
> +    BlockAIOCB * coroutine_fn (*bdrv_aio_readv)(BlockDriverState *bs,
>          int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
>          BlockCompletionFunc *cb, void *opaque);
> -    BlockAIOCB *(*bdrv_aio_writev)(BlockDriverState *bs,
> +    BlockAIOCB * coroutine_fn (*bdrv_aio_writev)(BlockDriverState *bs,
>          int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
>          BlockCompletionFunc *cb, void *opaque);
> -    BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
> +    BlockAIOCB * coroutine_fn (*bdrv_aio_flush)(BlockDriverState *bs,
>          BlockCompletionFunc *cb, void *opaque);
> -    BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
> +    BlockAIOCB * coroutine_fn (*bdrv_aio_pdiscard)(BlockDriverState *bs,
>          int64_t offset, int bytes,
>          BlockCompletionFunc *cb, void *opaque);
>  
> @@ -247,7 +247,7 @@ struct BlockDriver {
>      void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
>  
>      /* to control generic scsi devices */
> -    BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs,
> +    BlockAIOCB * coroutine_fn (*bdrv_aio_ioctl)(BlockDriverState *bs,
>          unsigned long int req, void *buf,
>          BlockCompletionFunc *cb, void *opaque);
>      int coroutine_fn (*bdrv_co_ioctl)(BlockDriverState *bs,
> 


They are, but it's an implementation detail.  Why is this patch necessary?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 04/35] coroutine: remove coroutine_fn from qemu_coroutine_self()

[Paolo Bonzini]

On 05/07/2017 00:03, Marc-André Lureau wrote:
> The function may be safely called from non-coroutine context.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

It can, but it shouldn't...  What are the callers?

Paolo

> ---
>  include/qemu/coroutine.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
> index 35ff394f51..ec55fe295c 100644
> --- a/include/qemu/coroutine.h
> +++ b/include/qemu/coroutine.h
> @@ -121,7 +121,7 @@ void coroutine_fn qemu_coroutine_yield(void);
>  /**
>   * Get the currently executing coroutine
>   */
> -Coroutine *coroutine_fn qemu_coroutine_self(void);
> +Coroutine *qemu_coroutine_self(void);
>  
>  /**
>   * Return whether or not currently inside a coroutine
> 

Re: [Qemu-devel] [PATCH 01/35] WIP: coroutine: annotate coroutine with clang thread safety attributes

[Paolo Bonzini]

On 05/07/2017 00:03, Marc-André Lureau wrote:
> +typedef int TAA_ROLE coroutine_role;
> +extern coroutine_role _coroutine_fn;
> +
> +static inline void co_role_acquire(coroutine_role R) TAA_ACQUIRE(R) TAA_NO_ANALYSYS {}
> +static inline void co_role_release(coroutine_role R) TAA_RELEASE(R) TAA_NO_ANALYSYS {}

Is the argument necessary, or could it be simply TAA_ACQUIRE(_coroutine_fn)?

Paolo

Re: [Qemu-devel] [PATCH 04/35] coroutine: remove coroutine_fn from qemu_coroutine_self()

[Marc-André Lureau]

Hi

----- Original Message -----
> On 05/07/2017 00:03, Marc-André Lureau wrote:
> > The function may be safely called from non-coroutine context.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> It can, but it shouldn't...  What are the callers?

There is aio_co_enter() & qemu_aio_coroutine_enter() that call it without coroutine context, but they are probably safe and can be manually tagged as coroutine-section.

This help reveal a few more functions to be marked coroutine_fn:

diff --git a/block/io.c b/block/io.c
index a53a86df3e..e2dc1bf061 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1952,6 +1952,7 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
  *  allocated/unallocated state.
  *
  */
+coroutine_fn
 int bdrv_is_allocated_above(BlockDriverState *top,
                             BlockDriverState *base,
                             int64_t sector_num,
diff --git a/block/iscsi.c b/block/iscsi.c
index e16311cb4a..5af98d9e99 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -310,6 +310,7 @@ out:
     }
 }
 
+coroutine_fn
 static void iscsi_co_init_iscsitask(IscsiLun *iscsilun, struct IscsiTask *iTask)
 {
     *iTask = (struct IscsiTask) {
diff --git a/block/nfs.c b/block/nfs.c
index 3f393a95a4..9198d4406e 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -220,6 +220,7 @@ static void nfs_process_write(void *arg)
     qemu_mutex_unlock(&client->mutex);
 }
 
+coroutine_fn
 static void nfs_co_init_task(BlockDriverState *bs, NFSRPC *task)
 {
     *task = (NFSRPC) {
diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 1d25147392..9891908970 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -164,6 +164,7 @@ static int qcow2_cache_flush_dependency(BlockDriverState *bs, Qcow2Cache *c)
     return 0;
 }
 
+coroutine_fn
 static int qcow2_cache_entry_flush(BlockDriverState *bs, Qcow2Cache *c, int i)
 {
     BDRVQcow2State *s = bs->opaque;
@@ -221,6 +222,7 @@ static int qcow2_cache_entry_flush(BlockDriverState *bs, Qcow2Cache *c, int i)
     return 0;
 }
 
+coroutine_fn
 int qcow2_cache_write(BlockDriverState *bs, Qcow2Cache *c)
 {
     BDRVQcow2State *s = bs->opaque;
@@ -240,6 +242,7 @@ int qcow2_cache_write(BlockDriverState *bs, Qcow2Cache *c)
     return result;
 }
 
+coroutine_fn
 int qcow2_cache_flush(BlockDriverState *bs, Qcow2Cache *c)
 {
     int result = qcow2_cache_write(bs, c);
@@ -282,6 +285,7 @@ void qcow2_cache_depends_on_flush(Qcow2Cache *c)
     c->depends_on_flush = true;
 }
 
+coroutine_fn
 int qcow2_cache_empty(BlockDriverState *bs, Qcow2Cache *c)
 {
     int ret, i;
@@ -304,6 +308,7 @@ int qcow2_cache_empty(BlockDriverState *bs, Qcow2Cache *c)
     return 0;
 }
 
+coroutine_fn
 static int qcow2_cache_do_get(BlockDriverState *bs, Qcow2Cache *c,
     uint64_t offset, void **table, bool read_from_disk)
 {
@@ -378,12 +383,14 @@ found:
     return 0;
 }
 
+coroutine_fn
 int qcow2_cache_get(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,
     void **table)
 {
     return qcow2_cache_do_get(bs, c, offset, table, true);
 }
 
+coroutine_fn
 int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset,
     void **table)
 {
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 964d23aee8..da4d0c2b98 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1087,6 +1087,7 @@ handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
  *
  *  -errno: in error cases
  */
+coroutine_fn
 static int handle_copied(BlockDriverState *bs, uint64_t guest_offset,
     uint64_t *host_offset, uint64_t *bytes, QCowL2Meta **m)
 {
@@ -1195,6 +1196,7 @@ out:
  * function has been waiting for another request and the allocation must be
  * restarted, but the whole request should not be failed.
  */
+coroutine_fn
 static int do_alloc_cluster_offset(BlockDriverState *bs, uint64_t guest_offset,
                                    uint64_t *host_offset, uint64_t *nb_clusters)
 {
@@ -1243,6 +1245,7 @@ static int do_alloc_cluster_offset(BlockDriverState *bs, uint64_t guest_offset,
  *
  *  -errno: in error cases
  */
+coroutine_fn
 static int handle_alloc(BlockDriverState *bs, uint64_t guest_offset,
     uint64_t *host_offset, uint64_t *bytes, QCowL2Meta **m)
 {
diff --git a/block/quorum.c b/block/quorum.c
index b086d70daa..9772b20a78 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -154,6 +154,7 @@ static bool quorum_64bits_compare(QuorumVoteValue *a, QuorumVoteValue *b)
     return a->l == b->l;
 }
 
+coroutine_fn
 static QuorumAIOCB *quorum_aio_get(BlockDriverState *bs,
                                    QEMUIOVector *qiov,
                                    uint64_t offset,
diff --git a/nbd/server.c b/nbd/server.c
index 4112b4b184..eb7f98ba32 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -995,6 +995,7 @@ nbd_co_send_reply(NBDRequestData *req, NBDReply *reply, int len)
  * the client (although the caller may still need to disconnect after reporting
  * the error).
  */
+coroutine_fn
 static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request)
 {
     NBDClient *client = req->client;
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index b44b5d55eb..a15cdfc6cd 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -173,6 +173,7 @@ typedef struct CoWaitRecord {
     QSLIST_ENTRY(CoWaitRecord) next;
 } CoWaitRecord;
 
+coroutine_fn
 static void push_waiter(CoMutex *mutex, CoWaitRecord *w)
 {
     w->co = qemu_coroutine_self();

Re: [Qemu-devel] [PATCH 04/35] coroutine: remove coroutine_fn from qemu_coroutine_self()

[Paolo Bonzini]

On 05/07/2017 15:39, Marc-André Lureau wrote:
>>> The function may be safely called from non-coroutine context.
>>> 
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> It can, but it shouldn't...  What are the callers?
>
> There is aio_co_enter() & qemu_aio_coroutine_enter() that call it
> without coroutine context, but they are probably safe and can be
> manually tagged as coroutine-section.

Yes, they are.  One is under qemu_in_coroutine (patch 2), the other is
part of the runtime (patch 1 maybe).

Awesome work by the way.

Paolo

Re: [Qemu-devel] [PATCH 01/35] WIP: coroutine: annotate coroutine with clang thread safety attributes

[Marc-André Lureau]

Hi

----- Original Message -----
> On 05/07/2017 00:03, Marc-André Lureau wrote:
> > +typedef int TAA_ROLE coroutine_role;
> > +extern coroutine_role _coroutine_fn;
> > +
> > +static inline void co_role_acquire(coroutine_role R) TAA_ACQUIRE(R)
> > TAA_NO_ANALYSYS {}
> > +static inline void co_role_release(coroutine_role R) TAA_RELEASE(R)
> > TAA_NO_ANALYSYS {}
> 
> Is the argument necessary, or could it be simply TAA_ACQUIRE(_coroutine_fn)?

Yes, it can be simplified.

thanks for the suggestion

Re: [Qemu-devel] [PATCH 31/35] parallels: mark coroutine_fn

[Denis V. Lunev]

On 07/05/2017 01:03 AM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  block/parallels.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 8be46a7d48..213e42b9d2 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -472,7 +472,8 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res,
>  }
>  
>  
> -static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
> +static int coroutine_fn
> +parallels_create(const char *filename, QemuOpts *opts, Error **errp)
>  {
>      int64_t total_size, cl_size;
>      uint8_t tmp[BDRV_SECTOR_SIZE];
Reviewed-by: Denis V. Lunev <den@openvz.org>

Re: [Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn

[Marc-André Lureau]

----- Original Message -----
> On 05/07/2017 00:03, Marc-André Lureau wrote:
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/block/block_int.h | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index 15fa602150..93eb2a9528 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -133,15 +133,15 @@ struct BlockDriver {
> >      void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
> >  
> >      /* aio */
> > -    BlockAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs,
> > +    BlockAIOCB * coroutine_fn (*bdrv_aio_readv)(BlockDriverState *bs,
> >          int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
> >          BlockCompletionFunc *cb, void *opaque);
> > -    BlockAIOCB *(*bdrv_aio_writev)(BlockDriverState *bs,
> > +    BlockAIOCB * coroutine_fn (*bdrv_aio_writev)(BlockDriverState *bs,
> >          int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
> >          BlockCompletionFunc *cb, void *opaque);
> > -    BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
> > +    BlockAIOCB * coroutine_fn (*bdrv_aio_flush)(BlockDriverState *bs,
> >          BlockCompletionFunc *cb, void *opaque);
> > -    BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
> > +    BlockAIOCB * coroutine_fn (*bdrv_aio_pdiscard)(BlockDriverState *bs,
> >          int64_t offset, int bytes,
> >          BlockCompletionFunc *cb, void *opaque);
> >  
> > @@ -247,7 +247,7 @@ struct BlockDriver {
> >      void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
> >  
> >      /* to control generic scsi devices */
> > -    BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs,
> > +    BlockAIOCB * coroutine_fn (*bdrv_aio_ioctl)(BlockDriverState *bs,
> >          unsigned long int req, void *buf,
> >          BlockCompletionFunc *cb, void *opaque);
> >      int coroutine_fn (*bdrv_co_ioctl)(BlockDriverState *bs,
> > 
> 
> 
> They are, but it's an implementation detail.  Why is this patch necessary?

I didn't think this would be controversial :) well, the checks I added to clang verify function pointer share the coroutine attribute.

The function themself are/need to be coroutine_fn (as they will call coroutine_fn too)

Re: [Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn

[Paolo Bonzini]

On 05/07/2017 16:21, Marc-André Lureau wrote:
>>
>> They are, but it's an implementation detail.  Why is this patch necessary?
> I didn't think this would be controversial :) well, the checks I added to clang verify function pointer share the coroutine attribute.
> 
> The function themself are/need to be coroutine_fn (as they will call coroutine_fn too)

It's not controversial, I would not have expected the functions to call
coroutine_fn. :)  How do they do that?

Paolo

Re: [Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn

[Marc-André Lureau]

Hi

----- Original Message -----
> 
> 
> On 05/07/2017 16:21, Marc-André Lureau wrote:
> >>
> >> They are, but it's an implementation detail.  Why is this patch necessary?
> > I didn't think this would be controversial :) well, the checks I added to
> > clang verify function pointer share the coroutine attribute.
> > 
> > The function themself are/need to be coroutine_fn (as they will call
> > coroutine_fn too)
> 
> It's not controversial, I would not have expected the functions to call
> coroutine_fn. :)  How do they do that?
> 

For example,  null_co_readv() calls  null_co_common() which calls co_aio_sleep_ns()

Re: [Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn

[Paolo Bonzini]

On 05/07/2017 18:06, Marc-André Lureau wrote:
>>> coroutine_fn too)
>> It's not controversial, I would not have expected the functions to call
>> coroutine_fn. :)  How do they do that?
>>
> For example,  null_co_readv() calls  null_co_common() which calls co_aio_sleep_ns()

But these are bdrv_co_*, not bdrv_aio_*.

Paolo

Re: [Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn

[Marc-André Lureau]

Hi

----- Original Message -----
> 
> 
> On 05/07/2017 18:06, Marc-André Lureau wrote:
> >>> coroutine_fn too)
> >> It's not controversial, I would not have expected the functions to call
> >> coroutine_fn. :)  How do they do that?
> >>
> > For example,  null_co_readv() calls  null_co_common() which calls
> > co_aio_sleep_ns()
> 
> But these are bdrv_co_*, not bdrv_aio_*.

Oops, right.

Indeed, it's not needed, but to avoid coroutine annotation mismatch, we would have to remove a few:

static coroutine_fn BlockAIOCB *raw_aio_pdiscard(BlockDriverState *bs,

static coroutine_fn BlockAIOCB *hdev_aio_pdiscard(BlockDriverState *bs,

Only those 2, it seems.

Re: [Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn

[Paolo Bonzini]

On 05/07/2017 18:40, Marc-André Lureau wrote:
> Hi
> 
> ----- Original Message -----
>>
>>
>> On 05/07/2017 18:06, Marc-André Lureau wrote:
>>>>> coroutine_fn too)
>>>> It's not controversial, I would not have expected the functions to call
>>>> coroutine_fn. :)  How do they do that?
>>>>
>>> For example,  null_co_readv() calls  null_co_common() which calls
>>> co_aio_sleep_ns()
>>
>> But these are bdrv_co_*, not bdrv_aio_*.
> 
> Oops, right.
> 
> Indeed, it's not needed, but to avoid coroutine annotation mismatch, we would have to remove a few:
> 
> static coroutine_fn BlockAIOCB *raw_aio_pdiscard(BlockDriverState *bs,
> 
> static coroutine_fn BlockAIOCB *hdev_aio_pdiscard(BlockDriverState *bs,
> 
> Only those 2, it seems.

Good!  If it's just those two, they are wrong indeed.  I'd be surprised
to see more (and even more surprised to see that the annotations were
right :)).

Paolo

Re: [Qemu-devel] [PATCH 13/35] nbd: mark coroutine_fn

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3661 bytes --]

On 07/04/2017 05:03 PM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  block/nbd-client.h | 10 +++++-----
>  block/nbd-client.c | 24 ++++++++++++++++--------
>  block/nbd.c        |  3 ++-
>  nbd/server.c       |  3 ++-
>  4 files changed, 25 insertions(+), 15 deletions(-)
> 

> diff --git a/block/nbd-client.h b/block/nbd-client.h
> index 49636bc621..473d1f88fd 100644
> --- a/block/nbd-client.h
> +++ b/block/nbd-client.h
> @@ -42,13 +42,13 @@ int nbd_client_init(BlockDriverState *bs,
>                      Error **errp);
>  void nbd_client_close(BlockDriverState *bs);
>  
> -int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes);
> -int nbd_client_co_flush(BlockDriverState *bs);
> -int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
> +int coroutine_fn nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes);
> +int coroutine_fn nbd_client_co_flush(BlockDriverState *bs);
> +int coroutine_fn nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
>                            uint64_t bytes, QEMUIOVector *qiov, int flags);

Indentation is off.

> -int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
> +int coroutine_fn nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
>                                  int bytes, BdrvRequestFlags flags);
> -int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
> +int coroutine_fn nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
>                           uint64_t bytes, QEMUIOVector *qiov, int flags);
>  

and some more

>  void nbd_client_detach_aio_context(BlockDriverState *bs);
> diff --git a/block/nbd-client.c b/block/nbd-client.c
> index 02e928142e..63c0210c37 100644
> --- a/block/nbd-client.c
> +++ b/block/nbd-client.c
> @@ -111,7 +111,8 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
>      s->read_reply_co = NULL;
>  }
>  
> -static int nbd_co_send_request(BlockDriverState *bs,
> +static int coroutine_fn
> +nbd_co_send_request(BlockDriverState *bs,
>                                 NBDRequest *request,
>                                 QEMUIOVector *qiov)
>  {

I know some project specifically like the newline between return type
and function name (it becomes easier to search for function
implementations if ALL function names start in the first column), but it
looks a bit odd compared to the usual qemu style.  I guess you did it
for line length reasons.  But once you do it, now the indentation is off
on the remaining parameters.

> @@ -158,7 +159,8 @@ static int nbd_co_send_request(BlockDriverState *bs,
>      return rc;
>  }
>  
> -static void nbd_co_receive_reply(NBDClientSession *s,
> +static void coroutine_fn
> +nbd_co_receive_reply(NBDClientSession *s,
>                                   NBDRequest *request,
>                                   NBDReply *reply,
>                                   QEMUIOVector *qiov)

Recurring theme of whitespace.

> @@ -185,7 +187,8 @@ static void nbd_co_receive_reply(NBDClientSession *s,
>      }
>  }
>  
> -static void nbd_coroutine_end(BlockDriverState *bs,
> +static void coroutine_fn
> +nbd_coroutine_end(BlockDriverState *bs,
>                                NBDRequest *request)

This even fits on one line now.

Whitespace fixes are trivial enough, so I'm still okay if you 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 02/35] WIP: coroutine: manually tag the fast-paths

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1402 bytes --]

On Wed, Jul 05, 2017 at 12:03:13AM +0200, Marc-André Lureau wrote:
> Some functions are both regular and coroutine. They may call coroutine
> functions in some cases, if it is known to be running in a coroutine.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  block.c                 |  2 ++
>  block/block-backend.c   |  2 ++
>  block/io.c              | 16 +++++++++++++++-
>  block/sheepdog.c        |  2 ++
>  block/throttle-groups.c | 10 ++++++++--
>  migration/rdma.c        |  2 ++
>  6 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 694396281b..b08c006da4 100644
> --- a/block.c
> +++ b/block.c
> @@ -443,7 +443,9 @@ int bdrv_create(BlockDriver *drv, const char* filename,
>  
>      if (qemu_in_coroutine()) {
>          /* Fast-path if already in coroutine context */
> +        co_role_acquire(_coroutine_fn);
>          bdrv_create_co_entry(&cco);
> +        co_role_release(_coroutine_fn);
>      } else {
>          co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
>          qemu_coroutine_enter(co);

I guess the clever analysis for clang would be to detect that if
(qemu_in_coroutine()) means we have the _coroutine_fn role.  It's
similar to how Coverity sees an if (ptr) and knows whether the pointer
is NULL/non-NULL in the branches.

But this patch is okay too :-).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 05/35] coroutine: remove coroutine_fn from qemu_co_queue_run_restart()

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 348 bytes --]

On Wed, Jul 05, 2017 at 12:03:16AM +0200, Marc-André Lureau wrote:
> The function can be invoked from non-coroutine context.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/coroutine_int.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 06/35] coroutine: mark CoRwLock coroutine_fn

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 4204 bytes --]

On Wed, Jul 05, 2017 at 12:03:17AM +0200, Marc-André Lureau wrote:
>   CC      util/qemu-coroutine-lock.o
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:371:5: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
>     qemu_co_mutex_lock(&lock->mutex);
>     ^
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:371:5: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:374:9: warning: calling function 'qemu_co_queue_wait' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
>         qemu_co_queue_wait(&lock->queue, &lock->mutex);
>         ^
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:374:9: warning: calling function 'qemu_co_queue_wait' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:377:5: warning: calling function 'qemu_co_mutex_unlock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
>     qemu_co_mutex_unlock(&lock->mutex);
>     ^
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:377:5: warning: calling function 'qemu_co_mutex_unlock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:390:9: warning: calling function 'qemu_co_queue_restart_all' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
>         qemu_co_queue_restart_all(&lock->queue);
>         ^
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:390:9: warning: calling function 'qemu_co_queue_restart_all' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:394:9: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
>         qemu_co_mutex_lock(&lock->mutex);
>         ^
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:394:9: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:399:13: warning: calling function 'qemu_co_queue_next' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
>             qemu_co_queue_next(&lock->queue);
>             ^
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:399:13: warning: calling function 'qemu_co_queue_next' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:402:5: warning: calling function 'qemu_co_mutex_unlock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
>     qemu_co_mutex_unlock(&lock->mutex);
>     ^
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:402:5: warning: calling function 'qemu_co_mutex_unlock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:407:5: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
>     qemu_co_mutex_lock(&lock->mutex);
>     ^
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:407:5: warning: calling function 'qemu_co_mutex_lock' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:410:9: warning: calling function 'qemu_co_queue_wait' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
>         qemu_co_queue_wait(&lock->queue, &lock->mutex);
>         ^
> /home/elmarco/src/qemu/util/qemu-coroutine-lock.c:410:9: warning: calling function 'qemu_co_queue_wait' requires holding role '_coroutine_fn' exclusively [-Wthread-safety-analysis]
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/coroutine.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH 07/35] blockjob: mark coroutine_fn

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 779 bytes --]

On Wed, Jul 05, 2017 at 12:03:18AM +0200, Marc-André Lureau wrote:
> /home/elmarco/src/qemu/blockjob.c:820:9: error: calling function 'qemu_coroutine_yield' requires holding role '_coroutine_fn' exclusively [-Werror,-Wthread-safety-analysis]
>         qemu_coroutine_yield();
>         ^
> /home/elmarco/src/qemu/blockjob.c:824:5: error: calling function 'block_job_pause_point' requires holding role '_coroutine_fn' exclusively [-Werror,-Wthread-safety-analysis]
>     block_job_pause_point(job);
>     ^
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/block/blockjob_int.h | 4 ++--
>  blockjob.c                   | 6 ++++--
>  2 files changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 02/35] WIP: coroutine: manually tag the fast-paths

[Marc-André Lureau]

Hi

----- Original Message -----
> On Wed, Jul 05, 2017 at 12:03:13AM +0200, Marc-André Lureau wrote:
> > Some functions are both regular and coroutine. They may call coroutine
> > functions in some cases, if it is known to be running in a coroutine.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  block.c                 |  2 ++
> >  block/block-backend.c   |  2 ++
> >  block/io.c              | 16 +++++++++++++++-
> >  block/sheepdog.c        |  2 ++
> >  block/throttle-groups.c | 10 ++++++++--
> >  migration/rdma.c        |  2 ++
> >  6 files changed, 31 insertions(+), 3 deletions(-)
> > 
> > diff --git a/block.c b/block.c
> > index 694396281b..b08c006da4 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -443,7 +443,9 @@ int bdrv_create(BlockDriver *drv, const char* filename,
> >  
> >      if (qemu_in_coroutine()) {
> >          /* Fast-path if already in coroutine context */
> > +        co_role_acquire(_coroutine_fn);
> >          bdrv_create_co_entry(&cco);
> > +        co_role_release(_coroutine_fn);
> >      } else {
> >          co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
> >          qemu_coroutine_enter(co);
> 
> I guess the clever analysis for clang would be to detect that if
> (qemu_in_coroutine()) means we have the _coroutine_fn role.  It's
> similar to how Coverity sees an if (ptr) and knows whether the pointer
> is NULL/non-NULL in the branches.
> 
> But this patch is okay too :-).

Right, I though about using try_acquire_capability, similarly needed for try_lock etc. However, I don't see how to automatically release the capability when going out of scope. Apparently there are some known limitations around these patterns. I would love to hear from compilers folks what they think about -Wthread-safety and if it can be added to gcc with various kind of improvements.

Re: [Qemu-devel] [PATCH 09/35] block: bdrv_create() and bdrv_debug_event() are coroutine_fn

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 316 bytes --]

On Wed, Jul 05, 2017 at 12:03:20AM +0200, Marc-André Lureau wrote:
> Called from coroutine.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/block/block_int.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH 10/35] vmdk: mark coroutine_fn

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 282 bytes --]

On Wed, Jul 05, 2017 at 12:03:21AM +0200, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  block/vmdk.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 11/35] qcow2: mark coroutine_fn

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 414 bytes --]

On Wed, Jul 05, 2017 at 12:03:22AM +0200, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  block/qcow2.h         |  6 ++++--
>  block/qcow.c          |  4 +++-
>  block/qcow2-cluster.c | 11 +++++++----
>  block/qcow2.c         | 15 ++++++++++-----
>  4 files changed, 24 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 12/35] raw: mark coroutine_fn

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 281 bytes --]

On Wed, Jul 05, 2017 at 12:03:23AM +0200, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  block/raw-format.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 14/35] migration: mark coroutine_fn

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 280 bytes --]

On Wed, Jul 05, 2017 at 12:03:25AM +0200, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  migration/migration.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH 15/35] backup: mark coroutine_fn

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 337 bytes --]

On Wed, Jul 05, 2017 at 12:03:26AM +0200, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/block/block_backup.h | 4 ++--
>  block/backup.c               | 9 ++++++---
>  2 files changed, 8 insertions(+), 5 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH 14/35] migration: mark coroutine_fn

[Juan Quintela]

Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  migration/migration.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 51ccd1a4c5..3370482637 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -303,7 +303,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)
>  {
>      QEMUFile *f = opaque;
>      MigrationIncomingState *mis = migration_incoming_get_current();

Reviewed-by: Juan Quintela <quintela@redhat.com>


Should I include it on the next migration PULL or is this entering some
other way?

Later, Juan.

>  LocalWords:  Quintela

Re: [Qemu-devel] [PATCH 14/35] migration: mark coroutine_fn

[Marc-André Lureau]

Hi

----- Original Message -----
> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  migration/migration.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 51ccd1a4c5..3370482637 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -303,7 +303,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)
> >  {
> >      QEMUFile *f = opaque;
> >      MigrationIncomingState *mis = migration_incoming_get_current();
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> 
> Should I include it on the next migration PULL or is this entering some
> other way?

Not the I know of, feel free to include it next time.

Thanks

Re: [Qemu-devel] [PATCH 16/35] crypto: mark coroutine_fn

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1313 bytes --]

On 07/04/2017 05:03 PM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  block/crypto.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index 10e5ddccaa..0e30a4ea06 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -568,7 +568,8 @@ static int block_crypto_open_luks(BlockDriverState *bs,
>                                       bs, options, flags, errp);
>  }
>  
> -static int block_crypto_create_luks(const char *filename,
> +static int coroutine_fn
> +block_crypto_create_luks(const char *filename,
>                                      QemuOpts *opts,
>                                      Error **errp)

Indentation is now off.

I don't know how hard we should try to keep our usual style of:

really long labels fn_name(parameters)

vs. the style I'm used to in emacs:

really long labels
fn_name(parameters)

the latter makes it easier to grep for a function implementation (grep
for ^fn_name), and also makes it easier to fit more parameters without
line wrapping, but is not heavily used in our current code base.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]