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

[Andreas Färber]

Am 15.05.2014 09:04, schrieb Greg Kurz:
> On Thu, 15 May 2014 12:16:35 +0530
> Amit Shah <address@hidden> 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?

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