From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59040) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c4Rta-0006Fb-Pq for qemu-devel@nongnu.org; Wed, 09 Nov 2016 07:22:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c4RtZ-0001kW-Fe for qemu-devel@nongnu.org; Wed, 09 Nov 2016 07:22:26 -0500 Date: Wed, 9 Nov 2016 23:19:11 +1100 From: David Gibson Message-ID: <20161109121911.GH427@umbus.fritz.box> References: <1478663122-29357-1-git-send-email-david@gibson.dropbear.id.au> <8711c844-ccae-a4ef-3c42-eeda73cee5e2@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Yia77v5a8fyVHJSl" Content-Disposition: inline In-Reply-To: <8711c844-ccae-a4ef-3c42-eeda73cee5e2@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH] spapr: Fix migration of PCI host bridges from qemu-2.7 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: mdroth@linux.vnet.ibm.com, agraf@suse.de, thuth@redhat.com, lvivier@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org --Yia77v5a8fyVHJSl Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 09, 2016 at 04:14:25PM +1100, Alexey Kardashevskiy wrote: > On 09/11/16 14:45, David Gibson wrote: > > daa2369 "spapr_pci: Add a 64-bit MMIO window" subtly broke migration fr= om > > qemu-2.7 to the current version. It split the device's MMIO window into > > two pieces for 32-bit and 64-bit MMIO. > >=20 > > The patch included backwards compatibility code to convert the old prop= erty > > into the new format. However, the property value was also transferred = in > > the migration stream and compared with a (probably unwise) VMSTATE_EQUA= L. > > So, the "raw" value from 2.7 is compared to the new style converted val= ue > > from (pre-)2.8 giving a mismatch and migration failure. > >=20 > > Although it would be technically possible to fix this in a way allowing > > backwards migration, that would leave an ugly legacy around indefinitel= y. > > This patch takes the simpler approach of bumping the migration version, > > dropping the unwise VMSTATE_EQUAL (and some equally unwise ones around = it) > > and ignoring them on an incoming migration. > >=20 > > Signed-off-by: David Gibson > > --- > > hw/ppc/spapr_pci.c | 17 +++++++++++------ > > 1 file changed, 11 insertions(+), 6 deletions(-) > >=20 > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > > index 7cde30e..7f1cc29 100644 > > --- a/hw/ppc/spapr_pci.c > > +++ b/hw/ppc/spapr_pci.c > > @@ -1658,19 +1658,24 @@ static int spapr_pci_post_load(void *opaque, in= t version_id) > > return 0; > > } > > =20 > > +static bool version_before_3(void *opaque, int version_id) > > +{ > > + return version_id < 3; > > +} > > + > > static const VMStateDescription vmstate_spapr_pci =3D { > > .name =3D "spapr_pci", > > - .version_id =3D 2, > > + .version_id =3D 3, > > .minimum_version_id =3D 2, > > .pre_save =3D spapr_pci_pre_save, > > .post_load =3D spapr_pci_post_load, > > .fields =3D (VMStateField[]) { > > VMSTATE_UINT64_EQUAL(buid, sPAPRPHBState), >=20 >=20 > You could probably go one step further and get rid of @buid as well. I thought about it. buid at least is specified state that's vanishingly unlikely to change or disappear from the device. It also does serve to make sure that QOM instance matching - which is always a bit black magicy to me - is lining things up correctly. >=20 > Nevertheless, this works, >=20 > Reviewed-by: Alexey Kardashevskiy >=20 >=20 >=20 >=20 > > - VMSTATE_UINT32_EQUAL(dma_liobn[0], sPAPRPHBState), > > - VMSTATE_UINT64_EQUAL(mem_win_addr, sPAPRPHBState), > > - VMSTATE_UINT64_EQUAL(mem_win_size, sPAPRPHBState), > > - VMSTATE_UINT64_EQUAL(io_win_addr, sPAPRPHBState), > > - VMSTATE_UINT64_EQUAL(io_win_size, sPAPRPHBState), > > + VMSTATE_UNUSED_TEST(version_before_3, sizeof(uint32_t) /* dma_= liobn[0] */ > > + + sizeof(uint64_t) /* mem_win_addr */ > > + + sizeof(uint64_t) /* mem_win_size */ > > + + sizeof(uint64_t) /* io_win_addr */ > > + + sizeof(uint64_t) /* io_win_size */), > > VMSTATE_STRUCT_ARRAY(lsi_table, sPAPRPHBState, PCI_NUM_PINS, 0, > > vmstate_spapr_pci_lsi, struct spapr_pci_l= si), > > VMSTATE_INT32(msi_devs_num, sPAPRPHBState), > >=20 >=20 >=20 --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --Yia77v5a8fyVHJSl Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYIxQ8AAoJEGw4ysog2bOSJYsQAMgV7rjbNEuKUJoW/lzBM90P tFNB8Hf+nRTjI+R9m34kB1CaXnrIJL94vMd58pTOahSZQyJguPl66wMGzBp+COXA uJ7zWx4ynae2JrmPcLynVgJI40AZbKab71mhTG3KPZ1kUCkEncqedf40jH38eAfe u3GSMK2xkW176jGtrEDQTD2TPcaMUZjGyc3gOBGhOvkDamsdscoz5jZpUHUwwJLs 71sBQlQ2t79MmQW5EiVVh5ds5O7MI6UPUDrMJzTOMw98ADSIseS0DsMnWuCpB6Sw +w52djOkndF5JtaW0YwZ8drt55S20f6GdtLltzeCOgnRGd6k3/dyl7a9ll7hUgbB v+5ffEiphNZdn4sRU7esjMPxd4hJq1Yzbac4Cu2PbBFW9LLJCYT0ECs3zfDD+4pX YaC+Vlt8/tKZK2K0veWPiYX2E412LcHkwyNap56WHkXmWj043IuAMnCc5Zf3k91h dDMYYNo1sr6c7xQpSbuBKzP+ZetUntBUNUe7JachTGmYxezFyNlWiIzwsWCDqFtL OBga+9XHVOjIcQOc6qQxwvFM76MnX5COnifgsx/sE4fHTAKoP5EAuLEi+Sr1aY4X H45IzoIyuBN6uRHVGRTwRs1+ezL5AUsiBoMnZJJJsgQ0XjYFsuDNIuSc7rL9PjMf iJh4Xl8w4wotghLLRXmd =fh+m -----END PGP SIGNATURE----- --Yia77v5a8fyVHJSl--