Re: [Qemu-devel] [PATCH] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

[Alex Bligh]

Sorry that was v5 - git send-email fumble fingers as git-publish
doesn't seem to work well with single patches.

Alex

On 4 Oct 2014, at 17:37, Alex Bligh <address@hidden> wrote:

> Add a machine type pc-1.0-qemu-kvm for live migrate compatibility
> with qemu-kvm version 1.0.
> 
> This patch adds inbound migrate capability from qemu-kvm version
> 1.0. The main ideas are those set out in Cole Robinson's patch here:
> http://pkgs.fedoraproject.org/cgit/qemu.git/tree/0001-Fix-migration-from-qemu-kvm.patch?h=f20
> however, rather than patching statically (and breaking inbound
> migration on existing machine types), I have added a new machine
> type (pc-1.0-qemu-kvm) without affecting any other machine types.
> The existing pc-1.0 machine type is renamed to pc-1.0-qemu-git,
> with pc-1.0 becoming an alias for one or another, as selected
> by a configure option (defaulting to pc-1.0-qemu-git, IE no
> change).
> 
> Two aproaches are taken:
> 
> * In hw/timer/i8254_common.c, the VMSTATE_UINT32_TEST macro
> is used to test the version for the irq_disable flags,
> allowing version 3 or more, or version 2 for an inbound
> migrate from qemu-kvm (only).
> 
> * In hw/acpi/piix4.c, qemu-kvm incorrectly uses version 2 for
> a version 3 structure, causing acpi_load_old to be used.
> acpi_load_old detects this situation based on the machine type
> and restarts the attempt to load the vmstate using a
> customised VMStateDescription. The above cleaner approach is
> unavailable here.
> 
> I developed this on qemu 2.0 but have forward ported it (trivially)
> to master. My testing has been on a VM live-migrated-to-file from
> Ubuntu Precise qemu-kvm 1.0.
> 
> I have given this a moderate degree of testing but it could do
> with more.
> 
> Note that certain hardware devices (including QXL) will not
> migrate properly due to a fundamental difference in their internal
> state between versions.
> 
> Also note that (as expected) migration from qemu-2.x to qemu-1.0
> will not work, even if the machine types are the same.
> 
> Changes from v4
> 
> * Revert to using a machine type, but do not add alias machine types,
>  configure options, or (potentially) change the meaning of
>  pc-1.0 - leave this for distributions to ponder
> 
> * Add compat_props for qemu-kvm-migration to the PIIX4_PM driver
>  and the i8259 pit-common driver.
> 
> Signed-off-by: Alex Bligh <address@hidden>
> ---
> hw/acpi/piix4.c                   |   26 +++++++++++++++++++++++---
> hw/i386/pc_piix.c                 |   27 +++++++++++++++++++++++++++
> hw/timer/i8254_common.c           |   18 +++++++++++++++++-
> include/hw/i386/pc.h              |    8 ++++++++
> include/hw/timer/i8254_internal.h |    1 +
> 5 files changed, 76 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index b72b34e..5c68d69 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -86,6 +86,8 @@ typedef struct PIIX4PMState {
>     Notifier cpu_added_notifier;
> 
>     MemHotplugState acpi_memory_hotplug;
> +
> +    bool qemu_kvm_migration;
> } PIIX4PMState;
> 
> #define TYPE_PIIX4_PM "PIIX4_PM"
> @@ -200,12 +202,26 @@ static const VMStateDescription vmstate_pci_status = {
>     }
> };
> 
> +static const VMStateDescription vmstate_acpi;
> +
> static int acpi_load_old(QEMUFile *f, void *opaque, int version_id)
> {
>     PIIX4PMState *s = opaque;
>     int ret, i;
>     uint16_t temp;
> 
> +    /* If we are expecting the inbound migration to come from
> +     * qemu-kvm 1.0, it will have a version_id of 2 but really
> +     * be version 3, so call back the original vmstate_load_state
> +     * with a different more tolerant vmstate descriptor
> +     */
> +    if (version_id == 2 && s->qemu_kvm_migration) {
> +        VMStateDescription vmstate_acpi_compat = vmstate_acpi;
> +        vmstate_acpi_compat.minimum_version_id = 2;
> +        return vmstate_load_state(f, &vmstate_acpi_compat,
> +                                  opaque, version_id);
> +    }
> +
>     ret = pci_device_load(PCI_DEVICE(s), f);
>     if (ret < 0) {
>         return ret;
> @@ -267,9 +283,11 @@ static const VMStateDescription vmstate_memhp_state = {
> };
> 
> /* 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
> - * qemu 1.2).
> + * To support incoming qemu-kvm 1.2 migration, we support
> + * via a command line option a change to minimum_version_id
> + * of 2 in a _compat structure; we can't do this all the time
> + * as using a minimum_version_id of 2 (rather than 3) would
> + * break migration from qemu-git 1.2.
>  *
>  */
> static const VMStateDescription vmstate_acpi = {
> @@ -589,6 +607,8 @@ static Property piix4_pm_properties[] = {
>                      use_acpi_pci_hotplug, true),
>     DEFINE_PROP_BOOL("memory-hotplug-support", PIIX4PMState,
>                      acpi_memory_hotplug.is_enabled, true),
> +    DEFINE_PROP_BOOL("qemu-kvm-migration", PIIX4PMState,
> +                     qemu_kvm_migration, false),
>     DEFINE_PROP_END_OF_LIST(),
> };
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7081c08..56555c1 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -644,6 +644,32 @@ static QEMUMachine pc_machine_v1_0 = {
>     .hw_version = "1.0",
> };
> 
> +#define PC_COMPAT_1_0_QEMU_KVM \
> +        PC_COMPAT_1_0,\
> +        {\
> +            .driver   = "cirrus-vga",\
> +            .property = "vgamem_mb",\
> +            .value    = stringify(16),\
> +        },{\
> +            .driver   = "pit-common",\
> +            .property = "qemu-kvm-migration",\
> +            .value    = "on",\
> +        },{\
> +            .driver   = "PIIX4_PM",\
> +            .property = "qemu-kvm-migration",\
> +            .value    = "on",\
> +        }
> +
> +static QEMUMachine pc_machine_v1_0_qemu_kvm = {
> +    PC_I440FX_1_2_MACHINE_OPTIONS,
> +    .name = "pc-1.0-qemu-kvm",
> +    .compat_props = (GlobalProperty[]) {
> +        PC_COMPAT_1_0_QEMU_KVM,
> +        { /* end of list */ }
> +    },
> +    .hw_version = "1.0",
> +};
> +
> #define PC_COMPAT_0_15 \
>         PC_COMPAT_1_0
> 
> @@ -886,6 +912,7 @@ static void pc_machine_init(void)
>     qemu_register_pc_machine(&pc_machine_v1_2);
>     qemu_register_pc_machine(&pc_machine_v1_1);
>     qemu_register_pc_machine(&pc_machine_v1_0);
> +    qemu_register_pc_machine(&pc_machine_v1_0_qemu_kvm);
>     qemu_register_pc_machine(&pc_machine_v0_15);
>     qemu_register_pc_machine(&pc_machine_v0_14);
>     qemu_register_pc_machine(&pc_machine_v0_13);
> diff --git a/hw/timer/i8254_common.c b/hw/timer/i8254_common.c
> index 07345f6..7f3e4e3 100644
> --- a/hw/timer/i8254_common.c
> +++ b/hw/timer/i8254_common.c
> @@ -257,6 +257,14 @@ static int pit_dispatch_post_load(void *opaque, int 
> version_id)
>     return 0;
> }
> 
> +static bool has_irq_disabled(void *opaque, int version_id)
> +{
> +    PITCommonState *s = opaque;
> +    return (version_id >= 3) ||
> +        (version_id == 2 &&
> +         s->qemu_kvm_migration);
> +}
> +
> static const VMStateDescription vmstate_pit_common = {
>     .name = "i8254",
>     .version_id = 3,
> @@ -266,7 +274,8 @@ static const VMStateDescription vmstate_pit_common = {
>     .pre_save = pit_dispatch_pre_save,
>     .post_load = pit_dispatch_post_load,
>     .fields = (VMStateField[]) {
> -        VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3),
> +        VMSTATE_UINT32_TEST(channels[0].irq_disabled, PITCommonState,
> +                            has_irq_disabled),
>         VMSTATE_STRUCT_ARRAY(channels, PITCommonState, 3, 2,
>                              vmstate_pit_channel, PITChannelState),
>         VMSTATE_INT64(channels[0].next_transition_time,
> @@ -275,6 +284,12 @@ static const VMStateDescription vmstate_pit_common = {
>     }
> };
> 
> +static Property pit_common_properties[] = {
> +    DEFINE_PROP_BOOL("qemu-kvm-migration", PITCommonState,
> +                     qemu_kvm_migration, false),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> static void pit_common_class_init(ObjectClass *klass, void *data)
> {
>     DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -287,6 +302,7 @@ static void pit_common_class_init(ObjectClass *klass, 
> void *data)
>      * done by board code.
>      */
>     dc->cannot_instantiate_with_device_add_yet = true;
> +    dc->props = pit_common_properties;
> }
> 
> static const TypeInfo pit_common_type = {
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 1c0c382..e420dbc 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -346,6 +346,14 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t 
> *);
>             .driver   = "ioh3420",\
>             .property = COMPAT_PROP_PCP,\
>             .value    = "off",\
> +        },{\
> +            .driver   = "pit-common",\
> +            .property = "qemu-kvm-migration",\
> +            .value    = "off",\
> +        },{\
> +            .driver   = "PIIX4_PM",\
> +            .property = "qemu-kvm-migration",\
> +            .value    = "off",\
>         }
> 
> #define PC_COMPAT_1_7 \
> diff --git a/include/hw/timer/i8254_internal.h 
> b/include/hw/timer/i8254_internal.h
> index 61a1bfb..5f7ee36 100644
> --- a/include/hw/timer/i8254_internal.h
> +++ b/include/hw/timer/i8254_internal.h
> @@ -55,6 +55,7 @@ typedef struct PITCommonState {
>     MemoryRegion ioports;
>     uint32_t iobase;
>     PITChannelState channels[3];
> +    bool qemu_kvm_migration;
> } PITCommonState;
> 
> #define TYPE_PIT_COMMON "pit-common"
> -- 
> 1.7.9.5
> 
> 
> 

-- 
Alex Bligh