From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42892) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atI1Y-0003sq-Ne for qemu-devel@nongnu.org; Thu, 21 Apr 2016 13:04:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1atI1V-0007MQ-If for qemu-devel@nongnu.org; Thu, 21 Apr 2016 13:04:16 -0400 Received: from e34.co.us.ibm.com ([32.97.110.152]:38255) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1atI1V-0007MG-CB for qemu-devel@nongnu.org; Thu, 21 Apr 2016 13:04:13 -0400 Received: from localhost by e34.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 21 Apr 2016 11:04:12 -0600 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> From: Jianjun Duan Message-ID: <571907FC.8030303@linux.vnet.ibm.com> Date: Thu, 21 Apr 2016 10:03:56 -0700 MIME-Version: 1.0 In-Reply-To: <20160420043252.GF1133@voom> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: David Gibson Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, mdroth@linux.vnet.ibm.com 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. >> --- >> 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(Object *obj) >> NULL, NULL, NULL, NULL); >> } >> >> +static const VMStateDescription vmstate_spapr_drc = { >> + .name = "spapr_drc", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .fields = (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 = DEVICE_CLASS(k); >> @@ -618,6 +632,7 @@ static void spapr_dr_connector_class_init(ObjectClass *k, void *data) >> dk->reset = reset; >> dk->realize = realize; >> dk->unrealize = unrealize; >> + dk->vmsd = &vmstate_spapr_drc; >> drck->set_isolation_state = set_isolation_state; >> drck->set_indicator_state = set_indicator_state; >> drck->set_allocation_state = set_allocation_state;