diff mbox

[v4,17/17] spapr_pci: emit hotplug add/remove events during hotplug

Message ID 1419337831-16552-18-git-send-email-mdroth@linux.vnet.ibm.com
State New
Headers show

Commit Message

Michael Roth Dec. 23, 2014, 12:30 p.m. UTC
From: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>

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 <tyreld@linux.vnet.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr_pci.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

David Gibson Jan. 19, 2015, 6 a.m. UTC | #1
On Tue, Dec 23, 2014 at 06:30:31AM -0600, Michael Roth wrote:
> From: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> 
> 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 <tyreld@linux.vnet.ibm.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  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?

>  }
>  
>  static void spapr_phb_realize(DeviceState *dev, Error **errp)
Michael Roth Jan. 26, 2015, 9:32 p.m. UTC | #2
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 <tyreld@linux.vnet.ibm.com>
> > 
> > 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 <tyreld@linux.vnet.ibm.com>
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> >  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
diff mbox

Patch

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