From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50132) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WksLx-0007AU-7A for qemu-devel@nongnu.org; Thu, 15 May 2014 05:53:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WksLr-0002jP-2h for qemu-devel@nongnu.org; Thu, 15 May 2014 05:53:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:8686) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WksLq-0002jF-R7 for qemu-devel@nongnu.org; Thu, 15 May 2014 05:53:23 -0400 Date: Thu, 15 May 2014 12:52:08 +0300 From: "Michael S. Tsirkin" Message-ID: <20140515095208.GB16405@redhat.com> References: <20140514154130.10746.1412.stgit@bahia.local> <20140514154137.10746.94708.stgit@bahia.local> <20140515060425.GA31192@grmbl.mre> <20140515062351.GB14456@redhat.com> <20140515064635.GB31192@grmbl.mre> <20140515090449.2db0cbe0@bahia.local> <537486D2.1060609@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <537486D2.1060609@suse.de> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: Kevin Wolf , Fam Zheng , Stefan Hajnoczi , Juan Quintela , Alexander Graf , qemu-devel@nongnu.org, Anthony Liguori , Amit Shah , Paolo Bonzini , Greg Kurz On Thu, May 15, 2014 at 11:20:18AM +0200, Andreas F=E4rber wrote: > Am 15.05.2014 09:04, schrieb Greg Kurz: > > On Thu, 15 May 2014 12:16:35 +0530 > > Amit Shah 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 e= xit > >>>>> 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 t= hat > >>>> work without this patch. >=20 > 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 becau= se the > >>>>> subsections are streamed AFTER the device itself. >=20 > 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. >=20 > >>>> 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 devic= es > >>>> 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 th= en > >> 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. > >> > >=20 > > The main problem I see is that virtio sucks: as you see in patch 8, w= e 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 get= s > > migrated to ensure the endian section is streamed before the devices. >=20 > 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? >=20 > Would touch on Stefan's alias properties for anything but virtio-mmio. >=20 > Regards, > Andreas >=20 > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrn= berg