Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, May 15, 2014 at 11:20:18AM +0200, Andreas Färber wrote:
> Am 15.05.2014 09:04, schrieb Greg Kurz:
> > On Thu, 15 May 2014 12:16:35 +0530
> > Amit Shah <amit.shah@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?

Backwards and forwards migration need to be supported,
customers told us repeatedly. So some downstreams support this
and not supporting it upstream just means downstreams need
to do their own thing.

As importantly, ping-pong migration is the only
reliable way to stress migration.

So if we want to test cross-version we need it to work
both way.

Finally, the real issue and difficulty with cross-version migration is
making VM behave in a backwards compatible way.  Serializing in a
compatible way is a trivial problem, or would be if the code wasn't a
mess :) Once you do the hard part, breaking migration because of the
trivial serialization issue is just silly.  And special-casing forward
migration does not make code simpler, it really only leads to
proliferation of hacks and lack of symmetry.

So yes it's a useful feature, and no not supporting it does
not help anyway.


> >>>>> 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.
> 
> >>>> 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?
> 
> Would touch on Stefan's alias properties for anything but virtio-mmio.
> 
> Regards,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg