Dear David,
"Dr. David Alan Gilbert (git)" <dgilb...@redhat.com> writes:
> The 'virtqueue_state' and 'ringsize' can be saved using VMSTATE
> macros rather than hand coded .get/.put
[...]
> @@ -1161,44 +1143,20 @@ static const VMStateDescription
> vmstate_virtio_virtqueues = {
> .minimum_version_id = 1,
> .needed = &virtio_virtqueue_needed,
> .fields = (VMStateField[]) {
> - {
> - .name = "virtqueues",
> - .version_id = 0,
> - .field_exists = NULL,
> - .size = 0,
> - .info = &vmstate_info_virtqueue,
> - .flags = VMS_SINGLE,
> - .offset = 0,
> - },
> + VMSTATE_STRUCT_VARRAY_KNOWN(vq, struct VirtIODevice,
> VIRTIO_QUEUE_MAX,
> + 0, vmstate_virtqueue, VirtQueue),
> VMSTATE_END_OF_LIST()
> }
> };
This completely breaks migration (including virsh save / restore) on
s390x. It appears to work on x86_64 with a trivial test case, but that's
probably pure luck and I'd expect a more extensive test case to show
guest state corruption on migration.
After staring at the code for several hours (this whole VMSTATE_* stuff
is severely underdocumented), I think I've finally understood what's
going on.
Given the VMS_STRUCT|VMS_ARRAY field flags set by
VMSTATE_STRUCT_VARRAY_KNOWN, vmstate_save_state() expects an array of
fixed size embedded in the struct. However, VirtIODevice.vq is a pointer
to an allocated array. Adding the VMS_POINTER flag looks like it could
do the trick and indeed allows my trivial test case to complete on s390x
again. (No further testing was done).
I won't be sending a fix for this for now as I don't understand what the
naming conventions for VMSTATE_* are (both VMSTATE_ARRAY* and
VMSTATE_VARRAY* can use VMS_VARRAY, something with and sometimes without
VMS_POINTER). Should VMSTATE_STRUCT_VARRAY_KNOWN be fixed to include
VMS_POINTER or do you need to introduce another macro
VMSTATE_STRUCT_VARRAY_POINTER_KNOWN?
PS: Won't your patch break migration between different qemu versions? I
don't see any compat code and you're changing at least some field names
(e.g. "virtqueues" vs. "vq").
Sascha
--
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641
