Re: [Qemu-devel] [PATCH] block: fix bdrv_ioctl called from coroutine

[Fam Zheng <famz@redhat.com>]

On Wed, 12/16 19:33, Paolo Bonzini wrote:
> When called from a coroutine, bdrv_ioctl must be asynchronous just like
> e.g. bdrv_flush.  The code was incorrectly making it synchronous, fix
> it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         Fam, any reason why you did it this way?  I don't see
>         any coroutine caller, but it doesn't make much sense. :)

That is a surprising question!  From a coroutine, it is bdrv_flush ->
bdrv_flush_co_entry -> bdrv_co_flush, which I think is always synchronous,
especially, noticing the code around calling bs->bdrv_aio_flush:

        acb = bs->drv->bdrv_aio_flush(bs, bdrv_co_io_em_complete, &co);
        if (acb == NULL) {
            ret = -EIO;
        } else {
            qemu_coroutine_yield();
            ret = co.ret;
        }

Am I missing something?

Fam

> 
>  block/io.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index e00fb5d..841f5b5 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2614,10 +2614,11 @@ int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
>          bdrv_co_ioctl_entry(&data);
>      } else {
>          Coroutine *co = qemu_coroutine_create(bdrv_co_ioctl_entry);
> +
>          qemu_coroutine_enter(co, &data);
> -    }
> -    while (data.ret == -EINPROGRESS) {
> -        aio_poll(bdrv_get_aio_context(bs), true);
> +        while (data.ret == -EINPROGRESS) {
> +            aio_poll(bdrv_get_aio_context(bs), true);
> +        }
>      }
>      return data.ret;
>  }
> -- 
> 2.5.0
>