Re: [Qemu-devel] [Qemu-block] [PATCH 2/5] coroutine: abort if we try to enter coroutine scheduled for another ctx

[Stefan Hajnoczi <stefanha@gmail.com>]

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

On Sun, Nov 19, 2017 at 09:46:43PM -0500, Jeff Cody wrote:
> diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
> index cb98892..931cdc9 100644
> --- a/include/qemu/coroutine_int.h
> +++ b/include/qemu/coroutine_int.h
> @@ -53,6 +53,9 @@ struct Coroutine {
>  
>      /* Only used when the coroutine has yielded.  */
>      AioContext *ctx;
> +
> +    int scheduled;

s/int/bool/

> diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
> index d6095c1..2edab63 100644
> --- a/util/qemu-coroutine.c
> +++ b/util/qemu-coroutine.c
> @@ -109,6 +109,15 @@ void qemu_aio_coroutine_enter(AioContext *ctx, Coroutine *co)
>  
>      trace_qemu_aio_coroutine_enter(ctx, self, co, co->entry_arg);
>  
> +    /* if the Coroutine has already been scheduled, entering it again will
> +     * cause us to enter it twice, potentially even after the coroutine has
> +     * been deleted */
> +    if (co->scheduled == 1) {
> +        fprintf(stderr, "Cannot enter a co-routine that has already "
> +                        "been scheduled to run in a different AioContext\n");

This error message is too specific, the AioContext doesn't have to be
different from the current one:

block/blkdebug.c:        aio_co_schedule(qemu_get_current_aio_context(), qemu_coroutine_self());

If something calls qemu_aio_coroutine_enter() on the coroutine it might
be from the same AioContext - but still an error condition worth failing
loudly on.

I suggest simplifying the error message:

  fprintf(stderr, "Cannot enter a co-routine that has already "
                  "been scheduled\n");

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