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 (borntrae...@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.
