From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51835) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yt9on-00055t-5f for qemu-devel@nongnu.org; Fri, 15 May 2015 03:14:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yt9oi-0001QV-PA for qemu-devel@nongnu.org; Fri, 15 May 2015 03:14:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39478) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yt9oi-0001Q5-HM for qemu-devel@nongnu.org; Fri, 15 May 2015 03:13:56 -0400 Date: Fri, 15 May 2015 09:13:46 +0200 From: "Michael S. Tsirkin" Message-ID: <20150515091307-mutt-send-email-mst@redhat.com> References: <20150513165438-mutt-send-email-mst@redhat.com> <20150513170335.2d662124.cornelia.huck@de.ibm.com> <20150513180005-mutt-send-email-mst@redhat.com> <55539E7C.90004@de.ibm.com> <20150513234516-mutt-send-email-mst@redhat.com> <55546945.4050400@de.ibm.com> <20150514112845-mutt-send-email-mst@redhat.com> <55547938.4020201@de.ibm.com> <20150514170035.GJ2576@work-vm> <55559B57.5020704@de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55559B57.5020704@de.ibm.com> Subject: Re: [Qemu-devel] [PATCH RFC 1/1] virtio: migrate config_vector List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger Cc: Cornelia Huck , "Dr. David Alan Gilbert" , jjherne@linux.vnet.ibm.com, qemu-devel@nongnu.org On Fri, May 15, 2015 at 09:08:07AM +0200, Christian Borntraeger wrote: > Am 14.05.2015 um 19:00 schrieb Dr. David Alan Gilbert: > > * Christian Borntraeger (borntraeger@de.ibm.com) wrote: > >> Am 14.05.2015 um 11:36 schrieb Michael S. Tsirkin: > >>> On Thu, May 14, 2015 at 11:22:13AM +0200, Christian Borntraeger wrote: > >>>> Am 13.05.2015 um 23:47 schrieb Michael S. Tsirkin: > >>>>> On Wed, May 13, 2015 at 08:57:00PM +0200, Christian Borntraeger wrote: > >>>>>> Am 13.05.2015 um 18:14 schrieb Michael S. Tsirkin: > >>>>>>>> - AFAICS, there's no easy way to add transport-specific subsections - > >>>>>>>> and simply adding config_vector in ccw would break compatibility > >>>>>>> > >>>>>>> subsections break compatibility too. The only way around that is to set > >>>>>>> a flag to skip migrating config_vector for old machine types. > >>>>>> > >>>>>> My main concern is about undetected compatibility issues. A subsection will > >>>>>> tell the user that something went wrong. What happens if we just add a new > >>>>>> qemu_put_byte in the stream. Will the savevm core always detect that we have > >>>>>> too many or not enough bytes? If yes, adding new stuff in the stream will > >>>>>> always be detected in some way as error we can go with just adding > >>>>>> qemu_put_be16/qemu_get_be16 in virtio_ccw_save_config/virtio_ccw_load_config. > >>>>>> Old/new QEMUs will then not be compatible - but thats probably ok as long as it > >>>>>> errors out. > >>>>>> > >>>>>> My understanding was that we do not have a guarentee that this will be > >>>>>> detected all the time and having random junk in some variables is a debugging > >>>>>> nightmare. Is that correct? > >>>>>> > >>>>>> > >>>>>> Christian > >>>>> > >>>>> It's not too bad - normally there's a bunch of strings that > >>>>> helps you find out what's going on. > >>>>> But if you really care about debuggability of migration streams, help move > >>>>> forward dgilbert's RFC that switched to a self-delimiting format. > >>>>> Just piling up random hacks in virtio seems like a wrong approach. > >>>>> > >>>> > >>>> Thats not my question. PLEASE try to understand my question. > >>>> I want a hard stop if migration changes in incompatible ways. > >>>> If adding a qemu_put_byte in virtio_ccw gets detected we can just fix > >>>> virtio_ccw AS YOU SUGGESTED. I just want to know if I can rely on that > >>>> or not. > >>>> > >>>> Christian > >>> > >>> I answered exactly this question but let me try to spell the answer > >>> out a bit more. > >>> > >>> There are three answers: > >>> 1. Yes, it's sure to get detected because everything gets shifted > >>> and then you get an unexpected string instead of next device name. > >> > >> Ok got it. So simply adding a qemu_put/get_byte will always fail the migration and we > >> can just fixup virtio-ccw.c at the cost of being not migrateable between versions before/after that change. > > > > Gahhh! No! Adding an extra byte into the stream causes random horrible failures > > that get very very confusing. Yes, it will probably fail, but how it will > > fail and what error you get is just guess work. > > (And note, it's strictly a 'probably fail' - if you happen to land with > > a zero byte where you expect the start of the next section the migration > > code will think it's the end of the migration stream and blissfully start > > the CPUs). > > As Conny is away today, I will drive the dicussion a bit further :-) > So we really want a feature that detects this change and prevents migration. > I think its totally fine to not be able to migrate between todays QEMUs and > a fixed version for s390 as there no supported environment today I am aware > of. What would be the preferred way to go? > a: Connies approach of a subsection that is only migrated if necessary > (config vector != 0xffff) > b: change virtio-ccw.c with put/get_be16 and make a new version of the > s390-ccw machine? The old version will set a property to not migrate the config > vector. (like Michaels 2nd suggestion) > c: ? c. Set old machine as non-migrateable. > Christian I prefer b or c.