From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53590) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLV5u-0003wp-Ft for qemu-devel@nongnu.org; Tue, 19 Jan 2016 07:09:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLV5p-0008BO-R7 for qemu-devel@nongnu.org; Tue, 19 Jan 2016 07:09:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52558) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLV5p-0008BF-GS for qemu-devel@nongnu.org; Tue, 19 Jan 2016 07:09:01 -0500 Date: Tue, 19 Jan 2016 12:08:57 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20160119120856.GD2398@work-vm> References: <1452083019-15141-1-git-send-email-dgilbert@redhat.com> <1452083019-15141-2-git-send-email-dgilbert@redhat.com> <871t9jhnfs.fsf@oc4731375738.ibm.com> <20160115092419.GA2432@work-vm> <20160115120143.GB2432@work-vm> <87twmaj0ye.fsf@oc4731375738.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <87twmaj0ye.fsf@oc4731375738.ibm.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2] migration/virtio: Remove simple .get/.put use List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sascha Silbe Cc: amit.shah@redhat.com, Cornelia Huck , quintela@redhat.com, qemu-devel@nongnu.org, mst@redhat.com * Sascha Silbe (silbe@linux.vnet.ibm.com) wrote: > Dear David, >=20 > "Dr. David Alan Gilbert" writes: >=20 > > 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. >=20 > 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 an= y > knobs to set disable-modern/disable, I had to configure the devices > manually: >=20 > > > > > > > > >=20 > With the above, migration fails on x86_64, too. Thank you! With that example I used: (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: >=20 > Tested-By: Sascha Silbe 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: >=20 > Reviewed-By: Sascha Silbe >=20 > A commit message explaining what's going on would be nice, though. Mayb= e > something along these lines: >=20 > migration/virtio: fix migration of VirtQueues >=20 > 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 an= d > potentially sending random data instead of the VirtQueue content. >=20 > 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. >=20 > (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 > --=20 > Softwareentwicklung Sascha Silbe, Niederhofenstra=DFe 5/1, 71229 Leonbe= rg > https://se-silbe.de/ > USt-IdNr. DE281696641 >=20 -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK