diff mbox

[v3,1/2] Add machine type pc-1.0-qemu-kvm for live migrate compatibility with qemu-kvm

Message ID 1411310339-27733-2-git-send-email-alex@alex.org.uk
State New
Headers show

Commit Message

Alex Bligh Sept. 21, 2014, 2:38 p.m. UTC
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         |   47 +++++++++++++++++++++++++++++++++++++++++++++--
 hw/i386/pc_piix.c       |   30 ++++++++++++++++++++++++++++++
 hw/timer/i8254_common.c |   10 +++++++++-
 3 files changed, 84 insertions(+), 3 deletions(-)

Comments

Michael S. Tsirkin Sept. 22, 2014, 11:50 a.m. UTC | #1
On Sun, Sep 21, 2014 at 03:38:58PM +0100, Alex Bligh 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         |   47 +++++++++++++++++++++++++++++++++++++++++++++--
>  hw/i386/pc_piix.c       |   30 ++++++++++++++++++++++++++++++
>  hw/timer/i8254_common.c |   10 +++++++++-
>  3 files changed, 84 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index b72b34e..e016005 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -37,6 +37,8 @@
>  #include "hw/acpi/memory_hotplug.h"
>  #include "hw/acpi/acpi_dev_interface.h"
>  
> +extern int qemu_kvm_1_0_compat;
> +
>  //#define DEBUG
>  
>  #ifdef DEBUG

Please dont' put extern declarations in C files.
You lose a useful type-safety compile-time check.

> @@ -200,12 +202,24 @@ static const VMStateDescription vmstate_pci_status = {
>      }
>  };
>  
> +static const VMStateDescription vmstate_acpi_compat;
> +

Don't declare things like this, re-order them in a separate patch so forward
declarations are not necessary.

>  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_kvm_1_0_compat)) {
> +        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/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 7081c08..48a4942 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -70,6 +70,8 @@ static bool smbios_legacy_mode;
>  static bool gigabyte_align = true;
>  static bool has_reserved_memory = true;
>  
> +int qemu_kvm_1_0_compat;
> +
>  /* PC hardware initialisation */
>  static void pc_init1(MachineState *machine,
>                       int pci_enabled,
> @@ -386,6 +388,14 @@ 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)
> +{
> +    qemu_kvm_1_0_compat = 1;
> +    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 +654,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",

I think pc-qemu-kvm-1.0 is slightly better - more consistent with
other machine names.

> +    .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 +915,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..b4deead 100644
> --- a/hw/timer/i8254_common.c
> +++ b/hw/timer/i8254_common.c
> @@ -29,6 +29,8 @@
>  #include "hw/timer/i8254.h"
>  #include "hw/timer/i8254_internal.h"
>  
> +extern int qemu_kvm_1_0_compat;
> +
>  /* val must be 0 or 1 */
>  void pit_set_gate(ISADevice *dev, int channel, int val)
>  {

Can you add a device property like we do for
other compat behaviours?
That's better than a global variable.
Same for other devices.


> @@ -257,6 +259,11 @@ 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_kvm_1_0_compat);
> +}
> +
>  static const VMStateDescription vmstate_pit_common = {
>      .name = "i8254",
>      .version_id = 3,
> @@ -266,7 +273,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,
> -- 
> 1.7.9.5
Alex Bligh Sept. 22, 2014, 12:28 p.m. UTC | #2
Michael,

(Reordering)

On 22 Sep 2014, at 12:50, Michael S. Tsirkin <mst@redhat.com> wrote:

> 
>> @@ -257,6 +259,11 @@ 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_kvm_1_0_compat);
>> +}
>> +
> Can you add a device property like we do for
> other compat behaviours?
> That's better than a global variable.
> Same for other devices.

I can see 'opaque' points to PIIX4PMState or PITCommonState. Do you mean
to put them in there? Could you point to an example to copy.
Michael S. Tsirkin Sept. 22, 2014, 12:38 p.m. UTC | #3
On Mon, Sep 22, 2014 at 01:28:30PM +0100, Alex Bligh wrote:
> Michael,
> 
> (Reordering)
> 
> On 22 Sep 2014, at 12:50, Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> > 
> >> @@ -257,6 +259,11 @@ 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_kvm_1_0_compat);
> >> +}
> >> +
> > Can you add a device property like we do for
> > other compat behaviours?
> > That's better than a global variable.
> > Same for other devices.
> 
> I can see 'opaque' points to PIIX4PMState or PITCommonState. Do you mean
> to put them in there? Could you point to an example to copy.

Look for PC_COMPAT_ macros, we have a ton of flags like this.


> -- 
> Alex Bligh
> 
> 
>
diff mbox

Patch

diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index b72b34e..e016005 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -37,6 +37,8 @@ 
 #include "hw/acpi/memory_hotplug.h"
 #include "hw/acpi/acpi_dev_interface.h"
 
+extern int qemu_kvm_1_0_compat;
+
 //#define DEBUG
 
 #ifdef DEBUG
@@ -200,12 +202,24 @@  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_kvm_1_0_compat)) {
+        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/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7081c08..48a4942 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -70,6 +70,8 @@  static bool smbios_legacy_mode;
 static bool gigabyte_align = true;
 static bool has_reserved_memory = true;
 
+int qemu_kvm_1_0_compat;
+
 /* PC hardware initialisation */
 static void pc_init1(MachineState *machine,
                      int pci_enabled,
@@ -386,6 +388,14 @@  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)
+{
+    qemu_kvm_1_0_compat = 1;
+    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 +654,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 +915,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..b4deead 100644
--- a/hw/timer/i8254_common.c
+++ b/hw/timer/i8254_common.c
@@ -29,6 +29,8 @@ 
 #include "hw/timer/i8254.h"
 #include "hw/timer/i8254_internal.h"
 
+extern int qemu_kvm_1_0_compat;
+
 /* val must be 0 or 1 */
 void pit_set_gate(ISADevice *dev, int channel, int val)
 {
@@ -257,6 +259,11 @@  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_kvm_1_0_compat);
+}
+
 static const VMStateDescription vmstate_pit_common = {
     .name = "i8254",
     .version_id = 3,
@@ -266,7 +273,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,