On Thu, 15 May 2014 13:12:12 +0300 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Thu, May 15, 2014 at 12:08:26PM +0200, Greg Kurz wrote: > > On Thu, 15 May 2014 11:20:18 +0200 > > Andreas Färber <afaer...@suse.de> wrote: > > > Am 15.05.2014 09:04, schrieb Greg Kurz: > > > > On Thu, 15 May 2014 12:16:35 +0530 > > > > Amit Shah <amit.s...@redhat.com> wrote: > > > >> On (Thu) 15 May 2014 [09:23:51], Michael S. Tsirkin wrote: > > > >>> On Thu, May 15, 2014 at 11:34:25AM +0530, Amit Shah wrote: > > > >>>> On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote: > > > >>>>> Since each virtio device is streamed in its own section, the idea > > > >>>>> is to > > > >>>>> stream subsections between the end of the device section and the > > > >>>>> start > > > >>>>> of the next sections. This allows an older QEMU to complain and exit > > > >>>>> when fed with subsections: > > > >>>>> > > > >>>>> Unknown savevm section type 5 > > > >>>>> Error -22 while loading VM state > > > >>>> > > > >>>> Please make this configurable -- either via configure or device > > > >>>> properties. That avoids having to break existing configurations that > > > >>>> work without this patch. > > > > > > Since backwards migration is not supported upstream, wouldn't it be > > > easiest to just add support for the subsection marker and skipping to > > > the end of section in that downstream? > > > > > > > Not sure I understand well... Do you suggest to stream the markers first, > > then the device, then the subsections ? And then there would be a way > > we can have the subsections restored before the device ? > > > > > >>>>> All users of virtio_load()/virtio_save() need to be patched because > > > >>>>> the > > > >>>>> subsections are streamed AFTER the device itself. > > > > > > IMO this is calling for inversion of control - i.e. let virtio devices > > > call generic load/save functions that then dispatch to device-specific > > > code and let us add common stuff in a central place without forgetting > > > to add calls in some new device. > > > > > > > That makes a lot of sense. > > > > > >>>> Since all have the same fixup, I'm wondering if a new section can be > > > >>>> added to the virtio-bus itself, which gets propagated to all devices > > > >>>> upon load in the dest. > > > >>> > > > >>> This calls for a way for devices to inherit properties from the bus, > > > >>> which doesn't exist ATM. > > > >>> Fine but let's not hold up this patchset because of this. > > > >> > > > >> No, only suggestion is to add a migration section in the bus, and then > > > >> it's easier to do this in the post-migrate functions for each device > > > >> -- so only one new section gets introduced instead of all devices > > > >> being modified to send a new subsection. > > > >> > > > > > > > > The main problem I see is that virtio sucks: as you see in patch 8, we > > > > have > > > > to be careful not to call vring or virtqueue stuff before the device > > > > knows > > > > its endianness or it breaks... I need to study how the virtio-bus gets > > > > migrated to ensure the endian section is streamed before the devices. > > > > > > There is no ordering guarantee. The state needs to be migrated in the > > > device or bus where it sits, if post-load processing is required; i.e., > > > if it's in VirtIODevice then something like this series, if it were on > > > VirtioBus exclusively (device asking bus for its endianness each time > > > and does not do post-load stuff) then endianness could be migrated as a > > > new bus section. Not sure if that would help the "broken" state though? > > > > > > > IIRW the "broken" state was proposed as a per-device property... > > > > Fam, > > > > Do you have plans about the "broken" property ? Is it still needed ? > > > > > Would touch on Stefan's alias properties for anything but virtio-mmio. > > > > > > > OMG... maybe I should hold on then. > > No need to wait imho. > Can this be made even simpler - call this stuff > from virtio_save/virtio_load? > Andreas already suggested this inversion of control. > Why not? > No reason indeed. I'll rewrite the code that way ! :) > > > > Regards, > > > Andreas > > > > > > > Thanks ! > > > > -- > > Gregory Kurz kurzg...@fr.ibm.com > > gk...@linux.vnet.ibm.com > > Software Engineer @ IBM/Meiosys http://www.ibm.com > > Tel +33 (0)562 165 496 > > > > "Anarchy is about taking complete responsibility for yourself." > > Alan Moore. > Thnaks ! -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 "Anarchy is about taking complete responsibility for yourself." Alan Moore.