Re: [Qemu-devel] [RFC][PATCH 9/9] block: Use bdrv_co_* instead of synchronous versions in coroutines

[Kevin Wolf]

Am 22.07.2011 11:17, schrieb Stefan Hajnoczi:
> On Fri, Jul 22, 2011 at 7:48 AM, Kevin Wolf <address@hidden> wrote:
>> Am 21.07.2011 17:23, schrieb Stefan Hajnoczi:
>>> On Fri, Jul 15, 2011 at 06:47:39PM +0200, Kevin Wolf wrote:
>>>> If we're already in a coroutine, there is no reason to use the synchronous
>>>> version of block layer functions when a coroutine one exists. This makes
>>>> bdrv_read/write/flush use bdrv_co_* when used inside a coroutine.
>>>>
>>>> Signed-off-by: Kevin Wolf <address@hidden>
>>>> ---
>>>>  block.c |   43 +++++++++++++++++++++++++++++++++++++++++++
>>>>  1 files changed, 43 insertions(+), 0 deletions(-)
>>>
>>> I made similar changes to prototype qcow2 coroutines.  They allowed
>>> synchronous code to run unmodified inside a coroutine.
>>>
>>> But do we want to keep the synchronous APIs?  They tend to be misused
>>> because they allow synchronous implementation of devices (extboot,
>>> onenand, and others).
>>>
>>> The only reason to keep these around is for qemu-img and perhaps some
>>> startup code before the VM is running.  But I think we could tackle
>>> those cases too simply by running in a coroutine and using a common
>>> event loop (which makes timers and bottom halves work too).
>>
>> One change at a time. :-)
>>
>> This series is manageable, can be reviewed in finite time and gives us
>> the immediate benefit of not blocking the VCPU any more. I know that
>> some tend to rewrite half of qemu for every idea they have, but I'd
>> prefer a more incremental approach.
> 
> Fair enough.  I just want to make sure that we're working in the
> direction of removing the synchronous stuff rather than propping it
> up.

Sure, I'm just trying to find an incremental approach to do it.

>> Removing all synchronous bdrv_read/write calls from the devices is a
>> task that I wouldn't underestimate, and at the same time a completely
>> unrelated task (you could have suggested the same with AIO callbacks). I
>> agree that we should do this change some time, but for now devices get
>> exactly what they got before.
> 
> AIO interfaces should be kept because they do not have drawbacks like
> synchronous interfaces.  The AIO interfaces provide the most efficient
> way of implementing raw.

Sorry, my wording was ambiguous. I meant you could have argued before
that synchronous bdrv_read/write calls in the device should be replaced
by AIO ones. It's not something entirely new that only coroutines
provide, they just provide it in a nicer way.

Kevin