From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42883) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLCqh-0008R8-I6 for qemu-devel@nongnu.org; Mon, 18 Jan 2016 11:40:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLCqc-0005tv-Fo for qemu-devel@nongnu.org; Mon, 18 Jan 2016 11:40:11 -0500 Received: from e06smtp16.uk.ibm.com ([195.75.94.112]:58661) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLCqc-0005tD-6f for qemu-devel@nongnu.org; Mon, 18 Jan 2016 11:40:06 -0500 Received: from localhost by e06smtp16.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 18 Jan 2016 16:40:04 -0000 Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 8EC2C1B0805F for ; Mon, 18 Jan 2016 16:40:04 +0000 (GMT) Received: from d06av07.portsmouth.uk.ibm.com (d06av07.portsmouth.uk.ibm.com [9.149.37.248]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u0IGe0m95243246 for ; Mon, 18 Jan 2016 16:40:00 GMT Received: from d06av07.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av07.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u0IGe0Q1018942 for ; Mon, 18 Jan 2016 11:40:00 -0500 From: Sascha Silbe In-Reply-To: <20160115120143.GB2432@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> Date: Mon, 18 Jan 2016 17:40:09 +0100 Message-ID: <87twmaj0ye.fsf@oc4731375738.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 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: "Dr. David Alan Gilbert" Cc: amit.shah@redhat.com, Cornelia Huck , quintela@redhat.com, qemu-devel@nongnu.org, mst@redhat.com Dear David, "Dr. David Alan Gilbert" 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: 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 (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 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 --=20 Softwareentwicklung Sascha Silbe, Niederhofenstra=C3=9Fe 5/1, 71229 Leonberg https://se-silbe.de/ USt-IdNr. DE281696641