Re: [Qemu-devel] [RFC] block: Removed coroutine ownership assumption

[Kevin Wolf]

Am 02.07.2012 13:12, schrieb Peter Crosthwaite:
> On Mon, Jul 2, 2012 at 8:44 PM, Kevin Wolf <address@hidden> wrote:
>> Am 02.07.2012 12:18, schrieb Peter Maydell:
>>> On 2 July 2012 11:01, Kevin Wolf <address@hidden> wrote:
>>>> Reading from block devices during device initialisation breaks
>>>> migration, so I'd like to see it go away wherever possible. Reading in
>>>> the whole image file doesn't sound like something for which a good
>>>> excuse exists, you can do that as well during the first access.
>>>
>>> It's much nicer to be able to produce an error message ("file
>>> doesn't exist", "file is too short for this flash device") at
>>> device startup rather than miles later on at first access,
>>
>> "file doesn't exist" is an error that occurs for the backend (-drive
>> if=none), not for the -device, so you shouldn't have to deal with that
>> at all.
>>
>>> and pulling in a 64K file at startup is a simple implementation.
>>> Why complicate things by adding code for "if this is the first
>>> access then read in the file"?
>>
>> Because then it works. :-)
>>
>> Migration works more or less like this:
>>
>> 1. Destination creates device model based on command line
>> 2. RAM is copied live, source keeps running
>> 3. Source stops, device state is transferred
>> 4. Destination starts running the VM
>>
>> Reading from a block device is meaningful the earliest in step 3,
>> because at earlier points the guest still runs on the source and can
>> overwrite the data on the block device. If you're reading in the whole
>> image, you're doing it in step 1, so your data will be outdated by the
>> time the migration completes.
>>
> 
> I feel like theres a "two birds with one stone" solution here, by
> making bdrv_aio_read just yield until step 3? Just an if (..)
> somewhere in the bdrv framework that says "while not ready for
> migration qemu_coroutine_yield()". You implement the postponed
> bdrv_read that you want, but you also get rid of the bdrv_read()s that
> everyone hates without having the rewrite all the small flashes with
> if-first-read-load-all logic.

Or we could just have a second "late init" callback into the devices
where such requests could be issued. That would feel a bit cleaner.

Kevin