Re: [Qemu-devel] [PATCH v4 17/17] spapr_pci: emit hotplug add/remove events during hotplug

[Michael Roth]

Quoting David Gibson (2015-01-19 00:00:00)
> On Tue, Dec 23, 2014 at 06:30:31AM -0600, Michael Roth wrote:
> > From: Tyrel Datwyler <address@hidden>
> > 
> > This uses extension of existing EPOW interrupt/event mechanism
> > to notify userspace tools like librtas/drmgr to handle
> > in-guest configuration/cleanup operations in response to
> > device_add/device_del.
> > 
> > Userspace tools that don't implement this extension will need
> > to be run manually in response/advance of device_add/device_del,
> > respectively.
> > 
> > Signed-off-by: Tyrel Datwyler <address@hidden>
> > Signed-off-by: Michael Roth <address@hidden>
> > ---
> >  hw/ppc/spapr_pci.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 94e33b4..f17f984 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -705,6 +705,9 @@ static void spapr_phb_hot_plug(HotplugHandler 
> > *plug_handler,
> >  
> >      g_assert(drc);
> >      spapr_device_hotplug_add(drc, phb, pdev);
> > +    if (plugged_dev->hotplugged) {
> > +        spapr_hotplug_req_add_event(drc);
> > +    }
> >  }
> >  
> >  static void spapr_phb_hot_unplug(HotplugHandler *plug_handler,
> > @@ -722,6 +725,7 @@ static void spapr_phb_hot_unplug(HotplugHandler 
> > *plug_handler,
> >      }
> >  
> >      spapr_device_hotplug_remove(drc, phb, pdev);
> > +    spapr_hotplug_req_remove_event(drc);
> 
> The event is sent after the "physical" remove.  Is that correct?

From the guest perspective it doesn't really matter since we default to an
allocation state of UNISOLATED, so we always end up waiting for the
guest-induced transition to ISOLATED before completing the removal (or a
reboot, in which case the event is not needed).

Thank you for the excellent review. I've responded where clarification
seemed warranted, but otherwise plan on addressing all the comments in
v5 which should go out soon.

> 
> >  }
> >  
> >  static void spapr_phb_realize(DeviceState *dev, Error **errp)
> 
> -- 
> 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