[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: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use |
Date: |
Tue, 19 Jan 2016 12:08:57 +0000 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
* Sascha Silbe (address@hidden) wrote:
> 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.
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 <address@hidden>
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 <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).
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 / address@hidden / Manchester, UK
- [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, 2016/01/18
- Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use,
Dr. David Alan Gilbert <=
- 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