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

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 23 May 2014 19:11:49 +0400
Andrey Korolyov <andrey@xdel.ru> wrote:

> On Wed, May 21, 2014 at 3:29 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> > Adds an optional subsection that allows to migrate current
> > state of acpi_memory_hotplug of ACPI PM device.
> >
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > 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
In my tests live migration works just fine if done manually (i.e without
libvirt). To work target has to be started with all already present dimm
devices on source (cold/hot-plugged on QEMU CLI using -device dimm,...)

> 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.

Thanks for feedback and testing!

-- 
Regards,
  Igor