* [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm @ 2014-09-22 19:34 Alex Bligh 2014-09-22 19:34 ` Alex Bligh ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Alex Bligh @ 2014-09-22 19:34 UTC (permalink / raw) To: qemu-devel Cc: Ryan Harper, Serge Hallyn, Michael S. Tsirkin, Libvirt, Serge Hallyn, Alexander Graf, Bruce Rogers, quintela, Alex Bligh, Cole Robinson, Amit Shah, Andreas Färber, Serge E. Hallyn This patch series 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 parameter (qemu-kvm-migration) which when turned on affects the pc-1.0 machine type. Usage: -machine pc-1.0,qemu-kvm-migration=on Three aproaches are taken: * cirrus-vga.vgamem_mb defaults to 16 rather than 8. In order to keep -global cirrus-vga.vgamem_mb working even with qemu-kvm-migration=on, this is monkey-patched into the default value of the MachineState structure's compat_props list. * 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. The above monkey-patching must be done between the selection of the MachineClass and the processing of the machine parameters (on the one hand) and the processing of the compat_props list and the globals on the command line. To do this I have added an earlyinit function to MachineState and QEMUMachine. 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 since v1: * Do not use a machine type, use a machine parameter. Alex Bligh (1): Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm hw/acpi/piix4.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- hw/core/machine.c | 16 ++++++++++++++++ hw/i386/pc.c | 1 + hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++ hw/timer/i8254_common.c | 11 ++++++++++- include/hw/boards.h | 7 +++++++ vl.c | 7 +++++++ 7 files changed, 111 insertions(+), 3 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-22 19:34 [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm Alex Bligh @ 2014-09-22 19:34 ` Alex Bligh 2014-09-28 15:30 ` Michael S. Tsirkin 2014-09-24 8:05 ` Markus Armbruster 2014-09-24 8:38 ` Michael Tokarev 2 siblings, 1 reply; 18+ messages in thread From: Alex Bligh @ 2014-09-22 19:34 UTC (permalink / raw) To: qemu-devel Cc: Ryan Harper, Serge Hallyn, Michael S. Tsirkin, Libvirt, Serge Hallyn, Alexander Graf, Bruce Rogers, quintela, Alex Bligh, Cole Robinson, Amit Shah, Andreas Färber, Serge E. Hallyn Add a machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm version 1.0. Usage: -machine pc-1.0,qemu-kvm-migration=on This has three effects: 1. cirrus-vga.vgamem_mb defaults to 16 rather than 8 2. A type 2 piix4_pm record is interpreted as being type 3 3. A type 2 i8254 PIT common state is interpreted as being type 3 Signed-off-by: Alex Bligh <alex@alex.org.uk> --- hw/acpi/piix4.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- hw/core/machine.c | 16 ++++++++++++++++ hw/i386/pc.c | 1 + hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++ hw/timer/i8254_common.c | 11 ++++++++++- include/hw/boards.h | 7 +++++++ vl.c | 7 +++++++ 7 files changed, 111 insertions(+), 3 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index b72b34e..3c9da23 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -200,12 +200,26 @@ static const VMStateDescription vmstate_pci_status = { } }; +static const VMStateDescription vmstate_acpi_compat; + 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) && + qemu_opt_get_bool(qemu_get_machine_opts(), "qemu-kvm-migration", + DEFAULT_QEMU_KVM_MIGRATION)) { + 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,8 +281,9 @@ 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 + * 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 (which breaks migration from * qemu 1.2). * */ @@ -307,6 +322,34 @@ static const VMStateDescription vmstate_acpi = { } }; +static const VMStateDescription vmstate_acpi_compat = { + .name = "piix4_pm", + .version_id = 3, + .minimum_version_id = 2, + .minimum_version_id_old = 1, + .load_state_old = NULL, /* to avoid recursion */ + .post_load = vmstate_acpi_post_load, + .fields = (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() + } +}; + static void piix4_reset(void *opaque) { PIIX4PMState *s = opaque; diff --git a/hw/core/machine.c b/hw/core/machine.c index 7a66c57..bbccba9 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -235,6 +235,20 @@ static void machine_set_firmware(Object *obj, const char *value, Error **errp) ms->firmware = g_strdup(value); } +static bool machine_get_qemu_kvm_migration(Object *obj, Error **errp) +{ + MachineState *ms = MACHINE(obj); + + return ms->qemu_kvm_migration; +} + +static void machine_set_qemu_kvm_migration(Object *obj, bool value, Error **errp) +{ + MachineState *ms = MACHINE(obj); + + ms->qemu_kvm_migration = value; +} + static void machine_initfn(Object *obj) { object_property_add_str(obj, "accel", @@ -274,6 +288,8 @@ static void machine_initfn(Object *obj) object_property_add_bool(obj, "usb", machine_get_usb, machine_set_usb, NULL); object_property_add_str(obj, "firmware", machine_get_firmware, machine_set_firmware, NULL); + object_property_add_bool(obj, "qemu-kvm-migration", + machine_get_qemu_kvm_migration, machine_set_qemu_kvm_migration, NULL); } static void machine_finalize(Object *obj) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 2cf22b1..c74f2f9 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1516,6 +1516,7 @@ static void pc_generic_machine_class_init(ObjectClass *oc, void *data) mc->alias = qm->alias; mc->desc = qm->desc; mc->init = qm->init; + mc->earlyinit = qm->earlyinit; mc->reset = qm->reset; mc->hot_add_cpu = qm->hot_add_cpu; mc->kvm_type = qm->kvm_type; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 7081c08..0f122e2 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -386,6 +386,21 @@ static void pc_init_pci_1_2(MachineState *machine) pc_init_pci(machine); } +static void pc_early_init_pci_1_0(MachineState *machine) +{ + /* default value of cirrus-vga.vgamem_mb should be 16 if migrating from qemu-kvm */ + if (qemu_opt_get_bool(qemu_get_machine_opts(), "qemu-kvm-migration", + DEFAULT_QEMU_KVM_MIGRATION)) { + GlobalProperty *gp; + MachineClass *mc = MACHINE_GET_CLASS(machine); + for (gp = mc->compat_props; gp->driver; gp++) { + if (!strcmp(gp->driver, "cirrus-vga") && !strcmp(gp->property, "vgamem_mb")) { + gp->value = stringify(16); + } + } + } +} + /* PC init function for pc-0.10 to pc-0.13 */ static void pc_init_pci_no_kvmclock(MachineState *machine) { @@ -614,6 +629,11 @@ static QEMUMachine pc_machine_v1_1 = { }, }; +/* 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 + */ + #define PC_COMPAT_1_0 \ PC_COMPAT_1_1,\ {\ @@ -632,10 +652,15 @@ static QEMUMachine pc_machine_v1_1 = { .driver = TYPE_USB_DEVICE,\ .property = "full-path",\ .value = "no",\ + },{\ + .driver = "cirrus-vga",\ + .property = "vgamem_mb",\ + .value = stringify(8),\ } static QEMUMachine pc_machine_v1_0 = { PC_I440FX_1_2_MACHINE_OPTIONS, + .earlyinit = pc_early_init_pci_1_0, .name = "pc-1.0", .compat_props = (GlobalProperty[]) { PC_COMPAT_1_0, diff --git a/hw/timer/i8254_common.c b/hw/timer/i8254_common.c index 07345f6..0342e93 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) +{ + return (version_id >= 3) || + ((version_id == 2) && + qemu_opt_get_bool(qemu_get_machine_opts(), "qemu-kvm-migration", + DEFAULT_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, diff --git a/include/hw/boards.h b/include/hw/boards.h index 605a970..ffde5dd 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -13,6 +13,8 @@ typedef struct MachineState MachineState; typedef void QEMUMachineInitFunc(MachineState *ms); +typedef void QEMUMachineEarlyInitFunc(MachineState *ms); + typedef void QEMUMachineResetFunc(void); typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp); @@ -24,6 +26,7 @@ struct QEMUMachine { const char *alias; const char *desc; QEMUMachineInitFunc *init; + QEMUMachineEarlyInitFunc *earlyinit; QEMUMachineResetFunc *reset; QEMUMachineHotAddCPUFunc *hot_add_cpu; QEMUMachineGetKvmtypeFunc *kvm_type; @@ -59,6 +62,8 @@ int qemu_register_machine(QEMUMachine *m); #define MACHINE_CLASS(klass) \ OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE) +#define DEFAULT_QEMU_KVM_MIGRATION false + MachineClass *find_default_machine(void); extern MachineState *current_machine; @@ -81,6 +86,7 @@ struct MachineClass { const char *desc; void (*init)(MachineState *state); + void (*earlyinit)(MachineState *state); void (*reset)(void); void (*hot_add_cpu)(const int64_t id, Error **errp); int (*kvm_type)(const char *arg); @@ -123,6 +129,7 @@ struct MachineState { bool mem_merge; bool usb; char *firmware; + bool qemu_kvm_migration; ram_addr_t ram_size; ram_addr_t maxram_size; diff --git a/vl.c b/vl.c index fe451aa..622afea 100644 --- a/vl.c +++ b/vl.c @@ -387,6 +387,10 @@ static QemuOptsList qemu_machine_opts = { .name = PC_MACHINE_MAX_RAM_BELOW_4G, .type = QEMU_OPT_SIZE, .help = "maximum ram below the 4G boundary (32bit boundary)", + },{ + .name = "qemu-kvm-migration", + .type = QEMU_OPT_BOOL, + .help = "enable/disable migration compatibility from qemu-kvm", }, { /* End of list */ } }, @@ -1557,6 +1561,7 @@ static void machine_class_init(ObjectClass *oc, void *data) mc->alias = qm->alias; mc->desc = qm->desc; mc->init = qm->init; + mc->earlyinit = qm->earlyinit; mc->reset = qm->reset; mc->hot_add_cpu = qm->hot_add_cpu; mc->kvm_type = qm->kvm_type; @@ -4003,6 +4008,8 @@ int main(int argc, char **argv, char **envp) OBJECT_CLASS(machine_class)))); object_property_add_child(object_get_root(), "machine", OBJECT(current_machine), &error_abort); + if (machine_class->earlyinit) + machine_class->earlyinit(current_machine); cpu_exec_init_all(); if (machine_class->hw_version) { -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-22 19:34 ` Alex Bligh @ 2014-09-28 15:30 ` Michael S. Tsirkin 2014-09-28 20:33 ` Alex Bligh 2014-10-04 16:29 ` Alex Bligh 0 siblings, 2 replies; 18+ messages in thread From: Michael S. Tsirkin @ 2014-09-28 15:30 UTC (permalink / raw) To: Alex Bligh Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn On Mon, Sep 22, 2014 at 08:34:56PM +0100, Alex Bligh wrote: > Add a machine parameter qemu-kvm-migration for live migrate compatibility > with qemu-kvm version 1.0. Usage: > -machine pc-1.0,qemu-kvm-migration=on I would rename "migration" to "compatibility" everywhere. Though it's a matter of taste, if you strongly prefer this one, go ahead. > > This has three effects: > 1. cirrus-vga.vgamem_mb defaults to 16 rather than 8 > 2. A type 2 piix4_pm record is interpreted as being type 3 > 3. A type 2 i8254 PIT common state is interpreted as being type 3 > > Signed-off-by: Alex Bligh <alex@alex.org.uk> Mostly looks good to me, except: > --- > hw/acpi/piix4.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- > hw/core/machine.c | 16 ++++++++++++++++ > hw/i386/pc.c | 1 + > hw/i386/pc_piix.c | 25 +++++++++++++++++++++++++ > hw/timer/i8254_common.c | 11 ++++++++++- > include/hw/boards.h | 7 +++++++ > vl.c | 7 +++++++ > 7 files changed, 111 insertions(+), 3 deletions(-) > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index b72b34e..3c9da23 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -200,12 +200,26 @@ static const VMStateDescription vmstate_pci_status = { > } > }; > > +static const VMStateDescription vmstate_acpi_compat; > + don't forward declare things, put them right here. > 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) && drop () around comparison pls > + qemu_opt_get_bool(qemu_get_machine_opts(), "qemu-kvm-migration", > + DEFAULT_QEMU_KVM_MIGRATION)) { > + return vmstate_load_state(f, &vmstate_acpi_compat, > + opaque, version_id); > + } else if version_id == 2 return -EINVAL? > + > ret = pci_device_load(PCI_DEVICE(s), f); > if (ret < 0) { > return ret; > @@ -267,8 +281,9 @@ 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 > + * 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 (which breaks migration from > * qemu 1.2). Actually it's version 3 that breaks migration right? Pls say this explicitly: s/which/version 3 breaks migration from qemu 1.2/ > * > */ > @@ -307,6 +322,34 @@ static const VMStateDescription vmstate_acpi = { > } > }; > > +static const VMStateDescription vmstate_acpi_compat = { > + .name = "piix4_pm", > + .version_id = 3, > + .minimum_version_id = 2, > + .minimum_version_id_old = 1, > + .load_state_old = NULL, /* to avoid recursion */ > + .post_load = vmstate_acpi_post_load, > + .fields = (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() > + } > +}; > + 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? > static void piix4_reset(void *opaque) > { > PIIX4PMState *s = opaque; > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 7a66c57..bbccba9 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -235,6 +235,20 @@ static void machine_set_firmware(Object *obj, const char *value, Error **errp) > ms->firmware = g_strdup(value); > } > > +static bool machine_get_qemu_kvm_migration(Object *obj, Error **errp) > +{ > + MachineState *ms = MACHINE(obj); > + > + return ms->qemu_kvm_migration; > +} > + > +static void machine_set_qemu_kvm_migration(Object *obj, bool value, Error **errp) > +{ > + MachineState *ms = MACHINE(obj); > + > + ms->qemu_kvm_migration = value; > +} > + > static void machine_initfn(Object *obj) > { > object_property_add_str(obj, "accel", > @@ -274,6 +288,8 @@ static void machine_initfn(Object *obj) > object_property_add_bool(obj, "usb", machine_get_usb, machine_set_usb, NULL); > object_property_add_str(obj, "firmware", > machine_get_firmware, machine_set_firmware, NULL); > + object_property_add_bool(obj, "qemu-kvm-migration", > + machine_get_qemu_kvm_migration, machine_set_qemu_kvm_migration, NULL); > } > > static void machine_finalize(Object *obj) > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 2cf22b1..c74f2f9 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1516,6 +1516,7 @@ static void pc_generic_machine_class_init(ObjectClass *oc, void *data) > mc->alias = qm->alias; > mc->desc = qm->desc; > mc->init = qm->init; > + mc->earlyinit = qm->earlyinit; > mc->reset = qm->reset; > mc->hot_add_cpu = qm->hot_add_cpu; > mc->kvm_type = qm->kvm_type; > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index 7081c08..0f122e2 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -386,6 +386,21 @@ static void pc_init_pci_1_2(MachineState *machine) > pc_init_pci(machine); > } > > +static void pc_early_init_pci_1_0(MachineState *machine) > +{ > + /* default value of cirrus-vga.vgamem_mb should be 16 if migrating from qemu-kvm */ > + if (qemu_opt_get_bool(qemu_get_machine_opts(), "qemu-kvm-migration", > + DEFAULT_QEMU_KVM_MIGRATION)) { > + GlobalProperty *gp; > + MachineClass *mc = MACHINE_GET_CLASS(machine); > + for (gp = mc->compat_props; gp->driver; gp++) { > + if (!strcmp(gp->driver, "cirrus-vga") && !strcmp(gp->property, "vgamem_mb")) { > + gp->value = stringify(16); > + } > + } > + } > +} > + > /* PC init function for pc-0.10 to pc-0.13 */ > static void pc_init_pci_no_kvmclock(MachineState *machine) > { > @@ -614,6 +629,11 @@ static QEMUMachine pc_machine_v1_1 = { > }, > }; > > +/* 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 > + */ > + 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? > #define PC_COMPAT_1_0 \ > PC_COMPAT_1_1,\ > {\ > @@ -632,10 +652,15 @@ static QEMUMachine pc_machine_v1_1 = { > .driver = TYPE_USB_DEVICE,\ > .property = "full-path",\ > .value = "no",\ > + },{\ > + .driver = "cirrus-vga",\ > + .property = "vgamem_mb",\ > + .value = stringify(8),\ > } > > static QEMUMachine pc_machine_v1_0 = { > PC_I440FX_1_2_MACHINE_OPTIONS, > + .earlyinit = pc_early_init_pci_1_0, > .name = "pc-1.0", > .compat_props = (GlobalProperty[]) { > PC_COMPAT_1_0, > diff --git a/hw/timer/i8254_common.c b/hw/timer/i8254_common.c > index 07345f6..0342e93 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) > +{ > + return (version_id >= 3) || > + ((version_id == 2) && > + qemu_opt_get_bool(qemu_get_machine_opts(), "qemu-kvm-migration", > + DEFAULT_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, > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 605a970..ffde5dd 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -13,6 +13,8 @@ typedef struct MachineState MachineState; > > typedef void QEMUMachineInitFunc(MachineState *ms); > > +typedef void QEMUMachineEarlyInitFunc(MachineState *ms); > + > typedef void QEMUMachineResetFunc(void); > > typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp); > @@ -24,6 +26,7 @@ struct QEMUMachine { > const char *alias; > const char *desc; > QEMUMachineInitFunc *init; > + QEMUMachineEarlyInitFunc *earlyinit; > QEMUMachineResetFunc *reset; > QEMUMachineHotAddCPUFunc *hot_add_cpu; > QEMUMachineGetKvmtypeFunc *kvm_type; > @@ -59,6 +62,8 @@ int qemu_register_machine(QEMUMachine *m); > #define MACHINE_CLASS(klass) \ > OBJECT_CLASS_CHECK(MachineClass, (klass), TYPE_MACHINE) > > +#define DEFAULT_QEMU_KVM_MIGRATION false > + > MachineClass *find_default_machine(void); > extern MachineState *current_machine; > > @@ -81,6 +86,7 @@ struct MachineClass { > const char *desc; > > void (*init)(MachineState *state); > + void (*earlyinit)(MachineState *state); > void (*reset)(void); > void (*hot_add_cpu)(const int64_t id, Error **errp); > int (*kvm_type)(const char *arg); > @@ -123,6 +129,7 @@ struct MachineState { > bool mem_merge; > bool usb; > char *firmware; > + bool qemu_kvm_migration; > > ram_addr_t ram_size; > ram_addr_t maxram_size; > diff --git a/vl.c b/vl.c > index fe451aa..622afea 100644 > --- a/vl.c > +++ b/vl.c > @@ -387,6 +387,10 @@ static QemuOptsList qemu_machine_opts = { > .name = PC_MACHINE_MAX_RAM_BELOW_4G, > .type = QEMU_OPT_SIZE, > .help = "maximum ram below the 4G boundary (32bit boundary)", > + },{ > + .name = "qemu-kvm-migration", > + .type = QEMU_OPT_BOOL, > + .help = "enable/disable migration compatibility from qemu-kvm", > }, > { /* End of list */ } > }, > @@ -1557,6 +1561,7 @@ static void machine_class_init(ObjectClass *oc, void *data) > mc->alias = qm->alias; > mc->desc = qm->desc; > mc->init = qm->init; > + mc->earlyinit = qm->earlyinit; > mc->reset = qm->reset; > mc->hot_add_cpu = qm->hot_add_cpu; > mc->kvm_type = qm->kvm_type; > @@ -4003,6 +4008,8 @@ int main(int argc, char **argv, char **envp) > OBJECT_CLASS(machine_class)))); > object_property_add_child(object_get_root(), "machine", > OBJECT(current_machine), &error_abort); > + if (machine_class->earlyinit) > + machine_class->earlyinit(current_machine); > cpu_exec_init_all(); > > if (machine_class->hw_version) { > -- > 1.7.9.5 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-28 15:30 ` Michael S. Tsirkin @ 2014-09-28 20:33 ` Alex Bligh 2014-09-29 7:02 ` Markus Armbruster 2014-09-29 10:08 ` Michael S. Tsirkin 2014-10-04 16:29 ` Alex Bligh 1 sibling, 2 replies; 18+ messages in thread From: Alex Bligh @ 2014-09-28 20:33 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, Alex Bligh, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn Michael, >> +static const VMStateDescription vmstate_acpi_compat = { >> + .name = "piix4_pm", >> + .version_id = 3, >> + .minimum_version_id = 2, >> + .minimum_version_id_old = 1, >> + .load_state_old = NULL, /* to avoid recursion */ >> + .post_load = vmstate_acpi_post_load, >> + .fields = (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() >> + } >> +}; >> + > > 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 >> + */ >> + > > 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). -- Alex Bligh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-28 20:33 ` Alex Bligh @ 2014-09-29 7:02 ` Markus Armbruster 2014-10-05 7:00 ` Paolo Bonzini 2014-09-29 10:08 ` Michael S. Tsirkin 1 sibling, 1 reply; 18+ messages in thread From: Markus Armbruster @ 2014-09-29 7:02 UTC (permalink / raw) To: Alex Bligh Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, Michael S. Tsirkin, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn Alex Bligh <alex@alex.org.uk> writes: [...] >>> +/* 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 >>> + */ >>> + >> >> 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). I also prefer a new machine type. Ideally, the management application understands that there are two incompatibile versions QEMU (upstream and old qemu-kvm), and how to map their machine types to current QEMU's. If that's not practical, then downstream can still alias the machine types around to make things just work in the most important downstream scenarios. The most important upstream scenario is QEMU <-> QEMU, of course. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-29 7:02 ` Markus Armbruster @ 2014-10-05 7:00 ` Paolo Bonzini 2014-10-05 10:26 ` Alex Bligh 0 siblings, 1 reply; 18+ messages in thread From: Paolo Bonzini @ 2014-10-05 7:00 UTC (permalink / raw) To: Markus Armbruster, Alex Bligh Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, Michael S. Tsirkin, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn Il 29/09/2014 09:02, Markus Armbruster ha scritto: >> 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. I was objecting to making pc-1.0 special. There's nothing special in pc-1.0, other machine types also had differences between qemu-kvm and qemu. And I do not think that upstream has any reason to make pc-1.0 special. So, if Ubuntu is okay with breaking pc-1.0 migration from 14.04-old to 14.04-new, the right thing to do is simply that Ubuntu makes its pc-1.0 machine type the qemu-kvm one. No new machine types, no aliases, no anything. For upstream, the option is acceptable because that one applies just as well to other types than 1.0. Other distros included 0.15 or 1.2, and can use the option as well. >> 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). You can add a new property to the machine, and do the early_init work in the property setter, I think. > I also prefer a new machine type. > > Ideally, the management application understands that there are two > incompatibile versions QEMU (upstream and old qemu-kvm), and how to map > their machine types to current QEMU's. That would mean patching the Ubuntu libvirt, right? At this point it's simpler to just patch Ubuntu QEMU to do what you write here: > If that's not practical, then downstream can still alias the machine > types around to make things just work in the most important downstream > scenarios. The most important upstream scenario is QEMU <-> QEMU, of > course. I'm not sure why this patch is of any interest upstream... Paolo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-10-05 7:00 ` Paolo Bonzini @ 2014-10-05 10:26 ` Alex Bligh 2014-10-05 12:26 ` Paolo Bonzini 0 siblings, 1 reply; 18+ messages in thread From: Alex Bligh @ 2014-10-05 10:26 UTC (permalink / raw) To: Paolo Bonzini Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, Markus Armbruster, qemu-devel, Alexander Graf, Michael S. Tsirkin, Alex Bligh, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn On 5 Oct 2014, at 08:00, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 29/09/2014 09:02, Markus Armbruster ha scritto: >>> 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. > > I was objecting to making pc-1.0 special. There's nothing special in > pc-1.0, other machine types also had differences between qemu-kvm and > qemu. And I do not think that upstream has any reason to make pc-1.0 > special. OK, so in v5, pc-1.0 is unchanged, and not made special. A new machine type is added which allows import from something (unfortunately) called pc-1.0 in something in qemu's past, as well as some distributions. > So, if Ubuntu is okay with breaking pc-1.0 migration from 14.04-old to > 14.04-new, the right thing to do is simply that Ubuntu makes its pc-1.0 > machine type the qemu-kvm one. No new machine types, no aliases, no > anything. That would not allow Ubuntu (or Suse - similarly affected I think) to import pc-1.0 VMs from things actually running pc-1.0, and would mean that newly created pc-1.0 VMs would be 'wrong', perpetuating the problem. -- Alex Bligh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-10-05 10:26 ` Alex Bligh @ 2014-10-05 12:26 ` Paolo Bonzini 2014-10-05 12:48 ` Michael S. Tsirkin 0 siblings, 1 reply; 18+ messages in thread From: Paolo Bonzini @ 2014-10-05 12:26 UTC (permalink / raw) To: Alex Bligh Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, Markus Armbruster, qemu-devel, Alexander Graf, Michael S. Tsirkin, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn > > > 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. > > > > I was objecting to making pc-1.0 special. There's nothing special in > > pc-1.0, other machine types also had differences between qemu-kvm and > > qemu. And I do not think that upstream has any reason to make pc-1.0 > > special. > > OK, so in v5, pc-1.0 is unchanged, and not made special. A new machine > type is added which allows import from something (unfortunately) called > pc-1.0 in something in qemu's past, as well as some distributions. The very fact that a clone of pc-1.0 is added, but not pc-0.15 or pc-1.2, makes pc-1.0 special. My proposal has always been that _all_ PC machines should have a property. This could be done, for example, by starting with Eduardo's patches that make a class hierarchy of PC machine types. pc-1.0 is special in Ubuntu world, because it was in an LTS release. This is why this patch should be added to Ubuntu, not to upstream. pc-1.0 is not necessarily special in other distros. And some of them (Fedora for example) are _always_ treating their machine types as the qemu-kvm variants, and have been doing so for a couple years. I very strongly object to including a patch upstream that is tailored after a particular downstream, and just because that particular downstream has failed in doing the integration testing that it was supposed to do. > > So, if Ubuntu is okay with breaking pc-1.0 migration from 14.04-old to > > 14.04-new, the right thing to do is simply that Ubuntu makes its pc-1.0 > > machine type the qemu-kvm one. No new machine types, no aliases, no > > anything. > > That would not allow Ubuntu (or Suse - similarly affected I think) > to import pc-1.0 VMs from things actually running pc-1.0, and would > mean that newly created pc-1.0 VMs would be 'wrong', perpetuating > the problem. The problem _is_ perpetual. "pc-1.0" makes no sense without a context (the distro). This is why it is not a problem for Fedora to always use the qemu-kvm variants. "pc-1.0" will always be the qemu-kvm variant in Ubuntu context (apart from the 6 months passed since the release of 14.04), because most "pc-1.0" machines will have been created with the qemu-kvm package in Ubuntu 12.04. It can be confusing---to avoid confusion, RHEL drops the upstream machine types apart from the "pc" generic type and adopts a completely different nomenclature. In fact as time passes the benefit of 12.04->14.04 migration becomes smaller and smaller and, by now, it should have gone almost completely. It's much simpler to ignore the problem at this point. For the next Ubuntu LTS, Canonical should include migration compatibility in their test plans. And start well in advance, for it may only take one person to fix the bugs, but it takes months of testing to find them. Paolo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-10-05 12:26 ` Paolo Bonzini @ 2014-10-05 12:48 ` Michael S. Tsirkin 2014-10-05 13:30 ` Paolo Bonzini 0 siblings, 1 reply; 18+ messages in thread From: Michael S. Tsirkin @ 2014-10-05 12:48 UTC (permalink / raw) To: Paolo Bonzini Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, Markus Armbruster, qemu-devel, Alexander Graf, Alex Bligh, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn On Sun, Oct 05, 2014 at 08:26:45AM -0400, Paolo Bonzini wrote: > > > > 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. > > > > > > I was objecting to making pc-1.0 special. There's nothing special in > > > pc-1.0, other machine types also had differences between qemu-kvm and > > > qemu. And I do not think that upstream has any reason to make pc-1.0 > > > special. > > > > OK, so in v5, pc-1.0 is unchanged, and not made special. A new machine > > type is added which allows import from something (unfortunately) called > > pc-1.0 in something in qemu's past, as well as some distributions. > > The very fact that a clone of pc-1.0 is added, but not pc-0.15 or > pc-1.2, makes pc-1.0 special. > > My proposal has always been that _all_ PC machines should have a property. > This could be done, for example, by starting with Eduardo's patches that > make a class hierarchy of PC machine types. > > pc-1.0 is special in Ubuntu world, because it was in an LTS release. This > is why this patch should be added to Ubuntu, not to upstream. pc-1.0 is > not necessarily special in other distros. And some of them (Fedora for > example) are _always_ treating their machine types as the qemu-kvm variants, > and have been doing so for > a couple years. > > I very strongly object to including a patch upstream that is tailored > after a particular downstream, and just because that particular downstream > has failed in doing the integration testing that it was supposed to do. > > > > So, if Ubuntu is okay with breaking pc-1.0 migration from 14.04-old to > > > 14.04-new, the right thing to do is simply that Ubuntu makes its pc-1.0 > > > machine type the qemu-kvm one. No new machine types, no aliases, no > > > anything. > > > > That would not allow Ubuntu (or Suse - similarly affected I think) > > to import pc-1.0 VMs from things actually running pc-1.0, and would > > mean that newly created pc-1.0 VMs would be 'wrong', perpetuating > > the problem. > > The problem _is_ perpetual. "pc-1.0" makes no sense without a context > (the distro). This is why it is not a problem for Fedora to always use > the qemu-kvm variants. > > "pc-1.0" will always be the qemu-kvm variant in Ubuntu context (apart > from the 6 months passed since the release of 14.04), because most > "pc-1.0" machines will have been created with the qemu-kvm package in > Ubuntu 12.04. > > It can be confusing---to avoid confusion, RHEL drops the upstream machine > types apart from the "pc" generic type and adopts a completely different > nomenclature. > > In fact as time passes the benefit of 12.04->14.04 migration becomes > smaller and smaller and, by now, it should have gone almost completely. > It's much simpler to ignore the problem at this point. For the next > Ubuntu LTS, Canonical should include migration compatibility in their > test plans. And start well in advance, for it may only take one > person to fix the bugs, but it takes months of testing to find them. > > Paolo In fact, if the pc_piix bits are dropped from the patch, you get a generic patchset that does exactly what you ask, correct? Downstream can then enable qemu-kvm compatibility by adding: -global cirrus-vga.vgamem_mb=16 -global pit-common.qemu-kvm-migration=on -global PIIX4_PM.qemu-kvm-migration=on -- MST ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-10-05 12:48 ` Michael S. Tsirkin @ 2014-10-05 13:30 ` Paolo Bonzini 0 siblings, 0 replies; 18+ messages in thread From: Paolo Bonzini @ 2014-10-05 13:30 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, Markus Armbruster, qemu-devel, Alexander Graf, Alex Bligh, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn Il 05/10/2014 14:48, Michael S. Tsirkin ha scritto: > In fact, if the pc_piix bits are dropped from the patch, > you get a generic patchset that does exactly what you ask, > correct? > > Downstream can then enable qemu-kvm compatibility by adding: > > -global cirrus-vga.vgamem_mb=16 -global pit-common.qemu-kvm-migration=on > -global PIIX4_PM.qemu-kvm-migration=on Yes. Paolo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-28 20:33 ` Alex Bligh 2014-09-29 7:02 ` Markus Armbruster @ 2014-09-29 10:08 ` Michael S. Tsirkin 2014-09-29 10:13 ` Alex Bligh 1 sibling, 1 reply; 18+ messages in thread From: Michael S. Tsirkin @ 2014-09-29 10:08 UTC (permalink / raw) To: Alex Bligh Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn On Sun, Sep 28, 2014 at 09:33:08PM +0100, Alex Bligh wrote: > 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. I think same applies to v3 that I reviewed right? Absolutely, I'm fine with just a new machine type. This means that management tools will need to learn to add -qemu-kvm suffix to the machine name if user requested compatibility with qemu-kvm. I think there were some implementation issues with patch 1/2 though. > 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). Good. > -- > Alex Bligh > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-29 10:08 ` Michael S. Tsirkin @ 2014-09-29 10:13 ` Alex Bligh 2014-09-29 14:52 ` Serge E. Hallyn 0 siblings, 1 reply; 18+ messages in thread From: Alex Bligh @ 2014-09-29 10:13 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, Alex Bligh, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn On 29 Sep 2014, at 11:08, Michael S. Tsirkin <mst@redhat.com> wrote: > On Sun, Sep 28, 2014 at 09:33:08PM +0100, Alex Bligh wrote: >> 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. > > I think same applies to v3 that I reviewed right? > Absolutely, I'm fine with just a new machine type. > This means that management tools will need to learn to > add -qemu-kvm suffix to the machine name if user > requested compatibility with qemu-kvm. > I think there were some implementation issues with patch 1/2 > though. > >> 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). > > Good. OK, I will respin this when I get a chance with the new machine type back and hopefully address some of the other issues you brought up. -- Alex Bligh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-29 10:13 ` Alex Bligh @ 2014-09-29 14:52 ` Serge E. Hallyn 0 siblings, 0 replies; 18+ messages in thread From: Serge E. Hallyn @ 2014-09-29 14:52 UTC (permalink / raw) To: Alex Bligh Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, Michael S. Tsirkin, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn Quoting Alex Bligh (alex@alex.org.uk): > > On 29 Sep 2014, at 11:08, Michael S. Tsirkin <mst@redhat.com> wrote: > > > On Sun, Sep 28, 2014 at 09:33:08PM +0100, Alex Bligh wrote: > >> 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. > > > > I think same applies to v3 that I reviewed right? > > Absolutely, I'm fine with just a new machine type. > > This means that management tools will need to learn to > > add -qemu-kvm suffix to the machine name if user > > requested compatibility with qemu-kvm. > > I think there were some implementation issues with patch 1/2 > > though. > > > >> 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). > > > > Good. > > OK, I will respin this when I get a chance with the new machine > type back and hopefully address some of the other issues you > brought up. Thanks, Alex. I've got working packages for 12.04, 14,04 and 14.10 at ppa:serge-hallyn/qemu-migration2 and have opened three bugs (https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1374622, https://bugs.launchpad.net/ubuntu/+bug/1374617, and https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1374612) to get them into the archive. However if the machine type patches may end up upstream, then obviously I'd prefer to wait for the final upstream set. As we're approaching final freeze for 14.10, I suspect this will mean getting the patches into 15.04 on the first day of the cycle (no FFEs needed there) and then a quick SRU into 12.04 and 14.04. -serge ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-28 15:30 ` Michael S. Tsirkin 2014-09-28 20:33 ` Alex Bligh @ 2014-10-04 16:29 ` Alex Bligh 1 sibling, 0 replies; 18+ messages in thread From: Alex Bligh @ 2014-10-04 16:29 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Ryan Harper, Serge Hallyn, quintela, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, Alex Bligh, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn Michael, I'm about to post a nice simple v5 of this, but there are a couple of your comments I am NOT addressing: >> >> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c >> index b72b34e..3c9da23 100644 >> --- a/hw/acpi/piix4.c >> +++ b/hw/acpi/piix4.c >> @@ -200,12 +200,26 @@ static const VMStateDescription vmstate_pci_status = { >> } >> }; >> >> +static const VMStateDescription vmstate_acpi_compat; >> + > > don't forward declare things, put them right here. This is not addressable in v5; in v5 this is a forward declaration of vmstate_acpi (not _compat). vmstate_acpi references acpi_load_old, and acpi_load_old references vmstate_acpi, so we need a forward reference for one of them in any case. >> + qemu_opt_get_bool(qemu_get_machine_opts(), "qemu-kvm-migration", >> + DEFAULT_QEMU_KVM_MIGRATION)) { >> + return vmstate_load_state(f, &vmstate_acpi_compat, >> + opaque, version_id); >> + } > > else if version_id == 2 return -EINVAL? This is incorrect I think. A version_id of 2 with qemu-kvm_migration off is permissible, and indicates an inbound migration from qemu-git 1.2, i.e. the old manual loading should be run. >> /* 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 >> + * 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 (which breaks migration from >> * qemu 1.2). > > Actually it's version 3 that breaks migration right? > Pls say this explicitly: s/which/version 3 breaks migration from qemu 1.2/ It's changing the minimum version ID to 2 (from 3) which would break migration from qemu (git) 1.2, because that uses a version ID of 2, and we want the old loader to run in that case. I've made this clearer. -- Alex Bligh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-22 19:34 [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm Alex Bligh 2014-09-22 19:34 ` Alex Bligh @ 2014-09-24 8:05 ` Markus Armbruster 2014-09-24 8:29 ` Alex Bligh 2014-09-24 8:38 ` Michael Tokarev 2 siblings, 1 reply; 18+ messages in thread From: Markus Armbruster @ 2014-09-24 8:05 UTC (permalink / raw) To: Alex Bligh Cc: Ryan Harper, Serge Hallyn, Michael S. Tsirkin, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, quintela, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn Alex Bligh <alex@alex.org.uk> writes: > This patch series 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 > parameter (qemu-kvm-migration) which when turned on affects the pc-1.0 > machine type. Usage: > -machine pc-1.0,qemu-kvm-migration=on Forgive me if this has been discussed already: why not simply a separate machine type "pc-1.0-qemu-kvm"? > Three aproaches are taken: > > * cirrus-vga.vgamem_mb defaults to 16 rather than 8. In order to > keep -global cirrus-vga.vgamem_mb working even with > qemu-kvm-migration=on, this is monkey-patched into the default > value of the MachineState structure's compat_props list. This part fires only for pc-1.0, because it's in pc_early_init_pci_1_0(). > * 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. These parts apply to all machine types, don't they? > The above monkey-patching must be done between the selection of > the MachineClass and the processing of the machine parameters > (on the one hand) and the processing of the compat_props list > and the globals on the command line. To do this I have added > an earlyinit function to MachineState and QEMUMachine. > > 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 since v1: > * Do not use a machine type, use a machine parameter. Okay, it has been discussed already. I'd appreciate a brief recap all the same. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-24 8:05 ` Markus Armbruster @ 2014-09-24 8:29 ` Alex Bligh 0 siblings, 0 replies; 18+ messages in thread From: Alex Bligh @ 2014-09-24 8:29 UTC (permalink / raw) To: Markus Armbruster Cc: Ryan Harper, Serge Hallyn, Michael S. Tsirkin, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, quintela, Alex Bligh, Cole Robinson, Amit Shah, Bruce Rogers, Andreas Färber, Serge E. Hallyn Markus, On 24 Sep 2014, at 09:05, Markus Armbruster <armbru@redhat.com> wrote: > Alex Bligh <alex@alex.org.uk> writes: > >> This patch series 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 >> parameter (qemu-kvm-migration) which when turned on affects the pc-1.0 >> machine type. Usage: >> -machine pc-1.0,qemu-kvm-migration=on > > Forgive me if this has been discussed already: why not simply a separate > machine type "pc-1.0-qemu-kvm"? That's what v2 of the patch set does (and I prefer v2). However, mst wanted it done this way. > >> Three aproaches are taken: >> >> * cirrus-vga.vgamem_mb defaults to 16 rather than 8. In order to >> keep -global cirrus-vga.vgamem_mb working even with >> qemu-kvm-migration=on, this is monkey-patched into the default >> value of the MachineState structure's compat_props list. > > This part fires only for pc-1.0, because it's in > pc_early_init_pci_1_0(). Yes, intentional. I should have noted that. That's because qemu-kvm-migration=on the VRAM change was designed to work only with pc-1.0. I haven't looked at trying to make other (older) qemu-kvm machine migrations work, but (with the stuff below), I would guess it would work with the appropriate command line options for VRAM size. Obviously I could put early init elsewhere. >> * 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. > > These parts apply to all machine types, don't they? They apply only when qemu-kvm-migration=on is selected, but to any machine type; however machine types newer than pc-1.0 will be exporting v3 I think anyway. >> Changes since v1: >> * Do not use a machine type, use a machine parameter. > > Okay, it has been discussed already. I'd appreciate a brief recap all > the same. See above. I preferred the machine type. But I got to learn more about QOM on the way :-) -- Alex Bligh ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-22 19:34 [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm Alex Bligh 2014-09-22 19:34 ` Alex Bligh 2014-09-24 8:05 ` Markus Armbruster @ 2014-09-24 8:38 ` Michael Tokarev 2014-09-25 8:02 ` Dr. David Alan Gilbert 2 siblings, 1 reply; 18+ messages in thread From: Michael Tokarev @ 2014-09-24 8:38 UTC (permalink / raw) To: Alex Bligh, qemu-devel Cc: Ryan Harper, Serge Hallyn, Michael S. Tsirkin, Libvirt, Serge Hallyn, Alexander Graf, Bruce Rogers, quintela, Cole Robinson, Amit Shah, Andreas Färber, Serge E. Hallyn 22.09.2014 23:34, Alex Bligh wrote: > This patch series adds inbound migrate capability from qemu-kvm version > 1.0. [...] Isn't it quite a bit too late already? That's an old version by now, and supporting migration from it is interesting for long-term support distributions - like redhat for example, with several years of release cycle. Is it really necessary at this time to add this ability to upstream qemu? It'd be very nice to have this capability right when qemu-kvm tree has been merged (or even before that), but it's been some years ago already. Thanks, /mjt ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm 2014-09-24 8:38 ` Michael Tokarev @ 2014-09-25 8:02 ` Dr. David Alan Gilbert 0 siblings, 0 replies; 18+ messages in thread From: Dr. David Alan Gilbert @ 2014-09-25 8:02 UTC (permalink / raw) To: Michael Tokarev Cc: Ryan Harper, Serge Hallyn, Michael S. Tsirkin, Libvirt, Serge Hallyn, qemu-devel, Alexander Graf, quintela, Alex Bligh, Cole Robinson, Amit Shah, Bruce Rogers, Andreas F?rber, Serge E. Hallyn * Michael Tokarev (mjt@tls.msk.ru) wrote: > 22.09.2014 23:34, Alex Bligh wrote: > > This patch series adds inbound migrate capability from qemu-kvm version > > 1.0. [...] > > Isn't it quite a bit too late already? That's an old version by > now, and supporting migration from it is interesting for long-term > support distributions - like redhat for example, with several > years of release cycle. Is it really necessary at this time to > add this ability to upstream qemu? > > It'd be very nice to have this capability right when qemu-kvm > tree has been merged (or even before that), but it's been some > years ago already. It's amazing what different combinations of QEMU people have out there; and it seems reasonable to let Alex fix this problem if it helps him; there's no reason to deny others the same fix. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2014-10-05 13:30 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-22 19:34 [Qemu-devel] [PATCH v4] Add machine parameter qemu-kvm-migration for live migrate compatibility with qemu-kvm Alex Bligh 2014-09-22 19:34 ` Alex Bligh 2014-09-28 15:30 ` Michael S. Tsirkin 2014-09-28 20:33 ` Alex Bligh 2014-09-29 7:02 ` Markus Armbruster 2014-10-05 7:00 ` Paolo Bonzini 2014-10-05 10:26 ` Alex Bligh 2014-10-05 12:26 ` Paolo Bonzini 2014-10-05 12:48 ` Michael S. Tsirkin 2014-10-05 13:30 ` Paolo Bonzini 2014-09-29 10:08 ` Michael S. Tsirkin 2014-09-29 10:13 ` Alex Bligh 2014-09-29 14:52 ` Serge E. Hallyn 2014-10-04 16:29 ` Alex Bligh 2014-09-24 8:05 ` Markus Armbruster 2014-09-24 8:29 ` Alex Bligh 2014-09-24 8:38 ` Michael Tokarev 2014-09-25 8:02 ` Dr. David Alan Gilbert
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.