Re: [Qemu-devel] [PATCH v2 26/31] pc: migrate piix4 & ich9 MemHotplugState

[Andrey Korolyov]

On Wed, May 21, 2014 at 3:29 PM, Igor Mammedov <address@hidden> wrote:
> Adds an optional subsection that allows to migrate current
> state of acpi_memory_hotplug of ACPI PM device.
>
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> v2:
>   * use subsection, requested by pbonzini
> ---
>  hw/acpi/ich9.c                   |   24 ++++++++++++++++++++++++
>  hw/acpi/memory_hotplug.c         |   27 +++++++++++++++++++++++++++
>  hw/acpi/piix4.c                  |   24 ++++++++++++++++++++++++
>  include/hw/acpi/memory_hotplug.h |    7 +++++++
>  4 files changed, 82 insertions(+), 0 deletions(-)
>
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 86c45ba..f021b33 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -140,6 +140,23 @@ static int ich9_pm_post_load(void *opaque, int 
> version_id)
>       .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
>   }
>
> +static bool vmstate_test_use_memhp(void *opaque)
> +{
> +    ICH9LPCPMRegs *s = opaque;
> +    return s->acpi_memory_hotplug.is_enabled;
> +}
> +
> +static const VMStateDescription vmstate_memhp_state = {
> +    .name = "ich9_pm/memhp",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_MEMORY_HOTPLUG(acpi_memory_hotplug, ICH9LPCPMRegs),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  const VMStateDescription vmstate_ich9_pm = {
>      .name = "ich9_pm",
>      .version_id = 1,
> @@ -157,6 +174,13 @@ const VMStateDescription vmstate_ich9_pm = {
>          VMSTATE_UINT32(smi_en, ICH9LPCPMRegs),
>          VMSTATE_UINT32(smi_sts, ICH9LPCPMRegs),
>          VMSTATE_END_OF_LIST()
> +    },
> +    .subsections = (VMStateSubsection[]) {
> +        {
> +            .vmsd = &vmstate_memhp_state,
> +            .needed = vmstate_test_use_memhp,
> +        },
> +        VMSTATE_END_OF_LIST()
>      }
>  };
>
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 79158f1..98a3d8c 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -158,3 +158,30 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, 
> MemHotplugState *mem_st,
>      acpi_update_sci(ar, irq);
>      return;
>  }
> +
> +static const VMStateDescription vmstate_memhp_sts = {
> +    .name = "memory hotplug device state",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_BOOL(is_enabled, MemStatus),
> +        VMSTATE_BOOL(is_inserting, MemStatus),
> +        VMSTATE_UINT32(ost_event, MemStatus),
> +        VMSTATE_UINT32(ost_status, MemStatus),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +const VMStateDescription vmstate_memory_hotplug = {
> +    .name = "memory hotplug state",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT32(selector, MemHotplugState),
> +        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(devs, MemHotplugState, 
> dev_count,
> +                                             vmstate_memhp_sts, MemStatus),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 056a7bc..c220715 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -250,6 +250,23 @@ static bool vmstate_test_no_use_acpi_pci_hotplug(void 
> *opaque, int version_id)
>      return !s->use_acpi_pci_hotplug;
>  }
>
> +static bool vmstate_test_use_memhp(void *opaque)
> +{
> +    PIIX4PMState *s = opaque;
> +    return s->acpi_memory_hotplug.is_enabled;
> +}
> +
> +static const VMStateDescription vmstate_memhp_state = {
> +    .name = "piix4_pm/memhp",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_MEMORY_HOTPLUG(acpi_memory_hotplug, PIIX4PMState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  /* qemu-kvm 1.2 uses version 3 but advertised as 2
>   * To support incoming qemu-kvm 1.2 migration, change version_id
>   * and minimum_version_id to 2 below (which breaks migration from
> @@ -281,6 +298,13 @@ static const VMStateDescription vmstate_acpi = {
>          VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState,
>                              vmstate_test_use_acpi_pci_hotplug),
>          VMSTATE_END_OF_LIST()
> +    },
> +    .subsections = (VMStateSubsection[]) {
> +        {
> +            .vmsd = &vmstate_memhp_state,
> +            .needed = vmstate_test_use_memhp,
> +        },
> +        VMSTATE_END_OF_LIST()
>      }
>  };
>
> diff --git a/include/hw/acpi/memory_hotplug.h 
> b/include/hw/acpi/memory_hotplug.h
> index 912c53f..4588459 100644
> --- a/include/hw/acpi/memory_hotplug.h
> +++ b/include/hw/acpi/memory_hotplug.h
> @@ -3,6 +3,7 @@
>
>  #include "hw/qdev-core.h"
>  #include "hw/acpi/acpi.h"
> +#include "migration/vmstate.h"
>
>  #define ACPI_MEMORY_HOTPLUG_STATUS 8
>
> @@ -27,4 +28,10 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object 
> *owner,
>
>  void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
>                           DeviceState *dev, Error **errp);
> +
> +extern const VMStateDescription vmstate_memory_hotplug;
> +#define VMSTATE_MEMORY_HOTPLUG(memhp, state) \
> +    VMSTATE_STRUCT(memhp, state, 1, \
> +                   vmstate_memory_hotplug, MemHotplugState)
> +
>  #endif
> --
> 1.7.1
>

As I just wrote to Igor, there is a need to preserve dimm states
across p2p migration (like one in libvirt), or live migration will be
not possible with changed dimm configuration at all. Otherwise
patchset works very well - I am not able to found any other flaws in
functionality. If reviewers consider to postpone this functionality -
memhp is still very useable and long-demanded option, so patchset is
self-sufficient.