[Qemu-devel] acpi_piix4 migration issue

[Marcelo Tosatti <mtosatti@redhat.com>]

qemu-kvm 1.2 -> qemu-1.3 migration fails with

Unknown savevm section type 48
load of migration failed

Due to a fix in acpi_piix4 in qemu-kvm (attached at the end of the
message). 

The problem is that qemu-kvm correctly uses 2 bytes for sts and 
2 bytes for en fields (which is their allocated size), while qemu 
uses 4*2 bytes for each.

The fix present in qemu-kvm is correct, but, having it in qemu 1.3 would break
qemu 1.2 -> qemu 1.3 migration (while allowing qemu-kvm 1.2 -> qemu 1.3
migration).

Any opinions on what to do?


commit b2e4a396f71ace63df6fa7e853f8835c4db6c920
Author: Isaku Yamahata <yamahata@valinux.co.jp>
Date:   Sun Apr 17 22:50:45 2011 +0900

    acpi, acpi_piix: factor out GPE logic
    
    On Sun, Apr 17, 2011 at 04:17:51PM +0300, Avi Kivity wrote:
    > On 03/16/2011 11:29 AM, Isaku Yamahata wrote:
    >> factor out ACPI GPE logic. Later it will be used by ICH9 ACPI.
    >>
    >
    > I think this patch is causing qemu-kvm failures on migration:
    > (gdb) bt
    > #0  0x000000000049aff4 in qemu_put_be16s (f=0x1a74490, pv=0x2c02580,
    > size=2) at hw/hw.h:108
    > #1  put_uint16 (f=0x1a74490, pv=0x2c02580, size=2) at savevm.c:855
    > #2  0x000000000049c3e4 in vmstate_save_state (f=0x1a74490,
    > vmsd=0x6f0b00, opaque=0x1842ef0) at savevm.c:1436
    > #3  0x000000000049c3b6 in vmstate_save_state (f=0x1a74490,
    > vmsd=0x6f0aa0, opaque=0x1842b90) at savevm.c:1434
    > #4  0x000000000049c6f1 in vmstate_save (mon=<value optimized out>,
    > f=0x1a74490) at savevm.c:1459
    > #5  qemu_savevm_state_complete (mon=<value optimized out>, f=0x1a74490)
    > at savevm.c:1600
    > #6  0x000000000049455a in migrate_fd_put_ready (opaque=0x1847890) at
    > migration.c:383
    > #7  0x00000000004ce2eb in qemu_run_timers (clock=<value optimized out>)
    > at qemu-timer.c:505
    > #8  0x00000000004ce806 in qemu_run_all_timers () at qemu-timer.c:619
    > #9  0x0000000000419463 in main_loop_wait (nonblocking=<value optimized
    > out>) at /build/home/tlv/akivity/qemu-kvm/vl.c:1339
    > #10 0x0000000000433927 in kvm_main_loop () at
    > /build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1590
    > #11 0x000000000041a3a6 in main_loop (argc=<value optimized out>,
    > argv=<value optimized out>, envp=<value optimized out>)
    >     at /build/home/tlv/akivity/qemu-kvm/vl.c:1369
    > #12 main (argc=<value optimized out>, argv=<value optimized out>,
    > envp=<value optimized out>) at /build/home/tlv/akivity/qemu-kvm/vl.c:3257
    >
    > The vmstate being migrated is "gpe".
    >
    >
    >
    >>
    >> +#define VMSTATE_GPE_ARRAY(_field, _state)                            \
    >> + {                                                                   \
    >> +     .name       = (stringify(_field)),                              \
    >> +     .version_id = 0,                                                \
    >> +     .num        = GPE_LEN,                                          \
    >> +     .info       =&vmstate_info_uint16,                             \
    >> +     .size       = sizeof(uint16_t),                                 \
    >> +     .flags      = VMS_ARRAY | VMS_POINTER,                          \
    >> +     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
    >> + }
    >> +
    >>   static const VMStateDescription vmstate_gpe = {
    >>       .name = "gpe",
    >>       .version_id = 1,
    >>       .minimum_version_id = 1,
    >>       .minimum_version_id_old = 1,
    >>       .fields      = (VMStateField []) {
    >> -        VMSTATE_UINT16(sts, struct gpe_regs),
    >> -        VMSTATE_UINT16(en, struct gpe_regs),
    >> +        VMSTATE_GPE_ARRAY(sts, ACPIGPE),
    >> +        VMSTATE_GPE_ARRAY(en, ACPIGPE),
    >>           VMSTATE_END_OF_LIST()
    >>       }
    >>   };
    >
    > I'm no vmstate expert, but this does look odd.  Why both VMS_ARRAY and
    > VMS_POINTER? aren't we trying to save/restore a simple 16-bit value?  Or
    > at least we did before this patch.
    
    That's right. the difference is, the new member type became uint8_t*.
    Does the following help?
    
    Signed-off-by: Avi Kivity <avi@redhat.com>

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index d65a7e9..9dc6f43 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -221,10 +221,9 @@ static int vmstate_acpi_post_load(void *opaque, int version_id)
  {                                                                   \
      .name       = (stringify(_field)),                              \
      .version_id = 0,                                                \
-     .num        = GPE_LEN,                                          \
      .info       = &vmstate_info_uint16,                             \
      .size       = sizeof(uint16_t),                                 \
-     .flags      = VMS_ARRAY | VMS_POINTER,                          \
+     .flags      = VMS_SINGLE | VMS_POINTER,                         \
      .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
  }