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

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

[Alex Bligh]

Add a machine type pc-1.0-qemu-kvm for live migrate compatibility
with qemu-kvm version 1.0.

Signed-off-by: Alex Bligh <alex@alex.org.uk>
---
 hw/acpi/piix4.c          |   49 ++++++++++++++++++++++++++++++++++++++++++++--
 hw/i386/pc_piix.c        |   31 +++++++++++++++++++++++++++++
 hw/timer/i8254_common.c  |   41 ++++++++++++++++++++++++++++++++++++++
 include/hw/acpi/piix4.h  |    1 +
 include/hw/timer/i8254.h |    2 ++
 5 files changed, 122 insertions(+), 2 deletions(-)

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

This requires 'hot patching' the VMStateDescription in a couple of
places, which in turn is less than obvious as there may be (indeed
are for i8259) derived classes. Whilst pretty nausea-inducing, this
approach has the benefit of being entirely self-contained.

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. Testing has been light to date (i.e.
can I migrate it inbound with -S without anything complaining).

I have not (yet) brought forward the qxl rom size (and possibly
video ram) changes in Cole's patches as I'd prefer an assessment
of whether this is the right approach first.

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b72b34e..708db79 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -267,8 +267,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 +308,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 = acpi_load_old,
+    .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;
@@ -619,6 +648,22 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
     adevc->ospm_status = piix4_ospm_status;
 }
 
+void piix4_pm_class_fix_compat(void)
+{
+    GSList *el, *devices = object_class_get_list(TYPE_DEVICE, false);
+    /*
+     * Change the vmstate field in this class and any derived classes
+     * if not overriden. As no other classes should be using this
+     * vmstate, we can simply iterate the class list
+     */
+    for (el = devices; el; el = el->next) {
+        DeviceClass *dc = el->data;
+        if (dc->vmsd == &vmstate_acpi) {
+            dc->vmsd = &vmstate_acpi_compat;
+        }
+    }
+}
+
 static const TypeInfo piix4_pm_info = {
     .name          = TYPE_PIIX4_PM,
     .parent        = TYPE_PCI_DEVICE,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7081c08..e400ea6 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -49,6 +49,8 @@
 #include "hw/acpi/acpi.h"
 #include "cpu.h"
 #include "qemu/error-report.h"
+#include "hw/acpi/piix4.h"
+#include "hw/timer/i8254.h"
 #ifdef CONFIG_XEN
 #  include <xen/hvm/hvm_info_table.h>
 #endif
@@ -386,6 +388,15 @@ static void pc_init_pci_1_2(MachineState *machine)
     pc_init_pci(machine);
 }
 
+/* PC machine init function for qemu-kvm 1.0 */
+static void pc_init_pci_1_2_qemu_kvm(MachineState *machine)
+{
+    piix4_pm_class_fix_compat();
+    pit_common_class_fix_compat();
+    pc_compat_1_2(machine);
+    pc_init_pci(machine);
+}
+
 /* PC init function for pc-0.10 to pc-0.13 */
 static void pc_init_pci_no_kvmclock(MachineState *machine)
 {
@@ -644,6 +655,25 @@ 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),\
+        }
+
+static QEMUMachine pc_machine_v1_0_qemu_kvm = {
+    PC_I440FX_1_2_MACHINE_OPTIONS,
+    .name = "pc-1.0-qemu-kvm",
+    .init = pc_init_pci_1_2_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 +916,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..511ea05 100644
--- a/hw/timer/i8254_common.c
+++ b/hw/timer/i8254_common.c
@@ -275,6 +275,29 @@ static const VMStateDescription vmstate_pit_common = {
     }
 };
 
+static const VMStateDescription vmstate_pit_common_compat = {
+    .name = "i8254",
+    .version_id = 3,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 1,
+    .load_state_old = pit_load_old,
+    .pre_save = pit_dispatch_pre_save,
+    .post_load = pit_dispatch_post_load,
+    .fields = (VMStateField[]) {
+        /* qemu-kvm version_id=2 had 'flags' here which is equivalent
+         * This fixes incoming migration from qemu-kvm 1.0, but breaks
+         * incoming migration from qemu < 1.1
+         */
+        /* VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3), */
+        VMSTATE_UINT32(channels[0].irq_disabled, PITCommonState),
+        VMSTATE_STRUCT_ARRAY(channels, PITCommonState, 3, 2,
+                             vmstate_pit_channel, PITChannelState),
+        VMSTATE_INT64(channels[0].next_transition_time,
+                      PITCommonState), /* formerly irq_timer */
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void pit_common_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -289,6 +312,24 @@ static void pit_common_class_init(ObjectClass *klass, void *data)
     dc->cannot_instantiate_with_device_add_yet = true;
 }
 
+void pit_common_class_fix_compat(void)
+{
+    GSList *el, *devices = object_class_get_list(TYPE_DEVICE, false);
+    /*
+     * Change the vmstate field in this class and any derived classes
+     * if not overriden. As no other classes should be using this
+     * vmstate, we can simply iterate the class list
+     */
+    for (el = devices; el; el = el->next) {
+        DeviceClass *dc = el->data;
+        if (dc->vmsd == &vmstate_pit_common) {
+            dc->vmsd = &vmstate_pit_common_compat;
+        }
+    }
+
+    g_slist_free(devices);
+}
+
 static const TypeInfo pit_common_type = {
     .name          = TYPE_PIT_COMMON,
     .parent        = TYPE_ISA_DEVICE,
diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h
index 65e6fd7..811d88f 100644
--- a/include/hw/acpi/piix4.h
+++ b/include/hw/acpi/piix4.h
@@ -4,5 +4,6 @@
 #include "qemu/typedefs.h"
 
 Object *piix4_pm_find(void);
+void piix4_pm_class_fix_compat(void);
 
 #endif
diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
index 4349033..0126424 100644
--- a/include/hw/timer/i8254.h
+++ b/include/hw/timer/i8254.h
@@ -72,4 +72,6 @@ static inline ISADevice *kvm_pit_init(ISABus *bus, int base)
 void pit_set_gate(ISADevice *dev, int channel, int val);
 void pit_get_channel_info(ISADevice *dev, int channel, PITChannelInfo *info);
 
+void pit_common_class_fix_compat(void);
+
 #endif /* !HW_I8254_H */
-- 
1.7.9.5

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

[Alex Bligh]

ping: anyone got any views on this one?

On 22 Jul 2014, at 19:43, Alex Bligh <alex@alex.org.uk> wrote:

> Add a machine type pc-1.0-qemu-kvm for live migrate compatibility
> with qemu-kvm version 1.0.
> 
> Signed-off-by: Alex Bligh <alex@alex.org.uk>
> ---
> hw/acpi/piix4.c          |   49 ++++++++++++++++++++++++++++++++++++++++++++--
> hw/i386/pc_piix.c        |   31 +++++++++++++++++++++++++++++
> hw/timer/i8254_common.c  |   41 ++++++++++++++++++++++++++++++++++++++
> include/hw/acpi/piix4.h  |    1 +
> include/hw/timer/i8254.h |    2 ++
> 5 files changed, 122 insertions(+), 2 deletions(-)
> 
> This RFC 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.
> 
> This requires 'hot patching' the VMStateDescription in a couple of
> places, which in turn is less than obvious as there may be (indeed
> are for i8259) derived classes. Whilst pretty nausea-inducing, this
> approach has the benefit of being entirely self-contained.
> 
> 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. Testing has been light to date (i.e.
> can I migrate it inbound with -S without anything complaining).
> 
> I have not (yet) brought forward the qxl rom size (and possibly
> video ram) changes in Cole's patches as I'd prefer an assessment
> of whether this is the right approach first.
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index b72b34e..708db79 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -267,8 +267,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 +308,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 = acpi_load_old,
> +    .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;
> @@ -619,6 +648,22 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
>     adevc->ospm_status = piix4_ospm_status;
> }
> 
> +void piix4_pm_class_fix_compat(void)
> +{
> +    GSList *el, *devices = object_class_get_list(TYPE_DEVICE, false);
> +    /*
> +     * Change the vmstate field in this class and any derived classes
> +     * if not overriden. As no other classes should be using this
> +     * vmstate, we can simply iterate the class list
> +     */
> +    for (el = devices; el; el = el->next) {
> +        DeviceClass *dc = el->data;
> +        if (dc->vmsd == &vmstate_acpi) {
> +            dc->vmsd = &vmstate_acpi_compat;
> +        }
> +    }
> +}
> +
> static const TypeInfo piix4_pm_info = {
>     .name          = TYPE_PIIX4_PM,
>     .parent        = TYPE_PCI_DEVICE,
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7081c08..e400ea6 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -49,6 +49,8 @@
> #include "hw/acpi/acpi.h"
> #include "cpu.h"
> #include "qemu/error-report.h"
> +#include "hw/acpi/piix4.h"
> +#include "hw/timer/i8254.h"
> #ifdef CONFIG_XEN
> #  include <xen/hvm/hvm_info_table.h>
> #endif
> @@ -386,6 +388,15 @@ static void pc_init_pci_1_2(MachineState *machine)
>     pc_init_pci(machine);
> }
> 
> +/* PC machine init function for qemu-kvm 1.0 */
> +static void pc_init_pci_1_2_qemu_kvm(MachineState *machine)
> +{
> +    piix4_pm_class_fix_compat();
> +    pit_common_class_fix_compat();
> +    pc_compat_1_2(machine);
> +    pc_init_pci(machine);
> +}
> +
> /* PC init function for pc-0.10 to pc-0.13 */
> static void pc_init_pci_no_kvmclock(MachineState *machine)
> {
> @@ -644,6 +655,25 @@ 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),\
> +        }
> +
> +static QEMUMachine pc_machine_v1_0_qemu_kvm = {
> +    PC_I440FX_1_2_MACHINE_OPTIONS,
> +    .name = "pc-1.0-qemu-kvm",
> +    .init = pc_init_pci_1_2_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 +916,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..511ea05 100644
> --- a/hw/timer/i8254_common.c
> +++ b/hw/timer/i8254_common.c
> @@ -275,6 +275,29 @@ static const VMStateDescription vmstate_pit_common = {
>     }
> };
> 
> +static const VMStateDescription vmstate_pit_common_compat = {
> +    .name = "i8254",
> +    .version_id = 3,
> +    .minimum_version_id = 2,
> +    .minimum_version_id_old = 1,
> +    .load_state_old = pit_load_old,
> +    .pre_save = pit_dispatch_pre_save,
> +    .post_load = pit_dispatch_post_load,
> +    .fields = (VMStateField[]) {
> +        /* qemu-kvm version_id=2 had 'flags' here which is equivalent
> +         * This fixes incoming migration from qemu-kvm 1.0, but breaks
> +         * incoming migration from qemu < 1.1
> +         */
> +        /* VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3), */
> +        VMSTATE_UINT32(channels[0].irq_disabled, PITCommonState),
> +        VMSTATE_STRUCT_ARRAY(channels, PITCommonState, 3, 2,
> +                             vmstate_pit_channel, PITChannelState),
> +        VMSTATE_INT64(channels[0].next_transition_time,
> +                      PITCommonState), /* formerly irq_timer */
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> static void pit_common_class_init(ObjectClass *klass, void *data)
> {
>     DeviceClass *dc = DEVICE_CLASS(klass);
> @@ -289,6 +312,24 @@ static void pit_common_class_init(ObjectClass *klass, void *data)
>     dc->cannot_instantiate_with_device_add_yet = true;
> }
> 
> +void pit_common_class_fix_compat(void)
> +{
> +    GSList *el, *devices = object_class_get_list(TYPE_DEVICE, false);
> +    /*
> +     * Change the vmstate field in this class and any derived classes
> +     * if not overriden. As no other classes should be using this
> +     * vmstate, we can simply iterate the class list
> +     */
> +    for (el = devices; el; el = el->next) {
> +        DeviceClass *dc = el->data;
> +        if (dc->vmsd == &vmstate_pit_common) {
> +            dc->vmsd = &vmstate_pit_common_compat;
> +        }
> +    }
> +
> +    g_slist_free(devices);
> +}
> +
> static const TypeInfo pit_common_type = {
>     .name          = TYPE_PIT_COMMON,
>     .parent        = TYPE_ISA_DEVICE,
> diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h
> index 65e6fd7..811d88f 100644
> --- a/include/hw/acpi/piix4.h
> +++ b/include/hw/acpi/piix4.h
> @@ -4,5 +4,6 @@
> #include "qemu/typedefs.h"
> 
> Object *piix4_pm_find(void);
> +void piix4_pm_class_fix_compat(void);
> 
> #endif
> diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
> index 4349033..0126424 100644
> --- a/include/hw/timer/i8254.h
> +++ b/include/hw/timer/i8254.h
> @@ -72,4 +72,6 @@ static inline ISADevice *kvm_pit_init(ISABus *bus, int base)
> void pit_set_gate(ISADevice *dev, int channel, int val);
> void pit_get_channel_info(ISADevice *dev, int channel, PITChannelInfo *info);
> 
> +void pit_common_class_fix_compat(void);
> +
> #endif /* !HW_I8254_H */
> -- 
> 1.7.9.5
> 
> 
> 

-- 
Alex Bligh

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

[Paolo Bonzini]

Il 25/07/2014 18:01, Alex Bligh ha scritto:
> ping: anyone got any views on this one?

Just wait for 2.1 to get out of the door. :)

Paolo

> On 22 Jul 2014, at 19:43, Alex Bligh <alex@alex.org.uk> wrote:
> 
>> Add a machine type pc-1.0-qemu-kvm for live migrate compatibility
>> with qemu-kvm version 1.0.
>>
>> Signed-off-by: Alex Bligh <alex@alex.org.uk>
>> ---
>> hw/acpi/piix4.c          |   49 ++++++++++++++++++++++++++++++++++++++++++++--
>> hw/i386/pc_piix.c        |   31 +++++++++++++++++++++++++++++
>> hw/timer/i8254_common.c  |   41 ++++++++++++++++++++++++++++++++++++++
>> include/hw/acpi/piix4.h  |    1 +
>> include/hw/timer/i8254.h |    2 ++
>> 5 files changed, 122 insertions(+), 2 deletions(-)
>>
>> This RFC 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.
>>
>> This requires 'hot patching' the VMStateDescription in a couple of
>> places, which in turn is less than obvious as there may be (indeed
>> are for i8259) derived classes. Whilst pretty nausea-inducing, this
>> approach has the benefit of being entirely self-contained.
>>
>> 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. Testing has been light to date (i.e.
>> can I migrate it inbound with -S without anything complaining).
>>
>> I have not (yet) brought forward the qxl rom size (and possibly
>> video ram) changes in Cole's patches as I'd prefer an assessment
>> of whether this is the right approach first.
>>
>> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
>> index b72b34e..708db79 100644
>> --- a/hw/acpi/piix4.c
>> +++ b/hw/acpi/piix4.c
>> @@ -267,8 +267,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 +308,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 = acpi_load_old,
>> +    .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;
>> @@ -619,6 +648,22 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
>>     adevc->ospm_status = piix4_ospm_status;
>> }
>>
>> +void piix4_pm_class_fix_compat(void)
>> +{
>> +    GSList *el, *devices = object_class_get_list(TYPE_DEVICE, false);
>> +    /*
>> +     * Change the vmstate field in this class and any derived classes
>> +     * if not overriden. As no other classes should be using this
>> +     * vmstate, we can simply iterate the class list
>> +     */
>> +    for (el = devices; el; el = el->next) {
>> +        DeviceClass *dc = el->data;
>> +        if (dc->vmsd == &vmstate_acpi) {
>> +            dc->vmsd = &vmstate_acpi_compat;
>> +        }
>> +    }
>> +}
>> +
>> static const TypeInfo piix4_pm_info = {
>>     .name          = TYPE_PIIX4_PM,
>>     .parent        = TYPE_PCI_DEVICE,
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 7081c08..e400ea6 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -49,6 +49,8 @@
>> #include "hw/acpi/acpi.h"
>> #include "cpu.h"
>> #include "qemu/error-report.h"
>> +#include "hw/acpi/piix4.h"
>> +#include "hw/timer/i8254.h"
>> #ifdef CONFIG_XEN
>> #  include <xen/hvm/hvm_info_table.h>
>> #endif
>> @@ -386,6 +388,15 @@ static void pc_init_pci_1_2(MachineState *machine)
>>     pc_init_pci(machine);
>> }
>>
>> +/* PC machine init function for qemu-kvm 1.0 */
>> +static void pc_init_pci_1_2_qemu_kvm(MachineState *machine)
>> +{
>> +    piix4_pm_class_fix_compat();
>> +    pit_common_class_fix_compat();
>> +    pc_compat_1_2(machine);
>> +    pc_init_pci(machine);
>> +}
>> +
>> /* PC init function for pc-0.10 to pc-0.13 */
>> static void pc_init_pci_no_kvmclock(MachineState *machine)
>> {
>> @@ -644,6 +655,25 @@ 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),\
>> +        }
>> +
>> +static QEMUMachine pc_machine_v1_0_qemu_kvm = {
>> +    PC_I440FX_1_2_MACHINE_OPTIONS,
>> +    .name = "pc-1.0-qemu-kvm",
>> +    .init = pc_init_pci_1_2_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 +916,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..511ea05 100644
>> --- a/hw/timer/i8254_common.c
>> +++ b/hw/timer/i8254_common.c
>> @@ -275,6 +275,29 @@ static const VMStateDescription vmstate_pit_common = {
>>     }
>> };
>>
>> +static const VMStateDescription vmstate_pit_common_compat = {
>> +    .name = "i8254",
>> +    .version_id = 3,
>> +    .minimum_version_id = 2,
>> +    .minimum_version_id_old = 1,
>> +    .load_state_old = pit_load_old,
>> +    .pre_save = pit_dispatch_pre_save,
>> +    .post_load = pit_dispatch_post_load,
>> +    .fields = (VMStateField[]) {
>> +        /* qemu-kvm version_id=2 had 'flags' here which is equivalent
>> +         * This fixes incoming migration from qemu-kvm 1.0, but breaks
>> +         * incoming migration from qemu < 1.1
>> +         */
>> +        /* VMSTATE_UINT32_V(channels[0].irq_disabled, PITCommonState, 3), */
>> +        VMSTATE_UINT32(channels[0].irq_disabled, PITCommonState),
>> +        VMSTATE_STRUCT_ARRAY(channels, PITCommonState, 3, 2,
>> +                             vmstate_pit_channel, PITChannelState),
>> +        VMSTATE_INT64(channels[0].next_transition_time,
>> +                      PITCommonState), /* formerly irq_timer */
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> static void pit_common_class_init(ObjectClass *klass, void *data)
>> {
>>     DeviceClass *dc = DEVICE_CLASS(klass);
>> @@ -289,6 +312,24 @@ static void pit_common_class_init(ObjectClass *klass, void *data)
>>     dc->cannot_instantiate_with_device_add_yet = true;
>> }
>>
>> +void pit_common_class_fix_compat(void)
>> +{
>> +    GSList *el, *devices = object_class_get_list(TYPE_DEVICE, false);
>> +    /*
>> +     * Change the vmstate field in this class and any derived classes
>> +     * if not overriden. As no other classes should be using this
>> +     * vmstate, we can simply iterate the class list
>> +     */
>> +    for (el = devices; el; el = el->next) {
>> +        DeviceClass *dc = el->data;
>> +        if (dc->vmsd == &vmstate_pit_common) {
>> +            dc->vmsd = &vmstate_pit_common_compat;
>> +        }
>> +    }
>> +
>> +    g_slist_free(devices);
>> +}
>> +
>> static const TypeInfo pit_common_type = {
>>     .name          = TYPE_PIT_COMMON,
>>     .parent        = TYPE_ISA_DEVICE,
>> diff --git a/include/hw/acpi/piix4.h b/include/hw/acpi/piix4.h
>> index 65e6fd7..811d88f 100644
>> --- a/include/hw/acpi/piix4.h
>> +++ b/include/hw/acpi/piix4.h
>> @@ -4,5 +4,6 @@
>> #include "qemu/typedefs.h"
>>
>> Object *piix4_pm_find(void);
>> +void piix4_pm_class_fix_compat(void);
>>
>> #endif
>> diff --git a/include/hw/timer/i8254.h b/include/hw/timer/i8254.h
>> index 4349033..0126424 100644
>> --- a/include/hw/timer/i8254.h
>> +++ b/include/hw/timer/i8254.h
>> @@ -72,4 +72,6 @@ static inline ISADevice *kvm_pit_init(ISABus *bus, int base)
>> void pit_set_gate(ISADevice *dev, int channel, int val);
>> void pit_get_channel_info(ISADevice *dev, int channel, PITChannelInfo *info);
>>
>> +void pit_common_class_fix_compat(void);
>> +
>> #endif /* !HW_I8254_H */
>> -- 
>> 1.7.9.5
>>
>>
>>
> 

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

[Alex Bligh]

On 22 Jul 2014, at 19:43, Alex Bligh <alex@alex.org.uk> wrote:

> I have not (yet) brought forward the qxl rom size (and possibly
> video ram) changes in Cole's patches as I'd prefer an assessment
> of whether this is the right approach first.

Whilst vga=cirrus, and vga=none I appear to be able to get to work,
I suspect the QXL problem is unfixable.

The symptom on import is:
  Unknown ramblock "0000:00:02.0/virtio-net-pci.rom", cannot accept migration

A little debugging and this shows that the expected RAM blocks on load are
(just looking at the ones starting 0000:00):
   0000:00:02.0/qxl.vram
   0000:00:02.0/qxl.rom
   0000:00:03.0/virtio-net-pci.rom
   0000:00:02.0/qxl.vrom

note the 03 in virtio-net-pci.rom, not an 02.

Looking at the migrated file (from qemu-kvm-1.0 with a pc-1.0 machine type)
with 'strings', I see:
   0000:00:02.0/virtio-net-pci.rom
   0000:00:00.0/I440FX
   0000:00:01.0/PIIX3
   0000:00:02.0/virtio-net
   0000:00:01.1/ide
   0000:00:01.2/uhci
   0000:00:01.3/piix4_pm
   0000:00:03.0/virtio-blk

A migrated file (from qemu 2.0 with a pc-1.0-qemu-kvm machine type) has
   0000:00:02.0/qxl.vram
   0000:00:03.0/virtio-net-pci.rom
   0000:00:02.0/qxl.rom
   0000:00:02.0/qxl.vrom
   0000:00:00.0/I440FX
   0000:00:01.0/PIIX3
   0000:00:02.0/qxl
   0000:00:03.0/virtio-net
   0000:00:01.1/ide
   0000:00:01.2/uhci
   0000:00:01.3/piix4_pm
   0000:00:04.0/virtio-blk

The symptom is the result of the assignment of device number 03 to
virtio-net, as (it would seem) qxl does not actually have a pci
device id in qemu-kvm-1.0 (or at least not one making it to migration);
I'm assuming the names are PCI BDF. Hence virtio-net is getting
device ID 02 not 03.

QXL does appear to migrate qemu-kvm-1.0 to qemu-kvm-1.0. I'm guessing
that once upon a time QXL was not a PCI device, but it is now.

For once I don't think Cole Robinson's patch is going to help here
as this seems to be handling QXL rom size mismatches between 8k
and 16k, and the QXL romsize (on both ubuntu 12.04 and 14.04 is
well over 32k) so that looks like not the issue.

I suspect QXL on qemu-kvm-1.0 is a bit niche anyway.

-- 
Alex Bligh

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

[Andreas Färber]

Hi Alex,

+ quintela, mst, libvirt

Am 22.07.2014 20:43, schrieb Alex Bligh:
> Add a machine type pc-1.0-qemu-kvm for live migrate compatibility
> with qemu-kvm version 1.0.
> 
> Signed-off-by: Alex Bligh <alex@alex.org.uk>
> ---
>  hw/acpi/piix4.c          |   49 ++++++++++++++++++++++++++++++++++++++++++++--
>  hw/i386/pc_piix.c        |   31 +++++++++++++++++++++++++++++
>  hw/timer/i8254_common.c  |   41 ++++++++++++++++++++++++++++++++++++++
>  include/hw/acpi/piix4.h  |    1 +
>  include/hw/timer/i8254.h |    2 ++
>  5 files changed, 122 insertions(+), 2 deletions(-)
> 
> This RFC 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.

This sounds like a really cool feature that SUSE would probably be
interested in extending back to 0.14 and 0.15, but I see a fundamental
flaw: libvirt on those old source systems does not know it should use a
different machine name on the destination side and would still use
pc-1.0, wouldn't it? After all, it needs to be able to migrate to other
old qemu-kvm machines, so it can't just be updated to use the new name.

Minor bikeshedding: I would ask to keep the package name in front of the
machine version, e.g. qemu-kvm-pc-1.0. Or just kvm-pc-1.0 since this is
a QEMU parameter anyway.

Haven't reviewed the code in detail yet.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

[Alex Bligh]

Andreas,

On 27 Jul 2014, at 15:10, Andreas Färber <afaerber@suse.de> wrote:

> Hi Alex,
> 
> + quintela, mst, libvirt

Thanks for your comments!

> This sounds like a really cool feature that SUSE would probably be
> interested in extending back to 0.14 and 0.15, but I see a fundamental
> flaw: libvirt on those old source systems does not know it should use a
> different machine name on the destination side and would still use
> pc-1.0, wouldn't it? After all, it needs to be able to migrate to other
> old qemu-kvm machines, so it can't just be updated to use the new name.

Right. I'm not using libvirt but face a similar problem. The destination
libvirt (or equivalent) now has two 'pc-1.0' machine types. One (allegedly)
use by qemu-git 1.0, and one used by qemu-kvm 1.0. The name 'pc-1.0' needs
to refer to exactly one of these (obviously), and it currently refers
to qemu-git 1.0, so I've left it that way.

In a distribution environment where qemu-1.0 was never used, one might
want to make my new machine type called 'pc-1.0' and the existing qemu.git
pc-1.0 called 'pc-1.0-qemu-git' or similar.

Why don't I just change it? Well, imagine someone running Ubuntu 13.04 (say)
which shipped with an early post merge qemu made a machine with machine type
pc-1.0. Despite the fact that Ubuntu 12.04 was shipped with qemu-kvm (and
hence -m pc-1.0 there meant qemu-kvm's pc-1.0), on 13.04, this would mean
qemu-git's pc-1.0, and changing the name would break the migration of an
Ubuntu 13.04 pc-1.0 machine, whilst fixing migration of a 12.04 pc-1.0
machine. Let no one say this isn't a mess.

So, I'm expect whatever generates the qemu command line to know (from the
other end) whether to use the different version.

Perhaps a slightly nicer fix, if somewhat rococo, would be:
* Use pc-1.0-qemu-kvm as one machine name
* Use pc-1.0-qemu-git as another machine name
* Make pc-1.0 an alias of either one or the other, configurable at the
  command line, and subject to a build-time default.

This would let distributions / users simply decide which kind of pc-1.0
migration they'd like by working by default, and which they'd like
broken; the broken one is a second class citizen which can only
handle migration by changing the qemu command line appropriately.

This would perhaps make things more transparent by default.

BTW I would agree this is a significant issue, having spent quite a lot
of today playing with stuff that (mis)parses "qemu -version"

Also BTW, I did wonder whether I could autodetect this from the stream.
However, as it has to be read serially, by the time you know it's wrong,
you are too late AFAICT. Useful suggestion in case of future accidental
breakage: put the qemu version in a section right up top.

> Minor bikeshedding: I would ask to keep the package name in front of the
> machine version, e.g. qemu-kvm-pc-1.0. Or just kvm-pc-1.0 since this is
> a QEMU parameter anyway.

Obviously not a matter of great import, but per the list
below, the standard appears to be 'pc-(arch)?-version'
and I'd taken this as a subversion.

# qemu-system-x86_64 -machine '?'
Supported machines are:
pc-0.13              Standard PC (i440FX + PIIX, 1996)
pc-i440fx-2.0        Standard PC (i440FX + PIIX, 1996)
pc-1.0               Standard PC (i440FX + PIIX, 1996)
pc-q35-1.7           Standard PC (Q35 + ICH9, 2009)
pc-1.1               Standard PC (i440FX + PIIX, 1996)
q35                  Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-2.0)
pc-q35-2.0           Standard PC (Q35 + ICH9, 2009)
pc-i440fx-1.4        Standard PC (i440FX + PIIX, 1996)
pc-i440fx-1.5        Standard PC (i440FX + PIIX, 1996)
pc-0.14              Standard PC (i440FX + PIIX, 1996)
pc-0.15              Standard PC (i440FX + PIIX, 1996)
xenfv                Xen Fully-virtualized PC
pc-q35-1.4           Standard PC (Q35 + ICH9, 2009)
isapc                ISA-only PC
pc-0.10              Standard PC (i440FX + PIIX, 1996)
pc                   Ubuntu 14.04 PC (i440FX + PIIX, 1996) (alias of pc-i440fx-trusty)
pc-i440fx-trusty     Ubuntu 14.04 PC (i440FX + PIIX, 1996) (default)
pc-1.2               Standard PC (i440FX + PIIX, 1996)
pc-0.11              Standard PC (i440FX + PIIX, 1996)
pc-i440fx-1.7        Standard PC (i440FX + PIIX, 1996)
pc-i440fx-1.6        Standard PC (i440FX + PIIX, 1996)
none                 empty machine
xenpv                Xen Para-virtualized PC
pc-q35-1.5           Standard PC (Q35 + ICH9, 2009)
pc-1.0-qemu-kvm      Standard PC (i440FX + PIIX, 1996)
pc-q35-1.6           Standard PC (Q35 + ICH9, 2009)
pc-0.12              Standard PC (i440FX + PIIX, 1996)
pc-1.3               Standard PC (i440FX + PIIX, 1996)

-- 
Alex Bligh

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

[Alex Bligh]

On 22 Jul 2014, at 19:43, Alex Bligh <alex@alex.org.uk> wrote:

> Testing has been light to date (i.e.
> can I migrate it inbound with -S without anything complaining).

I've given this quite a bit more testing today.

It works fine qemu-kvm 1.0 -> qemu-2.0+patch (cirrus vga)
It works fine qemu-2.0+patch -> qemu-2.0+patch (cirrus vga)
It doesn't (yet) work qemu-2.0+patch -> qemu-kvm 1.0 (cirrus vga).

The reason for this is (at least) that I need to emulate the
broken versioning of the mc146818rtc timer section, as writing
it correctly confuses qemu-kvm 1.0. Therefore please don't bother
testing migration back to 1.0 yet.

-- 
Alex Bligh

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

[Serge Hallyn]

Quoting Alex Bligh (alex@alex.org.uk):
> 
> On 22 Jul 2014, at 19:43, Alex Bligh <alex@alex.org.uk> wrote:
> 
> > Testing has been light to date (i.e.
> > can I migrate it inbound with -S without anything complaining).

thanks, Alex!

> 
> I've given this quite a bit more testing today.
> 
> It works fine qemu-kvm 1.0 -> qemu-2.0+patch (cirrus vga)
> It works fine qemu-2.0+patch -> qemu-2.0+patch (cirrus vga)
> It doesn't (yet) work qemu-2.0+patch -> qemu-kvm 1.0 (cirrus vga).
> 
> The reason for this is (at least) that I need to emulate the
> broken versioning of the mc146818rtc timer section, as writing
> it correctly confuses qemu-kvm 1.0. Therefore please don't bother
> testing migration back to 1.0 yet.

I don't think that is in any way a problem.  Is migrating to older
versions ever actually expected to work?  In either case I don't
think for this particular case it's a problem.

(The "how to handle this in libvirt" question is more interesting)

-serge

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

[Alex Bligh]

Serge,

> I don't think that is in any way a problem.  Is migrating to older
> versions ever actually expected to work?  In either case I don't
> think for this particular case it's a problem.

Good; no; and good - respectively.

> (The "how to handle this in libvirt" question is more interesting)

I've been giving this some thought. However rococo we make this,
I think the caller is often going to need to modify the command
line anyway, i.e. is going to need to be aware of the migration
problem.

For instance, taking Ubuntu as an example, 12.04 shipped with
qemu-kvm-1.0, and a pxe-virtio.rom of just under 64k, giving
a 64k ROM slot. 14.04 ships with qemu-2.0 and a pxe-virtio.rom
of over 80k, giving a 128k ROM slot. So however we fix the
machine types, when migrating in a 12.04 initiated VM, qemu
will need
 -global virtio-net-pci.romfile=/path/to/pxe-virtio.rom.12.04
on the command line (or, if you don't much care about PXE
working on a soft reboot, a blank file of the same size),
whereas when migrating in a 14.04 initiated VM, that must
not be on the command line.

Fixing this properly would entail requiring that the ROMs are
(effectively) distributed with qemu or at least that all
ROM sizes become part of the machine type standard. This
would have the advantage that loading a larger ROM than
the machine type specifies would fail unless the ROM
size was explicitly configured on the command line. But
this is a subject wider than this patch.

So the long and the short of it is that libvirt (sadly) like
anything else starting qemu machines needs to know a bit about
different versions of qemu, and be able to replace a machine
type option with a machine type option and more on the
command line.

My previous suggestion doesn't help much because qemu will
still need to be passed something on the command line.

I think the best way to go with this patch would be something
like:

* Add pc-1.0-qemu-kvm as a machine type (done)

* Rename pc-1.0 to pc-1.0-qemu-git

* Add an alias for pc-1.0 to either pc-1.0-qemu-git or
  pc-1.0-qemu-kvm, configurable at build time with
  a ./configure option. The distro can then set this
  appropriately. This would default to pc-1.0-qemu-git
  (i.e. the current behaviour).

On distros that only every used one of the above,
./configure will sort things out, +/- self-inflicted
injuries relating to ROM size changes. If life is
more complicated, libvirt (and other callers) will
need to be aware. pc-1.0-qemu-git and pc-1.0-qemu-kvm
can be used to unambiguously mean the relevant machine
type, which will fix things going forward for that
machine type.

WDYT?

-- 
Alex Bligh

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

[Serge E. Hallyn]

Quoting Alex Bligh (alex@alex.org.uk):
> Serge,
> 
> > I don't think that is in any way a problem.  Is migrating to older
> > versions ever actually expected to work?  In either case I don't
> > think for this particular case it's a problem.
> 
> Good; no; and good - respectively.
> 
> > (The "how to handle this in libvirt" question is more interesting)
> 
> I've been giving this some thought. However rococo we make this,
> I think the caller is often going to need to modify the command
> line anyway, i.e. is going to need to be aware of the migration
> problem.
> 
> For instance, taking Ubuntu as an example, 12.04 shipped with
> qemu-kvm-1.0, and a pxe-virtio.rom of just under 64k, giving
> a 64k ROM slot. 14.04 ships with qemu-2.0 and a pxe-virtio.rom
> of over 80k, giving a 128k ROM slot. So however we fix the
> machine types, when migrating in a 12.04 initiated VM, qemu
> will need
>  -global virtio-net-pci.romfile=/path/to/pxe-virtio.rom.12.04
> on the command line (or, if you don't much care about PXE
> working on a soft reboot, a blank file of the same size),
> whereas when migrating in a 14.04 initiated VM, that must
> not be on the command line.
> 
> Fixing this properly would entail requiring that the ROMs are
> (effectively) distributed with qemu or at least that all
> ROM sizes become part of the machine type standard. This
> would have the advantage that loading a larger ROM than
> the machine type specifies would fail unless the ROM
> size was explicitly configured on the command line. But
> this is a subject wider than this patch.
> 
> So the long and the short of it is that libvirt (sadly) like
> anything else starting qemu machines needs to know a bit about
> different versions of qemu, and be able to replace a machine
> type option with a machine type option and more on the
> command line.
> 
> My previous suggestion doesn't help much because qemu will
> still need to be passed something on the command line.
> 
> I think the best way to go with this patch would be something
> like:
> 
> * Add pc-1.0-qemu-kvm as a machine type (done)
> 
> * Rename pc-1.0 to pc-1.0-qemu-git
> 
> * Add an alias for pc-1.0 to either pc-1.0-qemu-git or
>   pc-1.0-qemu-kvm, configurable at build time with
>   a ./configure option. The distro can then set this
>   appropriately. This would default to pc-1.0-qemu-git
>   (i.e. the current behaviour).
> 
> On distros that only every used one of the above,
> ./configure will sort things out, +/- self-inflicted
> injuries relating to ROM size changes. If life is
> more complicated, libvirt (and other callers) will
> need to be aware. pc-1.0-qemu-git and pc-1.0-qemu-kvm
> can be used to unambiguously mean the relevant machine
> type, which will fix things going forward for that
> machine type.
> 
> WDYT?

That sounds good.

And from there I think the thing to do will be to introduce a transient
alternate package that has the pc-1.0 alias pointing ot pc-1.0-qemu-kvm and
depends on the legacy pxe rom.  And maybe users can then choose that package for
migration purposes if needed, without having to make any changes to libvirt at
all, while others are completely unaffected.

-serge

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

[Alex Bligh]

On 29 Jul 2014, at 14:03, Serge E. Hallyn <serge@hallyn.com> wrote:

> That sounds good.
> 
> And from there I think the thing to do will be to introduce a transient
> alternate package that has the pc-1.0 alias pointing ot pc-1.0-qemu-kvm and
> depends on the legacy pxe rom.  And maybe users can then choose that package for
> migration purposes if needed, without having to make any changes to libvirt at
> all, while others are completely unaffected.

OK, I'll do that the next time I roll the patch. I'd like to hear from
some others first and am mindful of Paolo's comment re waiting for the
release to ship.

-- 
Alex Bligh

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

[Paolo Bonzini]

Il 29/07/2014 15:03, Serge E. Hallyn ha scritto:
> 
> And from there I think the thing to do will be to introduce a transient
> alternate package that has the pc-1.0 alias pointing ot pc-1.0-qemu-kvm

This should be done in the main package, too.

> and depends on the legacy pxe rom.

If you can make the pxe-virtio.rom file 64k or less, then that would be
a good idea for 14.04 in general.  Newer machine types use
efi-virtio.rom, so you won't break "-M pc" migration.

Paolo

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

[Serge Hallyn]

Quoting Paolo Bonzini (pbonzini@redhat.com):
> Il 29/07/2014 15:03, Serge E. Hallyn ha scritto:
> > 
> > And from there I think the thing to do will be to introduce a transient
> > alternate package that has the pc-1.0 alias pointing ot pc-1.0-qemu-kvm
> 
> This should be done in the main package, too.

That seems like a problem, unless I"m misunderstanding something.  If we do
that in the main package, then anyone running a pc-1.0 system under the
qemu package won't be able to migrate.  Wouldn't it be better to have
pc-1.0 alias by default point to the pc-1.0-qemu machine type?

> > and depends on the legacy pxe rom.
> 
> If you can make the pxe-virtio.rom file 64k or less, then that would be
> a good idea for 14.04 in general.  Newer machine types use
> efi-virtio.rom, so you won't break "-M pc" migration.

Hm.  No idea offhand how I'd do that, but it sounds worth looking into.

thanks,
-serge

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

[Paolo Bonzini]

Il 29/07/2014 15:27, Serge Hallyn ha scritto:
> Quoting Paolo Bonzini (pbonzini@redhat.com):
>> Il 29/07/2014 15:03, Serge E. Hallyn ha scritto:
>>>
>>> And from there I think the thing to do will be to introduce a transient
>>> alternate package that has the pc-1.0 alias pointing ot pc-1.0-qemu-kvm
>>
>> This should be done in the main package, too.
> 
> That seems like a problem, unless I"m misunderstanding something.  If we do
> that in the main package, then anyone running a pc-1.0 system under the
> qemu package won't be able to migrate.  Wouldn't it be better to have
> pc-1.0 alias by default point to the pc-1.0-qemu machine type?

You'd break that for people who have already upgraded from 12.04 and
14.04 and are keeping the old machine type.  You'd fix it for people who
are upgrading now.

I think providing a smoother upgrade path is worthwhile, even if it
annoys someone else.

Unfortunately the only solution is a lot of testing *before* a release,
and in fact this is why 2.1 was delayed by a migration problem.  Once
the release is out, you'll have to make someone unhappy.

>>> and depends on the legacy pxe rom.
>>
>> If you can make the pxe-virtio.rom file 64k or less, then that would be
>> a good idea for 14.04 in general.  Newer machine types use
>> efi-virtio.rom, so you won't break "-M pc" migration.
> 
> Hm.  No idea offhand how I'd do that, but it sounds worth looking into.

I'm not sure either.  You could simply package the 12.04 ipxe ROMs into
14.04, and add a note about getting the sources for GPL friendliness.

Paolo

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

[Alex Bligh]

On 29 Jul 2014, at 14:35, Paolo Bonzini <pbonzini@redhat.com> wrote:

> I'm not sure either.  You could simply package the 12.04 ipxe ROMs into
> 14.04, and add a note about getting the sources for GPL friendliness.

This would be my preference (or in Ubuntu's case, add it to the ipxe-qemu
package) but I think it should only be used when the legacy machine type
is used or it will break inbound migrations from other 14.04 machines
started with 128k ROMs (AIUI).

-- 
Alex Bligh

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

[Paolo Bonzini]

Il 29/07/2014 15:39, Alex Bligh ha scritto:
>> > I'm not sure either.  You could simply package the 12.04 ipxe ROMs into
>> > 14.04, and add a note about getting the sources for GPL friendliness.
> This would be my preference (or in Ubuntu's case, add it to the ipxe-qemu
> package) but I think it should only be used when the legacy machine type
> is used or it will break inbound migrations from other 14.04 machines
> started with 128k ROMs (AIUI).

"-M pc" and its alias "-M pc-i440fx-2.0" doesn't use pxe-virtio.rom at
all (at least upstream).  Does Ubuntu 14.04 have efi-virtio.rom?

It would break "-M pc-1.0" started on older 14.04, but I think that's
acceptable.

Paolo

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

[Alex Bligh]

On 29 Jul 2014, at 14:42, Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 29/07/2014 15:39, Alex Bligh ha scritto:
>>>> I'm not sure either.  You could simply package the 12.04 ipxe ROMs into
>>>> 14.04, and add a note about getting the sources for GPL friendliness.
>> This would be my preference (or in Ubuntu's case, add it to the ipxe-qemu
>> package) but I think it should only be used when the legacy machine type
>> is used or it will break inbound migrations from other 14.04 machines
>> started with 128k ROMs (AIUI).
> 
> "-M pc" and its alias "-M pc-i440fx-2.0" doesn't use pxe-virtio.rom at
> all (at least upstream).

Really? How does it pxeboot on virtio then (it definitely does pxeboot
on virtio)?

>  Does Ubuntu 14.04 have efi-virtio.rom?

Yes. After 2 layers of symlinks you get to.

ubuntu@trustytest:~$ ls -la /usr/lib/ipxe/qemu/efi-virtio.rom
-rw-r--r-- 1 root root 220672 Jan  6  2014 /usr/lib/ipxe/qemu/efi-virtio.rom

> It would break "-M pc-1.0" started on older 14.04, but I think that's
> acceptable.

I was more worried about any previous versions of Ubuntu (newer than
12.04) which might also be using the larger rom size. But then I
haven't investigated at what stage the rom size grew.

-- 
Alex Bligh

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

[Paolo Bonzini]

Il 29/07/2014 15:56, Alex Bligh ha scritto:
> 
> On 29 Jul 2014, at 14:42, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>> Il 29/07/2014 15:39, Alex Bligh ha scritto:
>>>>> I'm not sure either.  You could simply package the 12.04 ipxe ROMs into
>>>>> 14.04, and add a note about getting the sources for GPL friendliness.
>>> This would be my preference (or in Ubuntu's case, add it to the ipxe-qemu
>>> package) but I think it should only be used when the legacy machine type
>>> is used or it will break inbound migrations from other 14.04 machines
>>> started with 128k ROMs (AIUI).
>>
>> "-M pc" and its alias "-M pc-i440fx-2.0" doesn't use pxe-virtio.rom at
>> all (at least upstream).
> 
> Really? How does it pxeboot on virtio then (it definitely does pxeboot
> on virtio)?

efi-virtio.rom contains both BIOS and UEFI ROMs.

>>  Does Ubuntu 14.04 have efi-virtio.rom?
> 
> Yes. After 2 layers of symlinks you get to.
> 
> ubuntu@trustytest:~$ ls -la /usr/lib/ipxe/qemu/efi-virtio.rom
> -rw-r--r-- 1 root root 220672 Jan  6  2014 /usr/lib/ipxe/qemu/efi-virtio.rom
> 
>> It would break "-M pc-1.0" started on older 14.04, but I think that's
>> acceptable.
> 
> I was more worried about any previous versions of Ubuntu (newer than
> 12.04) which might also be using the larger rom size. But then I
> haven't investigated at what stage the rom size grew.

You're right, but in Serge's shoes I wouldn't bother about anything
except LTS.

Paolo

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

[Alex Bligh]

On 29 Jul 2014, at 15:00, Paolo Bonzini <pbonzini@redhat.com> wrote:

> efi-virtio.rom contains both BIOS and UEFI ROMs.

You learn a new thing every day.

> You're right, but in Serge's shoes I wouldn't bother about anything
> except LTS.

Certainly this would be the most convenient path for me (with
my 'LTS user' hat on), i.e. make it 'just work' out the box.

-- 
Alex Bligh

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

[Serge Hallyn]

Quoting Paolo Bonzini (pbonzini@redhat.com):
> Il 29/07/2014 15:27, Serge Hallyn ha scritto:
> > Quoting Paolo Bonzini (pbonzini@redhat.com):
> >> Il 29/07/2014 15:03, Serge E. Hallyn ha scritto:
> >>>
> >>> And from there I think the thing to do will be to introduce a transient
> >>> alternate package that has the pc-1.0 alias pointing ot pc-1.0-qemu-kvm
> >>
> >> This should be done in the main package, too.
> > 
> > That seems like a problem, unless I"m misunderstanding something.  If we do
> > that in the main package, then anyone running a pc-1.0 system under the
> > qemu package won't be able to migrate.  Wouldn't it be better to have
> > pc-1.0 alias by default point to the pc-1.0-qemu machine type?
> 
> You'd break that for people who have already upgraded from 12.04 and
> 14.04 and are keeping the old machine type.  You'd fix it for people who
> are upgrading now.
> 
> I think providing a smoother upgrade path is worthwhile, even if it
> annoys someone else.
> 
> Unfortunately the only solution is a lot of testing *before* a release,
> and in fact this is why 2.1 was delayed by a migration problem.  Once
> the release is out, you'll have to make someone unhappy.

Right.

> >>> and depends on the legacy pxe rom.
> >>
> >> If you can make the pxe-virtio.rom file 64k or less, then that would be
> >> a good idea for 14.04 in general.  Newer machine types use
> >> efi-virtio.rom, so you won't break "-M pc" migration.
> > 
> > Hm.  No idea offhand how I'd do that, but it sounds worth looking into.
> 
> I'm not sure either.  You could simply package the 12.04 ipxe ROMs into
> 14.04, and add a note about getting the sources for GPL friendliness.

Right, as an alternate transient package, that was my original plan.

I need to look at ipxe at some point soon anyway, trimming it down
wouldn't hurt.

thanks,
-serge

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

[Alex Bligh]

On 29 Jul 2014, at 14:21, Paolo Bonzini <pbonzini@redhat.com> wrote:

> If you can make the pxe-virtio.rom file 64k or less, then that would be
> a good idea for 14.04 in general.  Newer machine types use
> efi-virtio.rom, so you won't break "-M pc" migration.

Without further, won't that break migration from 14.04 *with* the big
ROM?

-- 
Alex Bligh

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

[Serge Hallyn]

Quoting Alex Bligh (alex@alex.org.uk):
> 
> On 29 Jul 2014, at 14:21, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > If you can make the pxe-virtio.rom file 64k or less, then that would be
> > a good idea for 14.04 in general.  Newer machine types use
> > efi-virtio.rom, so you won't break "-M pc" migration.
> 
> Without further, won't that break migration from 14.04 *with* the big
> ROM?

Heh, good point.

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

[Michael S. Tsirkin]

On Tue, Jul 29, 2014 at 08:31:28AM +0100, Alex Bligh wrote:
> Serge,
> 
> > I don't think that is in any way a problem.  Is migrating to older
> > versions ever actually expected to work?  In either case I don't
> > think for this particular case it's a problem.
> 
> Good; no; and good - respectively.
> 
> > (The "how to handle this in libvirt" question is more interesting)
> 
> I've been giving this some thought. However rococo we make this,
> I think the caller is often going to need to modify the command
> line anyway, i.e. is going to need to be aware of the migration
> problem.
> 
> For instance, taking Ubuntu as an example, 12.04 shipped with
> qemu-kvm-1.0, and a pxe-virtio.rom of just under 64k, giving
> a 64k ROM slot. 14.04 ships with qemu-2.0 and a pxe-virtio.rom
> of over 80k, giving a 128k ROM slot. So however we fix the
> machine types, when migrating in a 12.04 initiated VM, qemu
> will need
>  -global virtio-net-pci.romfile=/path/to/pxe-virtio.rom.12.04
> on the command line (or, if you don't much care about PXE
> working on a soft reboot, a blank file of the same size),
> whereas when migrating in a 14.04 initiated VM, that must
> not be on the command line.
> 
> Fixing this properly would entail requiring that the ROMs are
> (effectively) distributed with qemu or at least that all
> ROM sizes become part of the machine type standard. This
> would have the advantage that loading a larger ROM than
> the machine type specifies would fail unless the ROM
> size was explicitly configured on the command line. But
> this is a subject wider than this patch.
> 
> So the long and the short of it is that libvirt (sadly) like
> anything else starting qemu machines needs to know a bit about
> different versions of qemu, and be able to replace a machine
> type option with a machine type option and more on the
> command line.
> 
> My previous suggestion doesn't help much because qemu will
> still need to be passed something on the command line.
> 
> I think the best way to go with this patch would be something
> like:
> 
> * Add pc-1.0-qemu-kvm as a machine type (done)
> 
> * Rename pc-1.0 to pc-1.0-qemu-git
> 
> * Add an alias for pc-1.0 to either pc-1.0-qemu-git or
>   pc-1.0-qemu-kvm, configurable at build time with
>   a ./configure option. The distro can then set this
>   appropriately. This would default to pc-1.0-qemu-git
>   (i.e. the current behaviour).
> 
> On distros that only every used one of the above,
> ./configure will sort things out, +/- self-inflicted
> injuries relating to ROM size changes. If life is
> more complicated, libvirt (and other callers) will
> need to be aware. pc-1.0-qemu-git and pc-1.0-qemu-kvm
> can be used to unambiguously mean the relevant machine
> type, which will fix things going forward for that
> machine type.
> 
> WDYT?


This just means we perpetuate the broken-ness.

I would rather we teach libvirt to do the right thing
unconditionally.


> -- 
> Alex Bligh
> 
> 
> 

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

[Serge Hallyn]

Quoting Michael S. Tsirkin (mst@redhat.com):
> On Tue, Jul 29, 2014 at 08:31:28AM +0100, Alex Bligh wrote:
> > Serge,
> > 
> > > I don't think that is in any way a problem.  Is migrating to older
> > > versions ever actually expected to work?  In either case I don't
> > > think for this particular case it's a problem.
> > 
> > Good; no; and good - respectively.
> > 
> > > (The "how to handle this in libvirt" question is more interesting)
> > 
> > I've been giving this some thought. However rococo we make this,
> > I think the caller is often going to need to modify the command
> > line anyway, i.e. is going to need to be aware of the migration
> > problem.
> > 
> > For instance, taking Ubuntu as an example, 12.04 shipped with
> > qemu-kvm-1.0, and a pxe-virtio.rom of just under 64k, giving
> > a 64k ROM slot. 14.04 ships with qemu-2.0 and a pxe-virtio.rom
> > of over 80k, giving a 128k ROM slot. So however we fix the
> > machine types, when migrating in a 12.04 initiated VM, qemu
> > will need
> >  -global virtio-net-pci.romfile=/path/to/pxe-virtio.rom.12.04
> > on the command line (or, if you don't much care about PXE
> > working on a soft reboot, a blank file of the same size),
> > whereas when migrating in a 14.04 initiated VM, that must
> > not be on the command line.
> > 
> > Fixing this properly would entail requiring that the ROMs are
> > (effectively) distributed with qemu or at least that all
> > ROM sizes become part of the machine type standard. This
> > would have the advantage that loading a larger ROM than
> > the machine type specifies would fail unless the ROM
> > size was explicitly configured on the command line. But
> > this is a subject wider than this patch.
> > 
> > So the long and the short of it is that libvirt (sadly) like
> > anything else starting qemu machines needs to know a bit about
> > different versions of qemu, and be able to replace a machine
> > type option with a machine type option and more on the
> > command line.
> > 
> > My previous suggestion doesn't help much because qemu will
> > still need to be passed something on the command line.
> > 
> > I think the best way to go with this patch would be something
> > like:
> > 
> > * Add pc-1.0-qemu-kvm as a machine type (done)
> > 
> > * Rename pc-1.0 to pc-1.0-qemu-git
> > 
> > * Add an alias for pc-1.0 to either pc-1.0-qemu-git or
> >   pc-1.0-qemu-kvm, configurable at build time with
> >   a ./configure option. The distro can then set this
> >   appropriately. This would default to pc-1.0-qemu-git
> >   (i.e. the current behaviour).
> > 
> > On distros that only every used one of the above,
> > ./configure will sort things out, +/- self-inflicted
> > injuries relating to ROM size changes. If life is
> > more complicated, libvirt (and other callers) will
> > need to be aware. pc-1.0-qemu-git and pc-1.0-qemu-kvm
> > can be used to unambiguously mean the relevant machine
> > type, which will fix things going forward for that
> > machine type.
> > 
> > WDYT?
> 
> 
> This just means we perpetuate the broken-ness.
> 
> I would rather we teach libvirt to do the right thing
> unconditionally.

Well, now, here's a thought - can we hot-patch qemu to
change the machine type while it is running before the
migrate?  Just s/pc-1.0/pc-x.0/ or something?

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

[Michael S. Tsirkin]

On Mon, Aug 04, 2014 at 03:08:31PM +0000, Serge Hallyn wrote:
> Quoting Michael S. Tsirkin (mst@redhat.com):
> > On Tue, Jul 29, 2014 at 08:31:28AM +0100, Alex Bligh wrote:
> > > Serge,
> > > 
> > > > I don't think that is in any way a problem.  Is migrating to older
> > > > versions ever actually expected to work?  In either case I don't
> > > > think for this particular case it's a problem.
> > > 
> > > Good; no; and good - respectively.
> > > 
> > > > (The "how to handle this in libvirt" question is more interesting)
> > > 
> > > I've been giving this some thought. However rococo we make this,
> > > I think the caller is often going to need to modify the command
> > > line anyway, i.e. is going to need to be aware of the migration
> > > problem.
> > > 
> > > For instance, taking Ubuntu as an example, 12.04 shipped with
> > > qemu-kvm-1.0, and a pxe-virtio.rom of just under 64k, giving
> > > a 64k ROM slot. 14.04 ships with qemu-2.0 and a pxe-virtio.rom
> > > of over 80k, giving a 128k ROM slot. So however we fix the
> > > machine types, when migrating in a 12.04 initiated VM, qemu
> > > will need
> > >  -global virtio-net-pci.romfile=/path/to/pxe-virtio.rom.12.04
> > > on the command line (or, if you don't much care about PXE
> > > working on a soft reboot, a blank file of the same size),
> > > whereas when migrating in a 14.04 initiated VM, that must
> > > not be on the command line.
> > > 
> > > Fixing this properly would entail requiring that the ROMs are
> > > (effectively) distributed with qemu or at least that all
> > > ROM sizes become part of the machine type standard. This
> > > would have the advantage that loading a larger ROM than
> > > the machine type specifies would fail unless the ROM
> > > size was explicitly configured on the command line. But
> > > this is a subject wider than this patch.
> > > 
> > > So the long and the short of it is that libvirt (sadly) like
> > > anything else starting qemu machines needs to know a bit about
> > > different versions of qemu, and be able to replace a machine
> > > type option with a machine type option and more on the
> > > command line.
> > > 
> > > My previous suggestion doesn't help much because qemu will
> > > still need to be passed something on the command line.
> > > 
> > > I think the best way to go with this patch would be something
> > > like:
> > > 
> > > * Add pc-1.0-qemu-kvm as a machine type (done)
> > > 
> > > * Rename pc-1.0 to pc-1.0-qemu-git
> > > 
> > > * Add an alias for pc-1.0 to either pc-1.0-qemu-git or
> > >   pc-1.0-qemu-kvm, configurable at build time with
> > >   a ./configure option. The distro can then set this
> > >   appropriately. This would default to pc-1.0-qemu-git
> > >   (i.e. the current behaviour).
> > > 
> > > On distros that only every used one of the above,
> > > ./configure will sort things out, +/- self-inflicted
> > > injuries relating to ROM size changes. If life is
> > > more complicated, libvirt (and other callers) will
> > > need to be aware. pc-1.0-qemu-git and pc-1.0-qemu-kvm
> > > can be used to unambiguously mean the relevant machine
> > > type, which will fix things going forward for that
> > > machine type.
> > > 
> > > WDYT?
> > 
> > 
> > This just means we perpetuate the broken-ness.
> > 
> > I would rather we teach libvirt to do the right thing
> > unconditionally.
> 
> Well, now, here's a thought - can we hot-patch qemu to
> change the machine type while it is running before the
> migrate?  Just s/pc-1.0/pc-x.0/ or something?

Frankly I don't see what will this accomplish.

If you really want it to be called pc-1.0, you
can make it a machine property instead.
E.g. qemu-kvm-compatibility.
Teach management to set it if remote is qemu-kvm:
	-machine pc-1.0,qemu-kvm-compatibility=on


-- 
MST

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

[Serge Hallyn]

Quoting Michael S. Tsirkin (mst@redhat.com):
> On Mon, Aug 04, 2014 at 03:08:31PM +0000, Serge Hallyn wrote:
> > Quoting Michael S. Tsirkin (mst@redhat.com):
> > > On Tue, Jul 29, 2014 at 08:31:28AM +0100, Alex Bligh wrote:
> > > > Serge,
> > > > 
> > > > > I don't think that is in any way a problem.  Is migrating to older
> > > > > versions ever actually expected to work?  In either case I don't
> > > > > think for this particular case it's a problem.
> > > > 
> > > > Good; no; and good - respectively.
> > > > 
> > > > > (The "how to handle this in libvirt" question is more interesting)
> > > > 
> > > > I've been giving this some thought. However rococo we make this,
> > > > I think the caller is often going to need to modify the command
> > > > line anyway, i.e. is going to need to be aware of the migration
> > > > problem.
> > > > 
> > > > For instance, taking Ubuntu as an example, 12.04 shipped with
> > > > qemu-kvm-1.0, and a pxe-virtio.rom of just under 64k, giving
> > > > a 64k ROM slot. 14.04 ships with qemu-2.0 and a pxe-virtio.rom
> > > > of over 80k, giving a 128k ROM slot. So however we fix the
> > > > machine types, when migrating in a 12.04 initiated VM, qemu
> > > > will need
> > > >  -global virtio-net-pci.romfile=/path/to/pxe-virtio.rom.12.04
> > > > on the command line (or, if you don't much care about PXE
> > > > working on a soft reboot, a blank file of the same size),
> > > > whereas when migrating in a 14.04 initiated VM, that must
> > > > not be on the command line.
> > > > 
> > > > Fixing this properly would entail requiring that the ROMs are
> > > > (effectively) distributed with qemu or at least that all
> > > > ROM sizes become part of the machine type standard. This
> > > > would have the advantage that loading a larger ROM than
> > > > the machine type specifies would fail unless the ROM
> > > > size was explicitly configured on the command line. But
> > > > this is a subject wider than this patch.
> > > > 
> > > > So the long and the short of it is that libvirt (sadly) like
> > > > anything else starting qemu machines needs to know a bit about
> > > > different versions of qemu, and be able to replace a machine
> > > > type option with a machine type option and more on the
> > > > command line.
> > > > 
> > > > My previous suggestion doesn't help much because qemu will
> > > > still need to be passed something on the command line.
> > > > 
> > > > I think the best way to go with this patch would be something
> > > > like:
> > > > 
> > > > * Add pc-1.0-qemu-kvm as a machine type (done)
> > > > 
> > > > * Rename pc-1.0 to pc-1.0-qemu-git
> > > > 
> > > > * Add an alias for pc-1.0 to either pc-1.0-qemu-git or
> > > >   pc-1.0-qemu-kvm, configurable at build time with
> > > >   a ./configure option. The distro can then set this
> > > >   appropriately. This would default to pc-1.0-qemu-git
> > > >   (i.e. the current behaviour).
> > > > 
> > > > On distros that only every used one of the above,
> > > > ./configure will sort things out, +/- self-inflicted
> > > > injuries relating to ROM size changes. If life is
> > > > more complicated, libvirt (and other callers) will
> > > > need to be aware. pc-1.0-qemu-git and pc-1.0-qemu-kvm
> > > > can be used to unambiguously mean the relevant machine
> > > > type, which will fix things going forward for that
> > > > machine type.
> > > > 
> > > > WDYT?
> > > 
> > > 
> > > This just means we perpetuate the broken-ness.
> > > 
> > > I would rather we teach libvirt to do the right thing
> > > unconditionally.
> > 
> > Well, now, here's a thought - can we hot-patch qemu to
> > change the machine type while it is running before the
> > migrate?  Just s/pc-1.0/pc-x.0/ or something?
> 
> Frankly I don't see what will this accomplish.
> 
> If you really want it to be called pc-1.0, you
> can make it a machine property instead.
> E.g. qemu-kvm-compatibility.
> Teach management to set it if remote is qemu-kvm:
> 	-machine pc-1.0,qemu-kvm-compatibility=on

That sounds nice - Alex, what do you think?

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

[Alex Bligh]

On 4 Aug 2014, at 16:38, Serge Hallyn <serge.hallyn@ubuntu.com> wrote:

>> 
>> If you really want it to be called pc-1.0, you
>> can make it a machine property instead.
>> E.g. qemu-kvm-compatibility.
>> Teach management to set it if remote is qemu-kvm:
>> 	-machine pc-1.0,qemu-kvm-compatibility=on
> 
> That sounds nice - Alex, what do you think?

Not having used the machine property stuff before,
or played with libvirt much, I'm not sure how this
helps libvirt.

I thought the issue here was that migrating from
1.0-qemu-kvm to 2.x OR 1.0-qemu-git to 2.x, libvirt
is going to to supply the same command line. As
libvirt doesn't know what the sender is (and
it's not possible to detect this automatically -
at least not without a far more intrusive patch),
one has to make a choice at build time as to what
'pc-1.0' represents. This is what patch #2 does.
I fully agree it is not pretty.

So I am not sure why
	-machine pc-1.0,qemu-kvm-compatibility=on
is any easier for libvirt than
	-machine pc-1.0-qemu-kvm

IE what does using a machine property rather than
a machine type buy us?

-- 
Alex Bligh

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

[Michael S. Tsirkin]

On Mon, Aug 04, 2014 at 04:47:11PM +0100, Alex Bligh wrote:
> 
> On 4 Aug 2014, at 16:38, Serge Hallyn <serge.hallyn@ubuntu.com> wrote:
> 
> >> 
> >> If you really want it to be called pc-1.0, you
> >> can make it a machine property instead.
> >> E.g. qemu-kvm-compatibility.
> >> Teach management to set it if remote is qemu-kvm:
> >> 	-machine pc-1.0,qemu-kvm-compatibility=on
> > 
> > That sounds nice - Alex, what do you think?
> 
> Not having used the machine property stuff before,
> or played with libvirt much, I'm not sure how this
> helps libvirt.
> 
> I thought the issue here was that migrating from
> 1.0-qemu-kvm to 2.x OR 1.0-qemu-git to 2.x, libvirt
> is going to to supply the same command line.
> As
> libvirt doesn't know what the sender is (and
> it's not possible to detect this automatically -
> at least not without a far more intrusive patch),

Yes, this is up to higher level user.
At libvirt xml level, you would just specify
something like "legacy qemu-kvm compatibility" in the xml.

> one has to make a choice at build time as to what
> 'pc-1.0' represents.

There's no choice really. Downstreams must make sure
their machine types are distinct from upstream ones.
qemu-kvm as a downstream violated this rule but
I don't think this means upstream should violate it.

> This is what patch #2 does.
> I fully agree it is not pretty.

The problem is not prettyness.
The problem is, it creates a situation where two instances
of qemu have different ideas about what a specific
machine type is.


> So I am not sure why
> 	-machine pc-1.0,qemu-kvm-compatibility=on
> is any easier for libvirt than
> 	-machine pc-1.0-qemu-kvm
> 
> IE what does using a machine property rather than
> a machine type buy us?

Seems to be easier to understand that it maps to pc-1.0
on the other side.

> -- 
> Alex Bligh
> 
> 
>