Re: [Qemu-devel] Re: [RFC][PATCH 00/12] qcow2: Convert qcow2 to use coroutines for async I/O

[Stefan Hajnoczi]

On Mon, Jan 24, 2011 at 11:58 AM, Kevin Wolf <address@hidden> wrote:
> Am 22.01.2011 10:29, schrieb Stefan Hajnoczi:
>> Kevin: Do you like this approach and do you want to develop it further?
>
> I think it looks like a good start. The code will look much nicer this
> way than with the callback jungle that you tried out in QED.
>
> I'm not completely sure about patches 10 and 12, I don't think I agree
> with the conversion approach. By making bdrv_pread/pwrite asynchronous,
> you force drivers to be converted all at once - which leads to big
> hammers as in patch 12 (by the way, I'm curious if you have tried how
> much performance is hurt?)

I have not measured performance.  We're serializing requests and doing
a bunch of extra system calls per request so I expect a noticable
degradation.  With some profiling and optimization we should be able
to get good performance though.

> Wouldn't we be better off if we added a bdrv_co_pread/pwrite and
> converted qcow2 step by step? I'm not sure what the easy way forward
> would be with patch 12, looks more like a dead end to me (though I
> haven't looked at it for more than a few minutes yet).

I think you're right.  I wanted to prove that it is possible to make
qcow2 asynchronous using coroutines.  Perhaps we should lay off on
making everything asynchronous and instead convert code incrementally.
 We wouldn't need patch 10 or patch 12.

There is one interesting feature of patch 10, it allows code to do
block I/O from normal and coroutine context.  i.e. you don't have to
rewrite all your metadata functions in order to use them from both
contexts.  This can be achieved or worked around in other ways, but I
think it's a neat feature :).

> One more thing I want to mention is that bdrv_aio_read doesn't have the
> same semantics as bdrv_read with respect to EOF. The AIO one returns
> -EINVAL when reading beyond EOF whereas bdrv_read returns zeros. I'd
> expect that we'll hit this with the conversion.

Thanks for pointing this out, I didn't know that.

I would like convert QED to use coroutines because we may be able to
simplify the code significantly.  Do you want to take over the qcow2
side of things from here?  I'm happy to do clean ups first so the code
is in a state that you feel comfortable with, just let me know.

Stefan