* Sascha Silbe (si...@linux.vnet.ibm.com) wrote: > Dear David, > > "Dr. David Alan Gilbert" <dgilb...@redhat.com> writes: > > > Can you try this and let me know if it fixes it for you; I've > > still not managed to persuade x86-64 to fail. > > With Conny's hint re. virtio-1 (thanks!) I managed to make it fail on > x86_64, too. I'm using libvirt for testing (virDomainSave() / > virDomainRestore() use the qemu migration API internally, allowing for > easy testing of migration code). Since current libvirt doesn't offer any > knobs to set disable-modern/disable, I had to configure the devices > manually: > > <qemu:commandline> > <qemu:arg value='-device'/> > <qemu:arg > value='virtio-serial-pci,id=virtio-serial0,bus=pci.0,disable-modern=off,disable-legacy=on'/> > <qemu:arg value='-chardev'/> > <qemu:arg value='file,id=charconsole0,path=/tmp/test0.log'/> > <qemu:arg value='-device'/> > <qemu:arg value='virtconsole,chardev=charconsole0'/> > </qemu:commandline> > > With the above, migration fails on x86_64, too.
Thank you! With that example I used: <qemu:commandline> <qemu:arg value='--global'/> <qemu:arg value='virtio-pci.disable-modern=off'/> <qemu:arg value='--global'/> <qemu:arg value='virtio-pci.disable-legacy=on'/> <qemu:arg value='--global'/> <qemu:arg value='virtio-pci.migrate-extra=on'/> </qemu:commandline> (I had to use ide disk, my guest didn't like virtio-disk with that; but still had virtio-net and virtio-serial). > basic save/resume test on both x86_64 and s390x, so: > > Tested-By: Sascha Silbe <si...@linux.vnet.ibm.com> Thanks. > (I currently don't have a more extensive test for migration; in > particular nothing that puts the guest in a pre-defined state and > compares on-the-wire data across qemu versions.) No, I don't think anyone does; too many fields change depending on timing etc - and the structure of the migration stream is too arbitrary to pull apart [One thing I'm trying to fix by avoiding .get/.put !]. > I'm also confident by now that I'm having a reasonable grasp of this > particular aspect of the code, so for the actual code changes: > > Reviewed-By: Sascha Silbe <si...@linux.vnet.ibm.com> > > A commit message explaining what's going on would be nice, though. Maybe > something along these lines: > > migration/virtio: fix migration of VirtQueues > > Commit 50e5ae4d [migration/virtio: Remove simple .get/.put use] > refactored the virtio migration code to use the VMStateDescription API > instead of the previous custom VMStateInfo API. It relied on > VMSTATE_STRUCT_VARRAY_KNOWN, introduced by commit 2cf01486 [Add > VMSTATE_STRUCT_VARRAY_KNOWN]. This was described as being for "a > variable length array (i.e. _type *_field) but we know the > length". However it actually specified operation for arrays embedded in > the struct (i.e. _type _field[]) since it lacked the VMS_POINTER > flag. This caused offset calculation to be completely off, examining and > potentially sending random data instead of the VirtQueue content. > > Replace the otherwise unused VMSTATE_STRUCT_VARRAY_KNOWN with a > VMSTATE_STRUCT_VARRAY_POINTER_KNOWN that includes the VMS_POINTER flag > (so now actually doing what it advertises) and use it in the virtio > migration code. > > (Feel free to reuse any or all of this). Thanks I've reused a chunk of that; I'll post the fix soon. Thanks for your help on this. Dave > Sascha > -- > Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg > https://se-silbe.de/ > USt-IdNr. DE281696641 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK