Re: [Qemu-devel] [PATCH v2 08/12] acpi, mem-hotplug: Add unplug cb for memory device.

[Igor Mammedov <imammedo@redhat.com>]

On Wed, 4 Feb 2015 10:51:22 +0800
Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:

> From: Tang Chen <tangchen@cn.fujitsu.com>
> 
> Reset all memory status, and unparent the memory device.
> 
> Signed-off-by: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
> ---
>  hw/acpi/memory_hotplug.c         | 34 ++++++++++++++++++++++++++++++++++
>  hw/core/qdev.c                   |  2 +-
>  include/hw/acpi/memory_hotplug.h |  2 ++
>  include/hw/qdev-core.h           |  1 +
>  4 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 3d3c1ec..3ae9629 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -1,6 +1,7 @@
>  #include "hw/acpi/memory_hotplug.h"
>  #include "hw/acpi/pc-hotplug.h"
>  #include "hw/mem/pc-dimm.h"
> +#include "hw/i386/pc.h"
>  #include "hw/boards.h"
>  #include "trace.h"
>  #include "qapi-event.h"
> @@ -221,6 +222,39 @@ void acpi_memory_unplug_request_cb(ACPIREGS *ar, qemu_irq irq,
>      acpi_send_gpe_event(ar, irq, ACPI_MEMORY_HOTPLUG_STATUS);
>  }
>  
> +void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq,
> +                           MemHotplugState *mem_st,
> +                           DeviceState *dev, Error **errp)
> +{
[...]
> +
> +    hotplug_dev = qdev_get_hotplug_handler(dev);
> +    pcms = PC_MACHINE(hotplug_dev);
> +    dimm = PC_DIMM(dev);
> +    ddc = PC_DIMM_GET_CLASS(dimm);
> +    mr = ddc->get_memory_region(dimm);
> +
> +    memory_region_del_subregion(&pcms->hotplug_memory, mr);
> +    vmstate_unregister_ram(mr, dev);
perhaps I wasn't clear previous time,
acpi device has no business in un-mapping and un-registering.

Pls, leave lines that deal with internal to acpi device state
/i.e. mdev/ and move memory_region_del_subregion/vmstate_unregister_ram
to pc_dimm_unplug().

> +}
> +
>  static const VMStateDescription vmstate_memhp_sts = {
>      .name = "memory hotplug device state",
>      .version_id = 1,




> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 2eacac0..2f3d1df 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -273,7 +273,7 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
>      dev->alias_required_for_version = required_for_version;
>  }
>  
> -static HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
> +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
>  {
>      HotplugHandler *hotplug_ctrl = NULL;
>  
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 15a226f..03d6239 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -266,6 +266,7 @@ int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT;
>  void qdev_init_nofail(DeviceState *dev);
>  void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
>                                   int required_for_version);
> +HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
>  void qdev_unplug(DeviceState *dev, Error **errp);
>  void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
>                                    DeviceState *dev, Error **errp);
move qdev_get_hotplug_handler changes into a separate patch,
though it won't be used in this patch it will be used by the last patch anyway.