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

[Fam Zheng <famz@redhat.com>]

On Thu, 12/17 09:44, Paolo Bonzini wrote:
> 
> 
> On 17/12/2015 01:59, Fam Zheng wrote:
> > 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?
> 
> In the coroutine case, the yield is hidden in the drivers, and it may or
> may not happen.  For example, qcow2_co_flush_to_os starts with
> 
>     qemu_co_mutex_lock(&s->lock);
> 
> which can yield.

bdrv_ioctl, on the contrary, is emulated with .bdrv_aio_ioctl, so it always
yields (unless -ENOTSUP), that's why I think aio_poll() is necessary in both
branches.

Fam