From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60897) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wnrcp-0006Ro-D3 for qemu-devel@nongnu.org; Fri, 23 May 2014 11:43:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wnrci-0006Ul-RU for qemu-devel@nongnu.org; Fri, 23 May 2014 11:43:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36715) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wnrci-0006UB-Ji for qemu-devel@nongnu.org; Fri, 23 May 2014 11:43:08 -0400 Date: Fri, 23 May 2014 17:41:44 +0200 From: Igor Mammedov Message-ID: <20140523174144.52b0e948@thinkpad> In-Reply-To: References: <1400598934-31921-1-git-send-email-imammedo@redhat.com> <1400671787-14214-4-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 26/31] pc: migrate piix4 & ich9 MemHotplugState List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrey Korolyov Cc: Peter Maydell , alex@alex.org.uk, "Michael S. Tsirkin" , aik@ozlabs.ru, jan.kiszka@siemens.com, Michael Tokarev , "qemu-devel@nongnu.org" , lcapitulino@redhat.com, Gerd Hoffmann , pasteka@kabsi.at, Stefan Priebe - Profihost AG , agarcia@igalia.com, armbru@redhat.com, aliguori@amazon.com, hutao@cn.fujitsu.com, david@gibson.dropbear.id.au, lersek@redhat.com, ehabkost@redhat.com, marcel.a@redhat.com, stefanha@redhat.com, cornelia.huck@de.ibm.com, tangchen@cn.fujitsu.com, rth@twiddle.net, Alexander Graf , vasilis.liaskovitis@profitbricks.com, Paolo Bonzini , afaerber@suse.de, aurelien@aurel32.net On Fri, 23 May 2014 19:11:49 +0400 Andrey Korolyov wrote: > On Wed, May 21, 2014 at 3:29 PM, Igor Mammedov wrote: > > Adds an optional subsection that allows to migrate current > > state of acpi_memory_hotplug of ACPI PM device. > > > > Signed-off-by: Igor Mammedov > > --- > > 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