From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55647) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bAsET-0001P0-FM for qemu-devel@nongnu.org; Thu, 09 Jun 2016 01:10:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bAsEO-00055R-3c for qemu-devel@nongnu.org; Thu, 09 Jun 2016 01:10:16 -0400 Date: Thu, 9 Jun 2016 14:35:03 +1000 From: David Gibson Message-ID: <20160609043503.GK9226@voom.fritz.box> References: <1465427217-5551-1-git-send-email-duanj@linux.vnet.ibm.com> <1465427217-5551-2-git-send-email-duanj@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="bYB/dvygrOI2NuBf" Content-Disposition: inline In-Reply-To: <1465427217-5551-2-git-send-email-duanj@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [QEMU RFC PATCH v4 1/6] migration: alternative way to set instance_id in SaveStateEntry List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jianjun Duan Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, dmitry@daynix.com, peter.maydell@linaro.org, kraxel@redhat.com, mst@redhat.com, pbonzini@redhat.com, veroniabahaa@gmail.com, quintela@redhat.com, amit.shah@redhat.com, mreitz@redhat.com, kwolf@redhat.com, rth@twiddle.net, aurelien@aurel32.net, leon.alrae@imgtec.com, blauwirbel@gmail.com, mark.cave-ayland@ilande.co.uk, mdroth@linux.vnet.ibm.com --bYB/dvygrOI2NuBf Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 08, 2016 at 04:06:52PM -0700, Jianjun Duan wrote: > In QOM(QEMU Object Model) migrated objects are identified with instance_id > which is calculated automatically using their path in the QOM composition > tree. For some objects, this path could change from source to target in > migration. To migrate such objects, we need to make sure the instance_id = does > not change from source to target. We add a hook in DeviceClass to do cust= omized > instance_id calculation in such cases. >=20 > As a result, in these cases compat will not be set in the concerned > SaveStateEntry. This will prevent the inconsistent idstr to be sent over = in > migration. We could have set alias_id in a similar way. But that will be > overloading the purpose of alias_id. >=20 > The first application will be setting instance_id for DRC using its unique > index. Doing this makes the instance_id of DRC to be consistent across mi= gration > and supports flexible management of DRC objects in migration. >=20 > Signed-off-by: Jianjun Duan Ah, ok. I misunderstood your earlier comment - I didn't realise you needed a change to the migration core in order to set the instance ids. That said, I think being able to customize the instance ids could give better ability to future-proof migration data, at least for certain buses and devices. Juan, Dave, what do you think? > --- > include/hw/qdev-core.h | 6 ++++++ > migration/savevm.c | 20 ++++++++++++++++++-- > 2 files changed, 24 insertions(+), 2 deletions(-) >=20 > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 1ce02b2..28173a7 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -139,6 +139,12 @@ typedef struct DeviceClass { > qdev_initfn init; /* TODO remove, once users are converted to realiz= e */ > qdev_event exit; /* TODO remove, once users are converted to unreali= ze */ > const char *bus_type; > + > + /* When this field is set, qemu will use it to get an unique instanc= e_id > + * instead of calculating an auto idstr and instanc_id for the relev= ant > + * SaveStateEntry > + */ > + int (*dev_get_instance_id)(DeviceState *dev); > } DeviceClass; > =20 > typedef struct NamedGPIOList NamedGPIOList; > diff --git a/migration/savevm.c b/migration/savevm.c > index 6c21231..f7e7c68 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -494,6 +494,11 @@ int register_savevm_live(DeviceState *dev, > void *opaque) > { > SaveStateEntry *se; > + /* when it is a device and it provides a way to get instance_id, > + * we will use it and skip setting idstr and compat. > + */ > + bool flag =3D (dev !=3D NULL) && > + (DEVICE_GET_CLASS(dev)->dev_get_instance_id !=3D NULL); > =20 > se =3D g_new0(SaveStateEntry, 1); > se->version_id =3D version_id; > @@ -506,7 +511,7 @@ int register_savevm_live(DeviceState *dev, > se->is_ram =3D 1; > } > =20 > - if (dev) { > + if (dev && !flag) { > char *id =3D qdev_get_dev_path(dev); > if (id) { > pstrcpy(se->idstr, sizeof(se->idstr), id); > @@ -522,6 +527,9 @@ int register_savevm_live(DeviceState *dev, > } > pstrcat(se->idstr, sizeof(se->idstr), idstr); > =20 > + if (flag) { > + instance_id =3D DEVICE_GET_CLASS(dev)->dev_get_instance_id(dev); > + } > if (instance_id =3D=3D -1) { > se->instance_id =3D calculate_new_instance_id(se->idstr); > } else { > @@ -579,6 +587,11 @@ int vmstate_register_with_alias_id(DeviceState *dev,= int instance_id, > int required_for_version) > { > SaveStateEntry *se; > + /* when it is a device and it provides a way to get instance_id, > + * we will use it and skip setting idstr and compat. > + */ > + bool flag =3D (dev !=3D NULL) && > + (DEVICE_GET_CLASS(dev)->dev_get_instance_id !=3D NULL); > =20 > /* If this triggers, alias support can be dropped for the vmsd. */ > assert(alias_id =3D=3D -1 || required_for_version >=3D vmsd->minimum= _version_id); > @@ -590,7 +603,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, = int instance_id, > se->vmsd =3D vmsd; > se->alias_id =3D alias_id; > =20 > - if (dev) { > + if (dev && !flag) { > char *id =3D qdev_get_dev_path(dev); > if (id) { > pstrcpy(se->idstr, sizeof(se->idstr), id); > @@ -606,6 +619,9 @@ int vmstate_register_with_alias_id(DeviceState *dev, = int instance_id, > } > pstrcat(se->idstr, sizeof(se->idstr), vmsd->name); > =20 > + if (flag) { > + instance_id =3D DEVICE_GET_CLASS(dev)->dev_get_instance_id(dev); > + } > if (instance_id =3D=3D -1) { > se->instance_id =3D calculate_new_instance_id(se->idstr); > } else { --=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 --bYB/dvygrOI2NuBf Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXWPH3AAoJEGw4ysog2bOSYwgQAIjBOgSdk2McJXDPnYmcOAf5 HsXTyMW9zpv50l7IvpHLvX5sKpJnNj3Zmo+C9npBZjssKMYx5p+ippZG7aqn8faY +v5GCp/igHBv/25rCY6LMrRqqZ8iadkCNVAI1bS/B9Ox34Fes3KFEHm/LZsLc2lP e3yHnuCIyRkMArzeBMNVnPmi9phwjcJLs6X/xelDkW8OByIKJxDhmgXNXDpH1zdr esHIdWBJXTZHF656vRFHLPz3T4VwEVfbs98FLsVZP/c7tw/x/E6ksfhLba5Wnv1d zZ6bS1ZiPDOJ4MHpvBcIVh+bYq1KNKaTB8t3yUBVnIpcQz7q8rvq85GTbZKA/B96 JOLSK1RuN3coLzO8hN204bwFv/IuREKO+Kootsr6pKC8ngDsehnKgExw8R9RaY2B VK3RBypcFownkN2j5nJkGDQ5AvcegWCE2QjalByoJIrtjEC6AWQfIM3gAYl3Bx9q llXgJjPUGexGDL9uVqvbe3Bz5uE/CGdJQPSf1opbDLmb9YOFvRwDhaqna09Rlyqw coXG72Hxs3SPBZopnT++yP0TvrgLmm9/vseadCT/yQKDHhfku9R7L8EewxmuM12l JtS49Dft+dvdZAM57+Vk3Tok7S93VW2A6LtgNy6WZDX8DZ4whSwzXlYAKRXgz1/A 3kjULNHIUBotQhUVIZGw =BNWa -----END PGP SIGNATURE----- --bYB/dvygrOI2NuBf--