On Mon, Jul 2, 2012 at 8:52 PM, Kevin Wolf <kw...@redhat.com> wrote: > Am 02.07.2012 12:18, schrieb Peter Crosthwaite: >> On Mon, Jul 2, 2012 at 8:01 PM, Kevin Wolf <kw...@redhat.com> wrote: >>> Am 02.07.2012 11:42, schrieb Peter Crosthwaite: >>>> On Mon, Jul 2, 2012 at 7:04 PM, Kevin Wolf <kw...@redhat.com> wrote: >>>>> Am 02.07.2012 10:57, schrieb Peter Crosthwaite: >>>>>> No conditional on the qemu_coroutine_create. So it will always create >>>>>> a new coroutine for its work which will solve my problem. All I need >>>>>> to do is pump events once at the end of machine model creation. Any my >>>>>> coroutines will never yield or get queued by block/AIO. Sound like a >>>>>> solution? >>>>> >>>>> If you don't need the read data in your initialisation code, >>>> >>>> definately not :) Just as long as the read data is there by the time >>>> the machine goes live. Whats the current policy with bdrv_read()ing >>>> from init functions anyway? Several devices in qemu have init >>>> functions that read the entire storage into a buffer (then the guest >>>> just talks to the buffer rather than the backing store). >>> >>> 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, >> >> Makes sense for small devices on embedded platforms. You end up with a >> very simple and clean device model. The approach is totally broken for >> HDDs but it does make some sense for the little embedded flashes where >> you can get away with caching the entire device storage in RAM for the >> lifetime of the system. > > It kind of works for read-only devices, it's just ugly there. With > writeable devices it makes the VM unmigratable. >
Isnt it just a simple case of syncing the buffer with the backing store on pre_save? Regards, Peter >>> you can do that as well during the first access. >>> >> >> Kind of painful though to change this for a lot of existing devices. >> Its a reasonable policy for new devices, but can we just fix the >> init->bdrv_read() calls in place for the moment? > > Sure, but you asked for the policy and the policy is "only with good > reasons". ;-) > >>>> Pflash (pflash_cfi_01.c) is the device that is causing me interference >>>> here and it works exactly like this. If we make the bdrv_read() aio >>>> though, how do you ensure that it has completed before the guest talks >>>> to the device? Will this just happen at the end of machine_init >>>> anyways? Can we put a one liner in the machine init framework that >>>> pumps all AIO events then just mass covert all these bdrv_reads (in >>>> init functions) to bdrv_aio_read with a nop completion callback? >>> >>> The initialisation function of the device can wait at its end for all >>> AIOs to return. >> >> You lose the performance increase discussed below however, as instead >> of the init function returning to continue on with the machine init, >> you block on disk IO. > > Do you think it really matters? I guess it might if you have two such > devices that take each a second or so to load, but otherwise there isn't > much to parallelise. > > Kevin