From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40362) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XYLA2-0007Pi-9B for qemu-devel@nongnu.org; Sun, 28 Sep 2014 16:33:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XYLA1-0002g9-0n for qemu-devel@nongnu.org; Sun, 28 Sep 2014 16:33:38 -0400 Received: from mail.avalus.com ([2001:41c8:10:1dd::10]:36037) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XYLA0-0002Wh-OD for qemu-devel@nongnu.org; Sun, 28 Sep 2014 16:33:36 -0400 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 7.3 \(1878.6\)) From: Alex Bligh In-Reply-To: <20140928153058.GA4994@redhat.com> Date: Sun, 28 Sep 2014 21:33:08 +0100 Content-Transfer-Encoding: quoted-printable Message-Id: References: <1411414496-46245-1-git-send-email-alex@alex.org.uk> <1411414496-46245-2-git-send-email-alex@alex.org.uk> <20140928153058.GA4994@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Ryan Harper , Serge Hallyn , "quintela@redhat.com" , Libvirt , Serge Hallyn , "qemu-devel@nongnu.org" , Alexander Graf , Alex Bligh , Cole Robinson , Amit Shah , Bruce Rogers , =?iso-8859-1?Q?Andreas_F=E4rber?= , "Serge E. Hallyn" Michael, >> +static const VMStateDescription vmstate_acpi_compat =3D { >> + .name =3D "piix4_pm", >> + .version_id =3D 3, >> + .minimum_version_id =3D 2, >> + .minimum_version_id_old =3D 1, >> + .load_state_old =3D NULL, /* to avoid recursion */ >> + .post_load =3D vmstate_acpi_post_load, >> + .fields =3D (VMStateField[]) { >> + VMSTATE_PCI_DEVICE(parent_obj, PIIX4PMState), >> + VMSTATE_UINT16(ar.pm1.evt.sts, PIIX4PMState), >> + VMSTATE_UINT16(ar.pm1.evt.en, PIIX4PMState), >> + VMSTATE_UINT16(ar.pm1.cnt.cnt, PIIX4PMState), >> + VMSTATE_STRUCT(apm, PIIX4PMState, 0, vmstate_apm, APMState), >> + VMSTATE_TIMER(ar.tmr.timer, PIIX4PMState), >> + VMSTATE_INT64(ar.tmr.overflow_time, PIIX4PMState), >> + VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, = ACPIGPE), >> + VMSTATE_STRUCT_TEST( >> + = acpi_pci_hotplug.acpi_pcihp_pci_status[ACPI_PCIHP_BSEL_DEFAULT], >> + PIIX4PMState, >> + vmstate_test_no_use_acpi_pci_hotplug, >> + 2, vmstate_pci_status, >> + struct AcpiPciHpPciStatus), >> + VMSTATE_PCI_HOTPLUG(acpi_pci_hotplug, PIIX4PMState, >> + vmstate_test_use_acpi_pci_hotplug), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >=20 > Please don't duplicate code like this. > What is the difference here? Is it just .minimum_version_id? > Why not just update it in vmstate_acpi? That and the change to load_state_old. I thought I had tried that, but that it got memcpy'd somewhere deep in QOM I think (as part of the inheritance). If the structure never gets used again (for e.g. export?) I suppose I could patch it live in acpi_load_old - is that what you meant? I'd also have to remove the 'const' tag, which I seem to remember was non-trivial. Perhaps I could make a copy and change the fields. >> +/* NB cirrus-vga default value is 8MB anyway, save if we >> + * monkey patch it to change the default when the qemu-kvm-migration >> + * machine parameter is selected >> + */ >> + >=20 > This is too hacky for my taste. > How about creating a new machine e.g. pc-qemu-kvm-1.0 and in > pc_early_init_pci_1_0, changing compat_props for pc-1.0 to point to = the > compat_props of pc-qemu-kvm-1.0? Hang on a second! v2 of this patch DID use a new virtual machine, called exactly that. I thought you were objecting to that and wanting a machine parameter instead! It's far easier with a new machine type, and I'd far prefer a new machine type. If you were just objecting to the fact that pc-1.0 was made to be an alias of either one or the other at compile time, simply drop the second patch of the v2 patchset. If we have a new machine type, I don't /think/ I need the early_init thing at all (I may be wrong about that). --=20 Alex Bligh