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

[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.

Regards,
Peter

> And this description doesn't even take things like cache coherency for
> image files into consideration.
>
>> I would have thought migration would be simpler with a "read
>> whole file at startup" implementation, because in that case
>> the required migration state is always "this block of memory",
>> rather than "sometimes this block of memory and sometimes a
>> flag which says we haven't initialised the memory and will
>> need to do a file read".
>
> Oh, so you're actually migrating the whole content of the flash device
> instead of using the same file on the migration destination? What's the
> advantage over rereading the file on the destination? You still need
> access to the file there, don't you?
>
> The approach with migrating the block device content probably works for
> your 64k flash, but what about my hard disk?
>
> Kevin