Dear David,
"Dr. David Alan Gilbert" <[email protected]> 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. Your patch fixes the
basic save/resume test on both x86_64 and s390x, so:
Tested-By: Sascha Silbe <[email protected]>
(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.)
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 <[email protected]>
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).
Sascha
--
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641
