Re: [PATCH 0/6] Save state error handling (kill off no_migrate)

[Alex Williamson <alex.williamson@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4783 bytes --]

Avi, Marcelo,

Assuming this gets merged to qemu.git, you'll need the attached trivial
updates for the qemu-kvm.git merge.  Thanks,

Alex

On Wed, 2010-10-06 at 14:58 -0600, Alex Williamson wrote:
> Our code paths for saving or migrating a VM are full of functions that
> return void, leaving no opportunity for a device to cancel a migration,
> either from error or incompatibility.  The ivshmem driver attempted to
> solve this with a no_migrate flag on the save state entry.  I think the
> more generic and flexible way to solve this is to allow driver save
> functions to fail.  This series implements that and converts ivshmem
> to uses a set_params function to NAK migration much earlier in the
> processes.  This touches a lot of files, but bulk of those changes are
> simply s/void/int/ and tacking a "return 0" to the end of functions.
> Thanks,
> 
> Alex
> 
> ---
> 
> Alex Williamson (6):
>       savevm: Remove register_device_unmigratable()
>       savevm: Allow set_params and save_live_state to error
>       virtio: Allow virtio_save() errors
>       pci: Allow pci_device_save() to return error
>       savevm: Allow vmsd->pre_save to return error
>       savevm: Allow SaveStateHandler() to return error
> 
> 
>  block-migration.c           |    4 +-
>  hw/adb.c                    |    8 +++-
>  hw/ads7846.c                |    4 +-
>  hw/arm_gic.c                |    4 +-
>  hw/arm_timer.c              |    6 ++-
>  hw/armv7m_nvic.c            |    4 +-
>  hw/cuda.c                   |    4 +-
>  hw/fdc.c                    |    3 +
>  hw/g364fb.c                 |    4 +-
>  hw/grackle_pci.c            |    4 +-
>  hw/gt64xxx.c                |    4 +-
>  hw/heathrow_pic.c           |    4 +-
>  hw/hpet.c                   |    3 +
>  hw/hw.h                     |   12 ++----
>  hw/i2c.c                    |    3 +
>  hw/ide/core.c               |    4 +-
>  hw/ivshmem.c                |   30 +++++++++++----
>  hw/lsi53c895a.c             |    4 +-
>  hw/m48t59.c                 |    4 +-
>  hw/mac_dbdma.c              |    4 +-
>  hw/mac_nvram.c              |    4 +-
>  hw/max111x.c                |    4 +-
>  hw/mipsnet.c                |    4 +-
>  hw/mst_fpga.c               |    3 +
>  hw/nand.c                   |    3 +
>  hw/openpic.c                |    4 +-
>  hw/pci.c                    |    9 +++-
>  hw/pci.h                    |    2 -
>  hw/piix4.c                  |    4 +-
>  hw/pl011.c                  |    4 +-
>  hw/pl022.c                  |    4 +-
>  hw/pl061.c                  |    4 +-
>  hw/ppc4xx_pci.c             |   11 ++++-
>  hw/ppce500_pci.c            |   11 ++++-
>  hw/pxa2xx.c                 |   28 ++++++++++----
>  hw/pxa2xx_dma.c             |    4 +-
>  hw/pxa2xx_gpio.c            |    4 +-
>  hw/pxa2xx_keypad.c          |    3 +
>  hw/pxa2xx_lcd.c             |    4 +-
>  hw/pxa2xx_mmci.c            |    4 +-
>  hw/pxa2xx_pic.c             |    4 +-
>  hw/pxa2xx_timer.c           |    4 +-
>  hw/rc4030.c                 |    4 +-
>  hw/rtl8139.c                |    4 +-
>  hw/serial.c                 |    3 +
>  hw/spitz.c                  |   14 +++++--
>  hw/ssd0323.c                |    4 +-
>  hw/ssi-sd.c                 |    4 +-
>  hw/stellaris.c              |   20 +++++++---
>  hw/stellaris_enet.c         |    4 +-
>  hw/stellaris_input.c        |    4 +-
>  hw/syborg_fb.c              |    4 +-
>  hw/syborg_interrupt.c       |    3 +
>  hw/syborg_keyboard.c        |    3 +
>  hw/syborg_pointer.c         |    3 +
>  hw/syborg_rtc.c             |    4 +-
>  hw/syborg_serial.c          |    4 +-
>  hw/syborg_timer.c           |    4 +-
>  hw/tsc2005.c                |    4 +-
>  hw/tsc210x.c                |    4 +-
>  hw/twl92230.c               |    3 +
>  hw/unin_pci.c               |    4 +-
>  hw/usb-uhci.c               |    3 +
>  hw/virtio-balloon.c         |    9 +++-
>  hw/virtio-blk.c             |   10 ++++-
>  hw/virtio-net.c             |   11 ++++-
>  hw/virtio-pci.c             |   10 ++++-
>  hw/virtio-serial-bus.c      |   10 ++++-
>  hw/virtio.c                 |   14 +++++--
>  hw/virtio.h                 |    4 +-
>  hw/wm8750.c                 |    3 +
>  hw/zaurus.c                 |    4 +-
>  qemu-common.h               |    2 -
>  savevm.c                    |   88 +++++++++++++++++++------------------------
>  slirp/slirp.c               |    6 ++-
>  target-arm/machine.c        |    3 +
>  target-cris/machine.c       |    3 +
>  target-i386/machine.c       |    7 ++-
>  target-microblaze/machine.c |    3 +
>  target-mips/machine.c       |    3 +
>  target-ppc/machine.c        |    3 +
>  target-s390x/machine.c      |    3 +
>  target-sparc/machine.c      |    3 +
>  83 files changed, 365 insertions(+), 181 deletions(-)



[-- Attachment #2: kvm-1.patch --]
[-- Type: text/x-patch, Size: 868 bytes --]

kvm: update for: savevm: Allow SaveStateHandler() to return error

From: Alex Williamson <alex.williamson@redhat.com>

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 kvm-tpr-opt.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)


diff --git a/kvm-tpr-opt.c b/kvm-tpr-opt.c
index 46890e2..632b6d6 100644
--- a/kvm-tpr-opt.c
+++ b/kvm-tpr-opt.c
@@ -296,7 +296,7 @@ void kvm_tpr_access_report(CPUState *env, uint64_t rip, int is_write)
     patch_instruction(env, rip);
 }
 
-static void tpr_save(QEMUFile *f, void *s)
+static int tpr_save(QEMUFile *f, void *s)
 {
     int i;
 
@@ -307,6 +307,8 @@ static void tpr_save(QEMUFile *f, void *s)
     qemu_put_be32s(f, &bios_addr);
     qemu_put_be32s(f, &vapic_phys);
     qemu_put_be32s(f, &vbios_desc_phys);
+
+    return 0;
 }
 
 static int tpr_load(QEMUFile *f, void *s, int version_id)

[-- Attachment #3: kvm-2.patch --]
[-- Type: text/x-patch, Size: 2542 bytes --]

kvm: update for: savevm: Allow vmsd->pre_save to return error

From: Alex Williamson <alex.williamson@redhat.com>

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

 hw/i8254-kvm.c |    3 ++-
 hw/i8259.c     |    3 ++-
 hw/ioapic.c    |    3 ++-
 qemu-kvm-x86.c |    4 +++-
 4 files changed, 9 insertions(+), 4 deletions(-)


diff --git a/hw/i8254-kvm.c b/hw/i8254-kvm.c
index 6125213..9c62b87 100644
--- a/hw/i8254-kvm.c
+++ b/hw/i8254-kvm.c
@@ -32,7 +32,7 @@ extern VMStateDescription vmstate_pit;
 
 static PITState pit_state;
 
-static void kvm_pit_pre_save(void *opaque)
+static int kvm_pit_pre_save(void *opaque)
 {
     PITState *s = (void *)opaque;
     struct kvm_pit_state2 pit2;
@@ -64,6 +64,7 @@ static void kvm_pit_pre_save(void *opaque)
 	sc->gate = c->gate;
 	sc->count_load_time = c->count_load_time;
     }
+    return 0;
 }
 
 static int kvm_pit_post_load(void *opaque, int version_id)
diff --git a/hw/i8259.c b/hw/i8259.c
index 986dcf5..fc06c44 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -470,13 +470,14 @@ static uint32_t elcr_ioport_read(void *opaque, uint32_t addr1)
 static void kvm_kernel_pic_save_to_user(PicState *s);
 static int kvm_kernel_pic_load_from_user(PicState *s);
 
-static void pic_pre_save(void *opaque)
+static int pic_pre_save(void *opaque)
 {
     PicState *s = opaque;
 
     if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_kernel_pic_save_to_user(s);
     }
+    return 0;
 }
 
 static int pic_post_load(void *opaque, int version_id)
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 276c72e..4f71027 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -249,13 +249,14 @@ static void kvm_kernel_ioapic_load_from_user(IOAPICState *s)
 #endif
 }
 
-static void ioapic_pre_save(void *opaque)
+static int ioapic_pre_save(void *opaque)
 {
     IOAPICState *s = (void *)opaque;
  
     if (kvm_enabled() && kvm_irqchip_in_kernel()) {
         kvm_kernel_ioapic_save_to_user(s);
     }
+    return 0;
 }
 
 static int ioapic_pre_load(void *opaque)
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index fd974b3..1dcea38 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -561,11 +561,13 @@ static int kvm_enable_tpr_access_reporting(CPUState *env)
 #ifdef KVM_CAP_ADJUST_CLOCK
 static struct kvm_clock_data kvmclock_data;
 
-static void kvmclock_pre_save(void *opaque)
+static int kvmclock_pre_save(void *opaque)
 {
     struct kvm_clock_data *cl = opaque;
 
     kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, cl);
+
+    return 0;
 }
 
 static int kvmclock_post_load(void *opaque, int version_id)