From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50399) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atSsR-0003dK-PY for qemu-devel@nongnu.org; Fri, 22 Apr 2016 00:39:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1atSsQ-0007IL-Gd for qemu-devel@nongnu.org; Fri, 22 Apr 2016 00:39:35 -0400 Date: Fri, 22 Apr 2016 14:25:38 +1000 From: David Gibson Message-ID: <20160422042538.GD15176@voom.fritz.box> References: <1460752385-13259-1-git-send-email-duanj@linux.vnet.ibm.com> <1460752385-13259-3-git-send-email-duanj@linux.vnet.ibm.com> <20160420043252.GF1133@voom> <571907FC.8030303@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="zbGR4y+acU1DwHSi" Content-Disposition: inline In-Reply-To: <571907FC.8030303@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH 2/5] Migration: Defined VMStateDescription struct for spapr_drc List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jianjun Duan Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, mdroth@linux.vnet.ibm.com --zbGR4y+acU1DwHSi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Apr 21, 2016 at 10:03:56AM -0700, Jianjun Duan wrote: >=20 >=20 > On 04/19/2016 09:32 PM, David Gibson wrote: > >On Fri, Apr 15, 2016 at 01:33:02PM -0700, Jianjun Duan wrote: > >>To manage hotplug/unplug of dynamic resources such as PCI cards, > >>memory, and CPU on sPAPR guests, a firmware abstraction known as > >>a Dynamic Resource Connector (DRC) is used to assign a particular > >>dynamic resource to the guest, and provide an interface for the > >>guest to manage configuration/removal of the resource associated > >>with it. > >> > >>To migrate the hotplugged resources in migration, the > >>associated DRC state need be migrated. To migrate the DRC state, > >>we defined the VMStateDescription struct for spapr_drc to enable > >>the transmission of spapr_drc state in migration. > >> > >>Not all the elements in the DRC state are migrated. Only those > >>ones modifiable by guest actions or device add/remove operation > >>are migrated. > >> > >>Signed-off-by: Jianjun Duan > >Urgh. It would be really nice if we could avoid this and instead > >calculate these states from other information. I hate migrating > >what's essentially transitory state if we can possibly avoid it - is > >there any way to defer or retry hotplug operations to make this > >unnececessary? > > > >Even if we have to migrate some state here, I'm a bit dubious about > >whether directly migrating the PAPR indicator states is the best way. > >It does have the advantage of having a spec, on the other hand the > >PAPR indicators are really weird and hard to understand the meaning > >of. > I don't think we can avoid this. I would think migrating the machine state > is > actually a clean approach, as you said it does have PAPR spec. Hm, ok. The next question, though, is what's the absolute minimum of state we can transfer. The indicator states are described by PAPR, but the configured and awaiting_release states are internal. Not thinking carefully about the data model of what we migrate is a recipe for future migration problems. > >>--- > >> hw/ppc/spapr_drc.c | 15 +++++++++++++++ > >> 1 file changed, 15 insertions(+) > >> > >>diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c > >>index 3173940..5f7a25f 100644 > >>--- a/hw/ppc/spapr_drc.c > >>+++ b/hw/ppc/spapr_drc.c > >>@@ -610,6 +610,20 @@ static void spapr_dr_connector_instance_init(Objec= t *obj) > >> NULL, NULL, NULL, NULL); > >> } > >>+static const VMStateDescription vmstate_spapr_drc =3D { > >>+ .name =3D "spapr_drc", > >>+ .version_id =3D 1, > >>+ .minimum_version_id =3D 1, > >>+ .fields =3D (VMStateField []) { > >>+ VMSTATE_UINT32(isolation_state, sPAPRDRConnector), > >>+ VMSTATE_UINT32(allocation_state, sPAPRDRConnector), > >>+ VMSTATE_UINT32(indicator_state, sPAPRDRConnector), > >>+ VMSTATE_BOOL(configured, sPAPRDRConnector), > >>+ VMSTATE_BOOL(awaiting_release, sPAPRDRConnector), > >>+ VMSTATE_END_OF_LIST() > >>+ } > >>+}; > >>+ > >> static void spapr_dr_connector_class_init(ObjectClass *k, void *data) > >> { > >> DeviceClass *dk =3D DEVICE_CLASS(k); > >>@@ -618,6 +632,7 @@ static void spapr_dr_connector_class_init(ObjectCla= ss *k, void *data) > >> dk->reset =3D reset; > >> dk->realize =3D realize; > >> dk->unrealize =3D unrealize; > >>+ dk->vmsd =3D &vmstate_spapr_drc; > >> drck->set_isolation_state =3D set_isolation_state; > >> drck->set_indicator_state =3D set_indicator_state; > >> drck->set_allocation_state =3D set_allocation_state; >=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 --zbGR4y+acU1DwHSi Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXGafCAAoJEGw4ysog2bOSezEP/j3xmT5lN0/5uIV2+qnNP5mk 1w1gwGJftUM+uPxICPYX2SfT37cgty3iB5UgwmSUxLbFb6QAkldf611uaE3SsOfB jEuuhIAV7wKSFskLXqpmiBMhaOgZg44MLp3ScySRHjtBM8Bl5ZZSghoLIoxfzMhE WlFtI7ygX3DLamdfaNgNgy9tnObijUWUF8mxlWWuGP03ExwJGsURTwLXKgO0WuUP 1aH/CBh/Mp1eNj0WMYi3DyxSzXUrMm1mPC/wMw0VelairZH9/m0v9JF34wBt5U5M RYGDxE3YNsnd82la72gQ/khF49gE6rfVYYFMbCqNRx049TKnVOujOKitYoEceiQY bGoI0iiWp/KDfrh/Yin6sWyFosGBThGQ41ZklmvEkRLNoTqKMz386SgtvOAxDqsd rYztAfyjiMUZjP7k42A9zDtp+CmrOrqD/Co/4CsfPGrRfI/ntZFGNl6fzUVS+dTG b88yOXuTkbNR+APwpumfGJflDSd1gjnIinC6dN9O5vQExuEUrP3w2HU5tSjjX258 rSXezmAw+KL31jRdoVnRUUQEaRoyAPAp1SLoy8xMV4lRvXQ1GpkBzcUhxuGvxdtP NfZLxkOuVVD4a9omWvQlL83c9pTGM8mDrT4jyJVlX+KSOx9Jp5Tp8AFSBwGk8VRf XN9VagiO6ZS3JftruDJ0 =o7sU -----END PGP SIGNATURE----- --zbGR4y+acU1DwHSi--