From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50421) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yt9jE-0003Lt-6P for qemu-devel@nongnu.org; Fri, 15 May 2015 03:08:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yt9jA-0007WS-PH for qemu-devel@nongnu.org; Fri, 15 May 2015 03:08:16 -0400 Received: from e06smtp12.uk.ibm.com ([195.75.94.108]:33455) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yt9jA-0007Uu-En for qemu-devel@nongnu.org; Fri, 15 May 2015 03:08:12 -0400 Received: from /spool/local by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 15 May 2015 08:08:11 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 1011A2190068 for ; Fri, 15 May 2015 08:07:50 +0100 (BST) Received: from d06av03.portsmouth.uk.ibm.com (d06av03.portsmouth.uk.ibm.com [9.149.37.213]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t4F788EH57802802 for ; Fri, 15 May 2015 07:08:08 GMT Received: from d06av03.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av03.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t4F787Wd029189 for ; Fri, 15 May 2015 01:08:08 -0600 Message-ID: <55559B57.5020704@de.ibm.com> Date: Fri, 15 May 2015 09:08:07 +0200 From: Christian Borntraeger MIME-Version: 1.0 References: <1431528122-50960-1-git-send-email-cornelia.huck@de.ibm.com> <1431528122-50960-2-git-send-email-cornelia.huck@de.ibm.com> <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> In-Reply-To: <20150514170035.GJ2576@work-vm> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: "Dr. David Alan Gilbert" Cc: Cornelia Huck , qemu-devel@nongnu.org, jjherne@linux.vnet.ibm.com, "Michael S. Tsirkin" 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: ? Christian