[RESEND,v3,3/8] pc: Add memory hot unplug support for pc machine.

From: Hu Tao <hutao@cn.fujitsu.com>

Implement device unplug callback for PCMachine. And it now only support
pc-dimm hot-remove. The callback will call piix4 or ich9 callbacks introduced
in previous patches.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
---
 hw/i386/pc.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

On Wed, 27 Aug 2014 16:08:34 +0800
Tang Chen <tangchen@cn.fujitsu.com> wrote:

> From: Hu Tao <hutao@cn.fujitsu.com>
> 
> Implement device unplug callback for PCMachine. And it now only support
> pc-dimm hot-remove. The callback will call piix4 or ich9 callbacks introduced
> in previous patches.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> ---
>  hw/i386/pc.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 8fa8d2f..8bafe7c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -61,6 +61,8 @@
>  #include "hw/mem/pc-dimm.h"
>  #include "trace.h"
>  #include "qapi/visitor.h"
> +#include "hw/acpi/piix4.h"
> +#include "hw/i386/ich9.h"
>  
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
> @@ -1607,6 +1609,27 @@ out:
>      error_propagate(errp, local_err);
>  }
>  
> +static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
> +                           DeviceState *dev, Error **errp)
> +{
> +    Object *acpi_dev;
> +    HotplugHandlerClass *hhc;
> +    Error *local_err = NULL;
> +
> +    acpi_dev = (acpi_dev = piix4_pm_find()) ? acpi_dev : ich9_lpc_find();
> +    if (!acpi_dev) {
> +        error_setg(&local_err,
> +                   "memory hotplug is not enabled: missing acpi device");
> +        error_propagate(errp, local_err);
> +        return;
> +    }
drop/replace above block, see: "if (!pcms->acpi_dev) {" for reference

> +
> +    hhc = HOTPLUG_HANDLER_GET_CLASS(acpi_dev);
> +    hhc->unplug(HOTPLUG_HANDLER(acpi_dev), dev, &local_err);

you should take care about dev's vmstate here, that was registered in pc_dimm_plug()
> +
> +    error_propagate(errp, local_err);
> +}
> +
>  static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>                                        DeviceState *dev, Error **errp)
>  {
> @@ -1615,6 +1638,14 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>      }
>  }
>  
> +static void pc_machine_device_unplug_cb(HotplugHandler *hotplug_dev,
> +                                        DeviceState *dev, Error **errp)
> +{
> +    if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> +        pc_dimm_unplug(hotplug_dev, dev, errp);
> +    }
> +}
> +
>  static HotplugHandler *pc_get_hotpug_handler(MachineState *machine,
>                                               DeviceState *dev)
>  {
> @@ -1701,6 +1732,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
>      pcmc->get_hotplug_handler = mc->get_hotplug_handler;
>      mc->get_hotplug_handler = pc_get_hotpug_handler;
>      hc->plug = pc_machine_device_plug_cb;
> +    hc->unplug = pc_machine_device_unplug_cb;
>  }
>  
>  static const TypeInfo pc_machine_info = {

On Thu, 4 Sep 2014 14:44:41 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Wed, 27 Aug 2014 16:08:34 +0800
> Tang Chen <tangchen@cn.fujitsu.com> wrote:
> 
> > From: Hu Tao <hutao@cn.fujitsu.com>
> > 
> > Implement device unplug callback for PCMachine. And it now only support
> > pc-dimm hot-remove. The callback will call piix4 or ich9 callbacks introduced
> > in previous patches.
> > 
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > Signed-off-by: Tang Chen <tangchen@cn.fujitsu.com>
> > ---
> >  hw/i386/pc.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 8fa8d2f..8bafe7c 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -61,6 +61,8 @@
> >  #include "hw/mem/pc-dimm.h"
> >  #include "trace.h"
> >  #include "qapi/visitor.h"
> > +#include "hw/acpi/piix4.h"
> > +#include "hw/i386/ich9.h"
> >  
> >  /* debug PC/ISA interrupts */
> >  //#define DEBUG_IRQ
> > @@ -1607,6 +1609,27 @@ out:
> >      error_propagate(errp, local_err);
> >  }
> >  
> > +static void pc_dimm_unplug(HotplugHandler *hotplug_dev,
> > +                           DeviceState *dev, Error **errp)
> > +{
> > +    Object *acpi_dev;
> > +    HotplugHandlerClass *hhc;
> > +    Error *local_err = NULL;
> > +
> > +    acpi_dev = (acpi_dev = piix4_pm_find()) ? acpi_dev : ich9_lpc_find();
> > +    if (!acpi_dev) {
> > +        error_setg(&local_err,
> > +                   "memory hotplug is not enabled: missing acpi device");
> > +        error_propagate(errp, local_err);
> > +        return;
> > +    }
> drop/replace above block, see: "if (!pcms->acpi_dev) {" for reference
> 
> > +
> > +    hhc = HOTPLUG_HANDLER_GET_CLASS(acpi_dev);
> > +    hhc->unplug(HOTPLUG_HANDLER(acpi_dev), dev, &local_err);
> 
> you should take care about dev's vmstate here, that was registered in pc_dimm_plug()
I'm wrong here, vmstate_unregister should take place at pc_dimm unrealize time,

hhc->unplug handler is more like unplug_request handler,
perhaps we need to give some thought to renaming it to unplug_request to avoid confusion
and have an optional unplug callback that could do tear-down with HotplugHandler if needed
before device is unrealized.