From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60827) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WksnP-0003yK-9h for qemu-devel@nongnu.org; Thu, 15 May 2014 06:21:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WksnH-0004LW-46 for qemu-devel@nongnu.org; Thu, 15 May 2014 06:21:51 -0400 Received: from e06smtp15.uk.ibm.com ([195.75.94.111]:33268) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WksnG-0004LF-SU for qemu-devel@nongnu.org; Thu, 15 May 2014 06:21:43 -0400 Received: from /spool/local by e06smtp15.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 15 May 2014 11:21:41 +0100 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id 5ED3F17D8068 for ; Thu, 15 May 2014 11:22:47 +0100 (BST) Received: from d06av04.portsmouth.uk.ibm.com (d06av04.portsmouth.uk.ibm.com [9.149.37.216]) by b06cxnps3075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s4FALcIK47054908 for ; Thu, 15 May 2014 10:21:38 GMT Received: from d06av04.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av04.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s4FALbTQ003604 for ; Thu, 15 May 2014 04:21:38 -0600 Date: Thu, 15 May 2014 12:21:36 +0200 From: Greg Kurz Message-ID: <20140515122136.02e4388f@bahia.local> In-Reply-To: <20140515101212.GA22512@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> <20140515120826.050cf6f8@bahia.local> <20140515101212.GA22512@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 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: "Michael S. Tsirkin" Cc: Kevin Wolf , Fam Zheng , Stefan Hajnoczi , Juan Quintela , Alexander Graf , qemu-devel@nongnu.org, Anthony Liguori , Amit Shah , Paolo Bonzini , Andreas =?UTF-8?B?RsOkcmJlcg==?= On Thu, 15 May 2014 13:12:12 +0300 "Michael S. Tsirkin" 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=C3=A4rber 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 id= ea is to > > > >>>>> stream subsections between the end of the device section and th= e 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. > > >=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? > > >=20 > >=20 > > Not sure I understand well... Do you suggest to stream the markers firs= t, > > then the device, then the subsections ? And then there would be a way > > we can have the subsections restored before the device ? > >=20 > > > >>>>> All users of virtio_load()/virtio_save() need to be patched bec= ause 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 > >=20 > > That makes a lot of sense. > >=20 > > > >>>> Since all have the same fixup, I'm wondering if a new section ca= n be > > > >>>> added to the virtio-bus itself, which gets propagated to all dev= ices > > > >>>> upon load in the dest. > > > >>> > > > >>> This calls for a way for devices to inherit properties from the b= us, > > > >>> 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 devi= ce > > > >> -- 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,= we have > > > > to be careful not to call vring or virtqueue stuff before the devic= e knows > > > > its endianness or it breaks... I need to study how the virtio-bus g= ets > > > > migrated to ensure the endian section is streamed before the device= s. > > >=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 thoug= h? > > >=20 > >=20 > > IIRW the "broken" state was proposed as a per-device property... > >=20 > > Fam, > >=20 > > Do you have plans about the "broken" property ? Is it still needed ? > >=20 > > > Would touch on Stefan's alias properties for anything but virtio-mmio. > > >=20 > >=20 > > OMG... maybe I should hold on then. >=20 > No need to wait imho. > Can this be made even simpler - call this stuff > from virtio_save/virtio_load? >=20 Andreas already suggested this inversion of control. > Why not? >=20 No reason indeed. I'll rewrite the code that way ! :) >=20 > > > Regards, > > > Andreas > > >=20 > >=20 > > Thanks ! > >=20 > > --=20 > > Gregory Kurz kurzgreg@fr.ibm.com > > gkurz@linux.vnet.ibm.c= om > > Software Engineer @ IBM/Meiosys http://www.ibm.com > > Tel +33 (0)562 165 496 > >=20 > > "Anarchy is about taking complete responsibility for yourself." > > Alan Moore. >=20 Thnaks ! --=20 Gregory Kurz kurzgreg@fr.ibm.com gkurz@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.