Re: [Qemu-devel] [PATCH 2/5] Migration: Defined VMStateDescription struct for spapr_drc

[Jianjun Duan]

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 <address@hidden>
> 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;