[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put u
From: |
Sascha Silbe |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use |
Date: |
Mon, 18 Jan 2016 17:40:09 +0100 |
User-agent: |
Notmuch/0.19+1~g6b3e223 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu) |
Dear David,
"Dr. David Alan Gilbert" <address@hidden> 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 <address@hidden>
(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 <address@hidden>
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
- [Qemu-devel] [PATCH 1/2] Add VMSTATE_STRUCT_VARRAY_KNOWN, Dr. David Alan Gilbert (git), 2016/01/06
- [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Dr. David Alan Gilbert (git), 2016/01/06
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Michael S. Tsirkin, 2016/01/07
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Amit Shah, 2016/01/08
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Sascha Silbe, 2016/01/14
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Dr. David Alan Gilbert, 2016/01/15
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Dr. David Alan Gilbert, 2016/01/15
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Cornelia Huck, 2016/01/18
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use,
Sascha Silbe <=
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Dr. David Alan Gilbert, 2016/01/19
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Sascha Silbe, 2016/01/21
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Cornelia Huck, 2016/01/29
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Dr. David Alan Gilbert, 2016/01/29
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Sascha Silbe, 2016/01/18
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use, Dr. David Alan Gilbert, 2016/01/19
- [Qemu-devel] [PATCH qemu] migration/vmstate: document VMStateFlags, Sascha Silbe, 2016/01/21
Re: [Qemu-devel] [PATCH 1/2] Add VMSTATE_STRUCT_VARRAY_KNOWN, Amit Shah, 2016/01/08