[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: fix bdrv_ioctl called from coroutine
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH] block: fix bdrv_ioctl called from coroutine |
Date: |
Thu, 17 Dec 2015 20:33:06 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> >> ---
> >> 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
Re: [Qemu-devel] [PATCH] block: fix bdrv_ioctl called from coroutine, Kevin Wolf, 2015/12/18