[PATCH 0/6] coroutine: new sleep/wake API

[PATCH 0/6] coroutine: new sleep/wake API

[Paolo Bonzini]

This is a revamp of the qemu_co_sleep* API that makes it easier to
extend the API: the state that is needed to wake up a coroutine is now
part of the public API instead of hidden behind a pointer-to-pointer;
the API is made more extensible by pushing the rest of QemuCoSleepState
into local variables.

In the future, this will be extended to introduce a prepare/sleep/cancel
API similar to Linux's prepare_to_wait/schedule/finish_wait functions.
For now, this is just a nice refactoring.

Paolo Bonzini (6):
  coroutine-sleep: use a stack-allocated timer
  coroutine-sleep: disallow NULL QemuCoSleepState** argument
  coroutine-sleep: allow qemu_co_sleep_wake(NULL)
  coroutine-sleep: move timer out of QemuCoSleepState
  coroutine-sleep: replace QemuCoSleepState pointer with struct in the
    API
  coroutine-sleep: introduce qemu_co_sleep

 block/block-copy.c          | 10 +++---
 block/nbd.c                 | 14 +++-----
 include/qemu/coroutine.h    | 26 ++++++++------
 util/qemu-coroutine-sleep.c | 69 +++++++++++++++++--------------------
 4 files changed, 57 insertions(+), 62 deletions(-)

-- 
2.31.1

[PATCH 1/6] coroutine-sleep: use a stack-allocated timer

[Paolo Bonzini]

The lifetime of the timer is well-known (it cannot outlive
qemu_co_sleep_ns_wakeable, because it's deleted by the time the
coroutine resumes), so it is not necessary to place it on the heap.

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

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 8c4dac4fd7..eec6e81f3f 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -21,7 +21,7 @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
 
 struct QemuCoSleepState {
     Coroutine *co;
-    QEMUTimer *ts;
+    QEMUTimer ts;
     QemuCoSleepState **user_state_pointer;
 };
 
@@ -35,7 +35,7 @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
     if (sleep_state->user_state_pointer) {
         *sleep_state->user_state_pointer = NULL;
     }
-    timer_del(sleep_state->ts);
+    timer_del(&sleep_state->ts);
     aio_co_wake(sleep_state->co);
 }
 
@@ -50,7 +50,6 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
     AioContext *ctx = qemu_get_current_aio_context();
     QemuCoSleepState state = {
         .co = qemu_coroutine_self(),
-        .ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &state),
         .user_state_pointer = sleep_state,
     };
 
@@ -63,10 +62,11 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
+    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
     if (sleep_state) {
         *sleep_state = &state;
     }
-    timer_mod(state.ts, qemu_clock_get_ns(type) + ns);
+    timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
     if (sleep_state) {
         /*
@@ -75,5 +75,4 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
          */
         assert(*sleep_state == NULL);
     }
-    timer_free(state.ts);
 }
-- 
2.31.1

Re: [PATCH 1/6] coroutine-sleep: use a stack-allocated timer

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:25, Paolo Bonzini wrote:
> The lifetime of the timer is well-known (it cannot outlive
> qemu_co_sleep_ns_wakeable, because it's deleted by the time the
> coroutine resumes), so it is not necessary to place it on the heap.
> 
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

[PATCH 2/6] coroutine-sleep: disallow NULL QemuCoSleepState** argument

[Paolo Bonzini]

Simplify the code by removing conditionals.  qemu_co_sleep_ns
can simply use point the argument to an on-stack temporary.

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

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index ce5b9c6851..c5d7742989 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -295,7 +295,7 @@ typedef struct QemuCoSleepState QemuCoSleepState;
 
 /**
  * Yield the coroutine for a given duration. During this yield, @sleep_state
- * (if not NULL) is set to an opaque pointer, which may be used for
+ * is set to an opaque pointer, which may be used for
  * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
  * timer fires. Don't save the obtained value to other variables and don't call
  * qemu_co_sleep_wake from another aio context.
@@ -304,7 +304,8 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
                                             QemuCoSleepState **sleep_state);
 static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
 {
-    qemu_co_sleep_ns_wakeable(type, ns, NULL);
+    QemuCoSleepState *unused = NULL;
+    qemu_co_sleep_ns_wakeable(type, ns, &unused);
 }
 
 /**
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index eec6e81f3f..3f6f637e81 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -32,9 +32,7 @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
                                            qemu_co_sleep_ns__scheduled, NULL);
 
     assert(scheduled == qemu_co_sleep_ns__scheduled);
-    if (sleep_state->user_state_pointer) {
-        *sleep_state->user_state_pointer = NULL;
-    }
+    *sleep_state->user_state_pointer = NULL;
     timer_del(&sleep_state->ts);
     aio_co_wake(sleep_state->co);
 }
@@ -63,16 +61,10 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
     }
 
     aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
-    if (sleep_state) {
-        *sleep_state = &state;
-    }
+    *sleep_state = &state;
     timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
-    if (sleep_state) {
-        /*
-         * Note that *sleep_state is cleared during qemu_co_sleep_wake
-         * before resuming this coroutine.
-         */
-        assert(*sleep_state == NULL);
-    }
+
+    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
+    assert(*sleep_state == NULL);
 }
-- 
2.31.1

Re: [PATCH 2/6] coroutine-sleep: disallow NULL QemuCoSleepState** argument

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:25, Paolo Bonzini wrote:
> Simplify the code by removing conditionals.  qemu_co_sleep_ns
> can simply use point the argument to an on-stack temporary.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   include/qemu/coroutine.h    |  5 +++--
>   util/qemu-coroutine-sleep.c | 18 +++++-------------
>   2 files changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
> index ce5b9c6851..c5d7742989 100644
> --- a/include/qemu/coroutine.h
> +++ b/include/qemu/coroutine.h
> @@ -295,7 +295,7 @@ typedef struct QemuCoSleepState QemuCoSleepState;
>   
>   /**
>    * Yield the coroutine for a given duration. During this yield, @sleep_state
> - * (if not NULL) is set to an opaque pointer, which may be used for
> + * is set to an opaque pointer, which may be used for
>    * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
>    * timer fires. Don't save the obtained value to other variables and don't call
>    * qemu_co_sleep_wake from another aio context.
> @@ -304,7 +304,8 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
>                                               QemuCoSleepState **sleep_state);
>   static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
>   {
> -    qemu_co_sleep_ns_wakeable(type, ns, NULL);
> +    QemuCoSleepState *unused = NULL;
> +    qemu_co_sleep_ns_wakeable(type, ns, &unused);
>   }
>   
>   /**
> diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
> index eec6e81f3f..3f6f637e81 100644
> --- a/util/qemu-coroutine-sleep.c
> +++ b/util/qemu-coroutine-sleep.c
> @@ -32,9 +32,7 @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
>                                              qemu_co_sleep_ns__scheduled, NULL);
>   
>       assert(scheduled == qemu_co_sleep_ns__scheduled);
> -    if (sleep_state->user_state_pointer) {
> -        *sleep_state->user_state_pointer = NULL;
> -    }
> +    *sleep_state->user_state_pointer = NULL;
>       timer_del(&sleep_state->ts);
>       aio_co_wake(sleep_state->co);
>   }
> @@ -63,16 +61,10 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
>       }
>   
>       aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
> -    if (sleep_state) {
> -        *sleep_state = &state;
> -    }
> +    *sleep_state = &state;
>       timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
>       qemu_coroutine_yield();
> -    if (sleep_state) {
> -        /*
> -         * Note that *sleep_state is cleared during qemu_co_sleep_wake
> -         * before resuming this coroutine.
> -         */
> -        assert(*sleep_state == NULL);
> -    }
> +
> +    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */

over-80 line

> +    assert(*sleep_state == NULL);
>   }
> 

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

[PATCH 3/6] coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing

[Paolo Bonzini]

All callers of qemu_co_sleep_wake are checking whether they are passing
a NULL argument inside the pointer-to-pointer: do the check in
qemu_co_sleep_wake itself.

As a side effect, qemu_co_sleep_wake can be called more than once and it
will only wake the coroutine once; after the first time, the argument
will be set to NULL via *sleep_state->user_state_pointer.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/block-copy.c          |  4 +---
 block/nbd.c                 |  8 ++------
 util/qemu-coroutine-sleep.c | 21 ++++++++++++---------
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index 9b4af00614..f896dc56f2 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -674,9 +674,7 @@ out:
 
 void block_copy_kick(BlockCopyCallState *call_state)
 {
-    if (call_state->sleep_state) {
-        qemu_co_sleep_wake(call_state->sleep_state);
-    }
+    qemu_co_sleep_wake(call_state->sleep_state);
 }
 
 /*
diff --git a/block/nbd.c b/block/nbd.c
index 1d4668d42d..1c6315b168 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -289,9 +289,7 @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
 
     s->drained = true;
-    if (s->connection_co_sleep_ns_state) {
-        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
-    }
+    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
 
     nbd_co_establish_connection_cancel(bs, false);
 
@@ -330,9 +328,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 
     s->state = NBD_CLIENT_QUIT;
     if (s->connection_co) {
-        if (s->connection_co_sleep_ns_state) {
-            qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
-        }
+        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
         nbd_co_establish_connection_cancel(bs, true);
     }
     if (qemu_in_coroutine()) {
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 3f6f637e81..096eea3444 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -27,19 +27,22 @@ struct QemuCoSleepState {
 
 void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
 {
-    /* Write of schedule protected by barrier write in aio_co_schedule */
-    const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
-                                           qemu_co_sleep_ns__scheduled, NULL);
+    if (sleep_state) {
+        /* Write of schedule protected by barrier write in aio_co_schedule */
+        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
+                                               qemu_co_sleep_ns__scheduled, NULL);
 
-    assert(scheduled == qemu_co_sleep_ns__scheduled);
-    *sleep_state->user_state_pointer = NULL;
-    timer_del(&sleep_state->ts);
-    aio_co_wake(sleep_state->co);
+        assert(scheduled == qemu_co_sleep_ns__scheduled);
+        *sleep_state->user_state_pointer = NULL;
+        timer_del(&sleep_state->ts);
+        aio_co_wake(sleep_state->co);
+    }
 }
 
 static void co_sleep_cb(void *opaque)
 {
-    qemu_co_sleep_wake(opaque);
+    QemuCoSleepState **sleep_state = opaque;
+    qemu_co_sleep_wake(*sleep_state);
 }
 
 void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
@@ -60,7 +63,7 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
-    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
+    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
     *sleep_state = &state;
     timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
-- 
2.31.1

Re: [PATCH 3/6] coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:25, Paolo Bonzini wrote:
> All callers of qemu_co_sleep_wake are checking whether they are passing
> a NULL argument inside the pointer-to-pointer: do the check in
> qemu_co_sleep_wake itself.
> 
> As a side effect, qemu_co_sleep_wake can be called more than once and it
> will only wake the coroutine once; after the first time, the argument
> will be set to NULL via *sleep_state->user_state_pointer.
> 

commit message say nothing about change in co_sleep_cb: it now passes the user pointer too.

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   block/block-copy.c          |  4 +---
>   block/nbd.c                 |  8 ++------
>   util/qemu-coroutine-sleep.c | 21 ++++++++++++---------
>   3 files changed, 15 insertions(+), 18 deletions(-)
> 
> diff --git a/block/block-copy.c b/block/block-copy.c
> index 9b4af00614..f896dc56f2 100644
> --- a/block/block-copy.c
> +++ b/block/block-copy.c
> @@ -674,9 +674,7 @@ out:
>   
>   void block_copy_kick(BlockCopyCallState *call_state)
>   {
> -    if (call_state->sleep_state) {
> -        qemu_co_sleep_wake(call_state->sleep_state);
> -    }
> +    qemu_co_sleep_wake(call_state->sleep_state);
>   }
>   
>   /*
> diff --git a/block/nbd.c b/block/nbd.c
> index 1d4668d42d..1c6315b168 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -289,9 +289,7 @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
>       BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
>   
>       s->drained = true;
> -    if (s->connection_co_sleep_ns_state) {
> -        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
> -    }
> +    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
>   
>       nbd_co_establish_connection_cancel(bs, false);
>   
> @@ -330,9 +328,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
>   
>       s->state = NBD_CLIENT_QUIT;
>       if (s->connection_co) {
> -        if (s->connection_co_sleep_ns_state) {
> -            qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
> -        }
> +        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
>           nbd_co_establish_connection_cancel(bs, true);
>       }
>       if (qemu_in_coroutine()) {
> diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
> index 3f6f637e81..096eea3444 100644
> --- a/util/qemu-coroutine-sleep.c
> +++ b/util/qemu-coroutine-sleep.c
> @@ -27,19 +27,22 @@ struct QemuCoSleepState {
>   
>   void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
>   {
> -    /* Write of schedule protected by barrier write in aio_co_schedule */
> -    const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
> -                                           qemu_co_sleep_ns__scheduled, NULL);
> +    if (sleep_state) {
> +        /* Write of schedule protected by barrier write in aio_co_schedule */
> +        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
> +                                               qemu_co_sleep_ns__scheduled, NULL);

over-80 line (and indentation is broken)

>   
> -    assert(scheduled == qemu_co_sleep_ns__scheduled);
> -    *sleep_state->user_state_pointer = NULL;
> -    timer_del(&sleep_state->ts);
> -    aio_co_wake(sleep_state->co);
> +        assert(scheduled == qemu_co_sleep_ns__scheduled);
> +        *sleep_state->user_state_pointer = NULL;
> +        timer_del(&sleep_state->ts);
> +        aio_co_wake(sleep_state->co);
> +    }
>   }
>   
>   static void co_sleep_cb(void *opaque)
>   {
> -    qemu_co_sleep_wake(opaque);
> +    QemuCoSleepState **sleep_state = opaque;
> +    qemu_co_sleep_wake(*sleep_state);
>   }
>   
>   void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
> @@ -60,7 +63,7 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
>           abort();
>       }
>   
> -    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);
> +    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
>       *sleep_state = &state;
>       timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
>       qemu_coroutine_yield();
> 

Patch looks OK itself:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

(still, please improve commit message).

-- 
Best regards,
Vladimir

[PATCH 4/6] coroutine-sleep: move timer out of QemuCoSleepState

[Paolo Bonzini]

This simplification is enabled by the previous patch.  Now aio_co_wake
will only be called once, therefore we do not care about a spurious
firing of the timer after a qemu_co_sleep_wake.

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

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 096eea3444..68e9505b2e 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -21,7 +21,6 @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
 
 struct QemuCoSleepState {
     Coroutine *co;
-    QEMUTimer ts;
     QemuCoSleepState **user_state_pointer;
 };
 
@@ -34,7 +33,6 @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
 
         assert(scheduled == qemu_co_sleep_ns__scheduled);
         *sleep_state->user_state_pointer = NULL;
-        timer_del(&sleep_state->ts);
         aio_co_wake(sleep_state->co);
     }
 }
@@ -49,6 +47,7 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
                                             QemuCoSleepState **sleep_state)
 {
     AioContext *ctx = qemu_get_current_aio_context();
+    QEMUTimer ts;
     QemuCoSleepState state = {
         .co = qemu_coroutine_self(),
         .user_state_pointer = sleep_state,
@@ -63,10 +62,11 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
-    aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);
+    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
     *sleep_state = &state;
-    timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);
+    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
+    timer_del(&ts);
 
     /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
     assert(*sleep_state == NULL);
-- 
2.31.1

Re: [PATCH 4/6] coroutine-sleep: move timer out of QemuCoSleepState

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:25, Paolo Bonzini wrote:
> This simplification is enabled by the previous patch.  Now aio_co_wake
> will only be called once, therefore we do not care about a spurious
> firing of the timer after a qemu_co_sleep_wake.
> 
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

[PATCH 5/6] coroutine-sleep: replace QemuCoSleepState pointer with struct in the API

[Paolo Bonzini]

Right now, users of qemu_co_sleep_ns_wakeable are passing
a pointer to QemuCoSleepState by reference to the function, but
QemuCoSleepState really is just a Coroutine*.  Making the
content of the struct public is just as efficient and lets us
skip the user_state_pointer indirection: the Coroutine* is
cleared directly, rather than the pointer to it.

Since the usage is changed, take the occasion to rename the
struct to QemuCoSleep.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/block-copy.c          |  8 +++----
 block/nbd.c                 | 10 ++++-----
 include/qemu/coroutine.h    | 22 +++++++++----------
 util/qemu-coroutine-sleep.c | 43 ++++++++++++++++---------------------
 4 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index f896dc56f2..c2e5090412 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -50,7 +50,7 @@ typedef struct BlockCopyCallState {
     /* State */
     int ret;
     bool finished;
-    QemuCoSleepState *sleep_state;
+    QemuCoSleep sleep;
     bool cancelled;
 
     /* OUT parameters */
@@ -625,8 +625,8 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
                 if (ns > 0) {
                     block_copy_task_end(task, -EAGAIN);
                     g_free(task);
-                    qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns,
-                                              &call_state->sleep_state);
+                    qemu_co_sleep_ns_wakeable(&call_state->sleep,
+                                              QEMU_CLOCK_REALTIME, ns);
                     continue;
                 }
             }
@@ -674,7 +674,7 @@ out:
 
 void block_copy_kick(BlockCopyCallState *call_state)
 {
-    qemu_co_sleep_wake(call_state->sleep_state);
+    qemu_co_sleep_wake(&call_state->sleep);
 }
 
 /*
diff --git a/block/nbd.c b/block/nbd.c
index 1c6315b168..616f9ae6c4 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -116,7 +116,7 @@ typedef struct BDRVNBDState {
     CoQueue free_sema;
     Coroutine *connection_co;
     Coroutine *teardown_co;
-    QemuCoSleepState *connection_co_sleep_ns_state;
+    QemuCoSleep reconnect_sleep;
     bool drained;
     bool wait_drained_end;
     int in_flight;
@@ -289,7 +289,7 @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
 
     s->drained = true;
-    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
+    qemu_co_sleep_wake(&s->reconnect_sleep);
 
     nbd_co_establish_connection_cancel(bs, false);
 
@@ -328,7 +328,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
 
     s->state = NBD_CLIENT_QUIT;
     if (s->connection_co) {
-        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
+        qemu_co_sleep_wake(&s->reconnect_sleep);
         nbd_co_establish_connection_cancel(bs, true);
     }
     if (qemu_in_coroutine()) {
@@ -685,8 +685,8 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
             }
             bdrv_inc_in_flight(s->bs);
         } else {
-            qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
-                                      &s->connection_co_sleep_ns_state);
+            qemu_co_sleep_ns_wakeable(&s->reconnect_sleep,
+                                      QEMU_CLOCK_REALTIME, timeout);
             if (s->drained) {
                 continue;
             }
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index c5d7742989..77cb8ce459 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -291,21 +291,21 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
  */
 void qemu_co_rwlock_unlock(CoRwlock *lock);
 
-typedef struct QemuCoSleepState QemuCoSleepState;
+typedef struct QemuCoSleep {
+    Coroutine *to_wake;
+} QemuCoSleep;
 
 /**
- * Yield the coroutine for a given duration. During this yield, @sleep_state
- * is set to an opaque pointer, which may be used for
- * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
- * timer fires. Don't save the obtained value to other variables and don't call
- * qemu_co_sleep_wake from another aio context.
+ * Yield the coroutine for a given duration. During this yield, @w
+ * can be used with qemu_co_sleep_wake() to terminate the sleep.
  */
-void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-                                            QemuCoSleepState **sleep_state);
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+                                            QEMUClockType type, int64_t ns);
+
 static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
 {
-    QemuCoSleepState *unused = NULL;
-    qemu_co_sleep_ns_wakeable(type, ns, &unused);
+    QemuCoSleep w = { 0 };
+    qemu_co_sleep_ns_wakeable(&w, type, ns);
 }
 
 /**
@@ -314,7 +314,7 @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
  * qemu_co_sleep_ns() and should be checked to be non-NULL before calling
  * qemu_co_sleep_wake().
  */
-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state);
+void qemu_co_sleep_wake(QemuCoSleep *w);
 
 /**
  * Yield until a file descriptor becomes readable
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 68e9505b2e..89c3b758c5 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -19,42 +19,37 @@
 
 static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
 
-struct QemuCoSleepState {
+void qemu_co_sleep_wake(QemuCoSleep *w)
+{
     Coroutine *co;
-    QemuCoSleepState **user_state_pointer;
-};
 
-void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
-{
-    if (sleep_state) {
+    co = w->to_wake;
+    w->to_wake = NULL;
+    if (co) {
         /* Write of schedule protected by barrier write in aio_co_schedule */
-        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
-                                               qemu_co_sleep_ns__scheduled, NULL);
+        const char *scheduled = qatomic_cmpxchg(&co->scheduled,
+                                                qemu_co_sleep_ns__scheduled, NULL);
 
         assert(scheduled == qemu_co_sleep_ns__scheduled);
-        *sleep_state->user_state_pointer = NULL;
-        aio_co_wake(sleep_state->co);
+        aio_co_wake(co);
     }
 }
 
 static void co_sleep_cb(void *opaque)
 {
-    QemuCoSleepState **sleep_state = opaque;
-    qemu_co_sleep_wake(*sleep_state);
+    QemuCoSleep *w = opaque;
+    qemu_co_sleep_wake(w);
 }
 
-void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
-                                            QemuCoSleepState **sleep_state)
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+                                            QEMUClockType type, int64_t ns)
 {
+    Coroutine *co = qemu_coroutine_self();
     AioContext *ctx = qemu_get_current_aio_context();
     QEMUTimer ts;
-    QemuCoSleepState state = {
-        .co = qemu_coroutine_self(),
-        .user_state_pointer = sleep_state,
-    };
 
-    const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL,
-                                           qemu_co_sleep_ns__scheduled);
+    const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
+                                            qemu_co_sleep_ns__scheduled);
     if (scheduled) {
         fprintf(stderr,
                 "%s: Co-routine was already scheduled in '%s'\n",
@@ -62,12 +57,12 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
         abort();
     }
 
-    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
-    *sleep_state = &state;
+    w->to_wake = co;
+    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
     timer_mod(&ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
     timer_del(&ts);
 
-    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
-    assert(*sleep_state == NULL);
+    /* w->to_wake is cleared before resuming this coroutine.  */
+    assert(w->to_wake == NULL);
 }
-- 
2.31.1

Re: [PATCH 5/6] coroutine-sleep: replace QemuCoSleepState pointer with struct in the API

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:25, Paolo Bonzini wrote:
> Right now, users of qemu_co_sleep_ns_wakeable are passing
> a pointer to QemuCoSleepState by reference to the function, but
> QemuCoSleepState really is just a Coroutine*.  Making the
> content of the struct public is just as efficient and lets us
> skip the user_state_pointer indirection: the Coroutine* is
> cleared directly, rather than the pointer to it.
> 
> Since the usage is changed, take the occasion to rename the
> struct to QemuCoSleep.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   block/block-copy.c          |  8 +++----
>   block/nbd.c                 | 10 ++++-----
>   include/qemu/coroutine.h    | 22 +++++++++----------
>   util/qemu-coroutine-sleep.c | 43 ++++++++++++++++---------------------
>   4 files changed, 39 insertions(+), 44 deletions(-)
> 
> diff --git a/block/block-copy.c b/block/block-copy.c
> index f896dc56f2..c2e5090412 100644
> --- a/block/block-copy.c
> +++ b/block/block-copy.c
> @@ -50,7 +50,7 @@ typedef struct BlockCopyCallState {
>       /* State */
>       int ret;
>       bool finished;
> -    QemuCoSleepState *sleep_state;
> +    QemuCoSleep sleep;
>       bool cancelled;
>   
>       /* OUT parameters */
> @@ -625,8 +625,8 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state)
>                   if (ns > 0) {
>                       block_copy_task_end(task, -EAGAIN);
>                       g_free(task);
> -                    qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns,
> -                                              &call_state->sleep_state);
> +                    qemu_co_sleep_ns_wakeable(&call_state->sleep,
> +                                              QEMU_CLOCK_REALTIME, ns);
>                       continue;
>                   }
>               }
> @@ -674,7 +674,7 @@ out:
>   
>   void block_copy_kick(BlockCopyCallState *call_state)
>   {
> -    qemu_co_sleep_wake(call_state->sleep_state);
> +    qemu_co_sleep_wake(&call_state->sleep);
>   }
>   
>   /*
> diff --git a/block/nbd.c b/block/nbd.c
> index 1c6315b168..616f9ae6c4 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -116,7 +116,7 @@ typedef struct BDRVNBDState {
>       CoQueue free_sema;
>       Coroutine *connection_co;
>       Coroutine *teardown_co;
> -    QemuCoSleepState *connection_co_sleep_ns_state;
> +    QemuCoSleep reconnect_sleep;
>       bool drained;
>       bool wait_drained_end;
>       int in_flight;
> @@ -289,7 +289,7 @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
>       BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
>   
>       s->drained = true;
> -    qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
> +    qemu_co_sleep_wake(&s->reconnect_sleep);
>   
>       nbd_co_establish_connection_cancel(bs, false);
>   
> @@ -328,7 +328,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
>   
>       s->state = NBD_CLIENT_QUIT;
>       if (s->connection_co) {
> -        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
> +        qemu_co_sleep_wake(&s->reconnect_sleep);
>           nbd_co_establish_connection_cancel(bs, true);
>       }
>       if (qemu_in_coroutine()) {
> @@ -685,8 +685,8 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)
>               }
>               bdrv_inc_in_flight(s->bs);
>           } else {
> -            qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout,
> -                                      &s->connection_co_sleep_ns_state);
> +            qemu_co_sleep_ns_wakeable(&s->reconnect_sleep,
> +                                      QEMU_CLOCK_REALTIME, timeout);
>               if (s->drained) {
>                   continue;
>               }
> diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
> index c5d7742989..77cb8ce459 100644
> --- a/include/qemu/coroutine.h
> +++ b/include/qemu/coroutine.h
> @@ -291,21 +291,21 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
>    */
>   void qemu_co_rwlock_unlock(CoRwlock *lock);
>   
> -typedef struct QemuCoSleepState QemuCoSleepState;
> +typedef struct QemuCoSleep {
> +    Coroutine *to_wake;
> +} QemuCoSleep;
>   
>   /**
> - * Yield the coroutine for a given duration. During this yield, @sleep_state
> - * is set to an opaque pointer, which may be used for
> - * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the
> - * timer fires. Don't save the obtained value to other variables and don't call
> - * qemu_co_sleep_wake from another aio context.
> + * Yield the coroutine for a given duration. During this yield, @w
> + * can be used with qemu_co_sleep_wake() to terminate the sleep.

I'd add that function initializes @w.

>    */
> -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
> -                                            QemuCoSleepState **sleep_state);
> +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
> +                                            QEMUClockType type, int64_t ns);
> +
>   static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
>   {
> -    QemuCoSleepState *unused = NULL;
> -    qemu_co_sleep_ns_wakeable(type, ns, &unused);
> +    QemuCoSleep w = { 0 };
> +    qemu_co_sleep_ns_wakeable(&w, type, ns);
>   }
>   
>   /**
> @@ -314,7 +314,7 @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
>    * qemu_co_sleep_ns() and should be checked to be non-NULL before calling
>    * qemu_co_sleep_wake().
>    */
> -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state);
> +void qemu_co_sleep_wake(QemuCoSleep *w);
>   
>   /**
>    * Yield until a file descriptor becomes readable
> diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
> index 68e9505b2e..89c3b758c5 100644
> --- a/util/qemu-coroutine-sleep.c
> +++ b/util/qemu-coroutine-sleep.c
> @@ -19,42 +19,37 @@
>   
>   static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";
>   
> -struct QemuCoSleepState {
> +void qemu_co_sleep_wake(QemuCoSleep *w)
> +{
>       Coroutine *co;
> -    QemuCoSleepState **user_state_pointer;
> -};
>   
> -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)
> -{
> -    if (sleep_state) {
> +    co = w->to_wake;
> +    w->to_wake = NULL;

Don't you like g_steal_pointer()?

> +    if (co) {
>           /* Write of schedule protected by barrier write in aio_co_schedule */
> -        const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled,
> -                                               qemu_co_sleep_ns__scheduled, NULL);
> +        const char *scheduled = qatomic_cmpxchg(&co->scheduled,
> +                                                qemu_co_sleep_ns__scheduled, NULL);

indentation fixed, but not over-80 line )

>   
>           assert(scheduled == qemu_co_sleep_ns__scheduled);
> -        *sleep_state->user_state_pointer = NULL;
> -        aio_co_wake(sleep_state->co);
> +        aio_co_wake(co);
>       }
>   }
>   
>   static void co_sleep_cb(void *opaque)
>   {
> -    QemuCoSleepState **sleep_state = opaque;
> -    qemu_co_sleep_wake(*sleep_state);
> +    QemuCoSleep *w = opaque;
> +    qemu_co_sleep_wake(w);
>   }
>   
> -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
> -                                            QemuCoSleepState **sleep_state)
> +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
> +                                            QEMUClockType type, int64_t ns)
>   {
> +    Coroutine *co = qemu_coroutine_self();
>       AioContext *ctx = qemu_get_current_aio_context();
>       QEMUTimer ts;
> -    QemuCoSleepState state = {
> -        .co = qemu_coroutine_self(),
> -        .user_state_pointer = sleep_state,
> -    };
>   
> -    const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL,
> -                                           qemu_co_sleep_ns__scheduled);
> +    const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
> +                                            qemu_co_sleep_ns__scheduled);

indentation fixed..

>       if (scheduled) {
>           fprintf(stderr,
>                   "%s: Co-routine was already scheduled in '%s'\n",
> @@ -62,12 +57,12 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,
>           abort();
>       }
>   
> -    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);
> -    *sleep_state = &state;
> +    w->to_wake = co;
> +    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
>       timer_mod(&ts, qemu_clock_get_ns(type) + ns);
>       qemu_coroutine_yield();
>       timer_del(&ts);
>   
> -    /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine.  */
> -    assert(*sleep_state == NULL);
> +    /* w->to_wake is cleared before resuming this coroutine.  */
> +    assert(w->to_wake == NULL);
>   }
> 

with indentations and over-80 line fixed in appropriate previous patch:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

[PATCH 6/6] coroutine-sleep: introduce qemu_co_sleep

[Paolo Bonzini]

Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake.
This makes the logic of qemu_co_sleep_ns_wakeable easy to understand.

In the future we could introduce an API that can work even if the
sleep and wake happen from different threads.  For now, initializing
w->to_wake after timer_mod is fine because the timer can only fire in
the same AioContext.

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

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 77cb8ce459..b53e9632b9 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -302,6 +302,11 @@ typedef struct QemuCoSleep {
 void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
                                             QEMUClockType type, int64_t ns);
 
+/**
+ * Yield the coroutine until the next call to qemu_co_sleep_wake.
+ */
+void coroutine_fn qemu_co_sleep(QemuCoSleep *w);
+
 static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)
 {
     QemuCoSleep w = { 0 };
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 89c3b758c5..df6edba2e4 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -41,12 +41,9 @@ static void co_sleep_cb(void *opaque)
     qemu_co_sleep_wake(w);
 }
 
-void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
-                                            QEMUClockType type, int64_t ns)
+void coroutine_fn qemu_co_sleep(QemuCoSleep *w)
 {
     Coroutine *co = qemu_coroutine_self();
-    AioContext *ctx = qemu_get_current_aio_context();
-    QEMUTimer ts;
 
     const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,
                                             qemu_co_sleep_ns__scheduled);
@@ -58,11 +55,20 @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
     }
 
     w->to_wake = co;
-    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),
-    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
     qemu_coroutine_yield();
-    timer_del(&ts);
 
     /* w->to_wake is cleared before resuming this coroutine.  */
     assert(w->to_wake == NULL);
 }
+
+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,
+                                            QEMUClockType type, int64_t ns)
+{
+    AioContext *ctx = qemu_get_current_aio_context();
+    QEMUTimer ts;
+
+    aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w);
+    timer_mod(&ts, qemu_clock_get_ns(type) + ns);
+    qemu_co_sleep(w);
+    timer_del(&ts);
+}
-- 
2.31.1

Re: [PATCH 6/6] coroutine-sleep: introduce qemu_co_sleep

[Vladimir Sementsov-Ogievskiy]

03.05.2021 14:25, Paolo Bonzini wrote:
> Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake.
> This makes the logic of qemu_co_sleep_ns_wakeable easy to understand.
> 
> In the future we could introduce an API that can work even if the
> sleep and wake happen from different threads.  For now, initializing
> w->to_wake after timer_mod is fine because the timer can only fire in
> the same AioContext.

worth a comment inside qemu_co_sleep_ns_wakeable() ?

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir

Re: [PATCH 0/6] coroutine: new sleep/wake API

[Stefan Hajnoczi]

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

On Mon, May 03, 2021 at 01:25:44PM +0200, Paolo Bonzini wrote:
> This is a revamp of the qemu_co_sleep* API that makes it easier to
> extend the API: the state that is needed to wake up a coroutine is now
> part of the public API instead of hidden behind a pointer-to-pointer;
> the API is made more extensible by pushing the rest of QemuCoSleepState
> into local variables.
> 
> In the future, this will be extended to introduce a prepare/sleep/cancel
> API similar to Linux's prepare_to_wait/schedule/finish_wait functions.
> For now, this is just a nice refactoring.
> 
> Paolo Bonzini (6):
>   coroutine-sleep: use a stack-allocated timer
>   coroutine-sleep: disallow NULL QemuCoSleepState** argument
>   coroutine-sleep: allow qemu_co_sleep_wake(NULL)
>   coroutine-sleep: move timer out of QemuCoSleepState
>   coroutine-sleep: replace QemuCoSleepState pointer with struct in the
>     API
>   coroutine-sleep: introduce qemu_co_sleep
> 
>  block/block-copy.c          | 10 +++---
>  block/nbd.c                 | 14 +++-----
>  include/qemu/coroutine.h    | 26 ++++++++------
>  util/qemu-coroutine-sleep.c | 69 +++++++++++++++++--------------------
>  4 files changed, 57 insertions(+), 62 deletions(-)
> 
> -- 
> 2.31.1
> 

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

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