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 <pbonz...@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