* [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate)
@ 2010-10-06 20:58 Alex Williamson
2010-10-06 20:59 ` [Qemu-devel] [PATCH 1/6] savevm: Allow SaveStateHandler() to return error Alex Williamson
` (8 more replies)
0 siblings, 9 replies; 26+ messages in thread
From: Alex Williamson @ 2010-10-06 20:58 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.williamson, cam, kvm, quintela
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(-)
^ permalink raw reply [flat|nested] 26+ messages in thread* [Qemu-devel] [PATCH 1/6] savevm: Allow SaveStateHandler() to return error 2010-10-06 20:58 [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson @ 2010-10-06 20:59 ` Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 2/6] savevm: Allow vmsd->pre_save " Alex Williamson ` (7 subsequent siblings) 8 siblings, 0 replies; 26+ messages in thread From: Alex Williamson @ 2010-10-06 20:59 UTC (permalink / raw) To: qemu-devel; +Cc: alex.williamson, cam, kvm, quintela Some devices may not always able to save their state, allow the save handler to return an error. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- 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/g364fb.c | 4 +++- hw/grackle_pci.c | 4 +++- hw/gt64xxx.c | 3 ++- hw/heathrow_pic.c | 4 +++- hw/hw.h | 2 +- hw/ivshmem.c | 3 ++- 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/piix4.c | 3 ++- hw/pl011.c | 4 +++- hw/pl022.c | 4 +++- hw/pl061.c | 4 +++- hw/ppc4xx_pci.c | 4 +++- hw/ppce500_pci.c | 4 +++- 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/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/unin_pci.c | 4 +++- hw/virtio-balloon.c | 3 ++- hw/virtio-blk.c | 4 +++- hw/virtio-net.c | 4 +++- hw/virtio-serial-bus.c | 4 +++- hw/zaurus.c | 4 +++- qemu-common.h | 2 +- savevm.c | 3 +-- slirp/slirp.c | 6 ++++-- target-arm/machine.c | 3 ++- target-cris/machine.c | 3 ++- target-i386/machine.c | 3 ++- 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 ++- 67 files changed, 219 insertions(+), 84 deletions(-) diff --git a/hw/adb.c b/hw/adb.c index 99b30f6..f400d12 100644 --- a/hw/adb.c +++ b/hw/adb.c @@ -261,7 +261,7 @@ static int adb_kbd_request(ADBDevice *d, uint8_t *obuf, return olen; } -static void adb_kbd_save(QEMUFile *f, void *opaque) +static int adb_kbd_save(QEMUFile *f, void *opaque) { KBDState *s = (KBDState *)opaque; @@ -269,6 +269,8 @@ static void adb_kbd_save(QEMUFile *f, void *opaque) qemu_put_sbe32s(f, &s->rptr); qemu_put_sbe32s(f, &s->wptr); qemu_put_sbe32s(f, &s->count); + + return 0; } static int adb_kbd_load(QEMUFile *f, void *opaque, int version_id) @@ -439,7 +441,7 @@ static int adb_mouse_reset(ADBDevice *d) return 0; } -static void adb_mouse_save(QEMUFile *f, void *opaque) +static int adb_mouse_save(QEMUFile *f, void *opaque) { MouseState *s = (MouseState *)opaque; @@ -448,6 +450,8 @@ static void adb_mouse_save(QEMUFile *f, void *opaque) qemu_put_sbe32s(f, &s->dx); qemu_put_sbe32s(f, &s->dy); qemu_put_sbe32s(f, &s->dz); + + return 0; } static int adb_mouse_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/ads7846.c b/hw/ads7846.c index b3bbeaf..4440ed2 100644 --- a/hw/ads7846.c +++ b/hw/ads7846.c @@ -105,7 +105,7 @@ static void ads7846_ts_event(void *opaque, } } -static void ads7846_save(QEMUFile *f, void *opaque) +static int ads7846_save(QEMUFile *f, void *opaque) { ADS7846State *s = (ADS7846State *) opaque; int i; @@ -115,6 +115,8 @@ static void ads7846_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->noise); qemu_put_be32(f, s->cycle); qemu_put_be32(f, s->output); + + return 0; } static int ads7846_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/arm_gic.c b/hw/arm_gic.c index 8286a28..7790a10 100644 --- a/hw/arm_gic.c +++ b/hw/arm_gic.c @@ -653,7 +653,7 @@ static void gic_reset(gic_state *s) #endif } -static void gic_save(QEMUFile *f, void *opaque) +static int gic_save(QEMUFile *f, void *opaque) { gic_state *s = (gic_state *)opaque; int i; @@ -685,6 +685,8 @@ static void gic_save(QEMUFile *f, void *opaque) qemu_put_byte(f, s->irq_state[i].model); qemu_put_byte(f, s->irq_state[i].trigger); } + + return 0; } static int gic_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/arm_timer.c b/hw/arm_timer.c index f009e9e..a6a4db4 100644 --- a/hw/arm_timer.c +++ b/hw/arm_timer.c @@ -140,13 +140,14 @@ static void arm_timer_tick(void *opaque) arm_timer_update(s); } -static void arm_timer_save(QEMUFile *f, void *opaque) +static int arm_timer_save(QEMUFile *f, void *opaque) { arm_timer_state *s = (arm_timer_state *)opaque; qemu_put_be32(f, s->control); qemu_put_be32(f, s->limit); qemu_put_be32(f, s->int_level); qemu_put_ptimer(f, s->timer); + return 0; } static int arm_timer_load(QEMUFile *f, void *opaque, int version_id) @@ -235,11 +236,12 @@ static CPUWriteMemoryFunc * const sp804_writefn[] = { sp804_write }; -static void sp804_save(QEMUFile *f, void *opaque) +static int sp804_save(QEMUFile *f, void *opaque) { sp804_state *s = (sp804_state *)opaque; qemu_put_be32(f, s->level[0]); qemu_put_be32(f, s->level[1]); + return 0; } static int sp804_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c index 6c7ce01..ec87962 100644 --- a/hw/armv7m_nvic.c +++ b/hw/armv7m_nvic.c @@ -365,7 +365,7 @@ static void nvic_writel(void *opaque, uint32_t offset, uint32_t value) } } -static void nvic_save(QEMUFile *f, void *opaque) +static int nvic_save(QEMUFile *f, void *opaque) { nvic_state *s = (nvic_state *)opaque; @@ -373,6 +373,8 @@ static void nvic_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->systick.reload); qemu_put_be64(f, s->systick.tick); qemu_put_timer(f, s->systick.timer); + + return 0; } static int nvic_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/cuda.c b/hw/cuda.c index 3f238b6..436ec93 100644 --- a/hw/cuda.c +++ b/hw/cuda.c @@ -654,7 +654,7 @@ static void cuda_save_timer(QEMUFile *f, CUDATimer *s) qemu_put_timer(f, s->timer); } -static void cuda_save(QEMUFile *f, void *opaque) +static int cuda_save(QEMUFile *f, void *opaque) { CUDAState *s = (CUDAState *)opaque; @@ -677,6 +677,8 @@ static void cuda_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &s->tick_offset); cuda_save_timer(f, &s->timers[0]); cuda_save_timer(f, &s->timers[1]); + + return 0; } static void cuda_load_timer(QEMUFile *f, CUDATimer *s) diff --git a/hw/g364fb.c b/hw/g364fb.c index 3c8fb98..b6c40bc 100644 --- a/hw/g364fb.c +++ b/hw/g364fb.c @@ -564,7 +564,7 @@ static int g364fb_load(QEMUFile *f, void *opaque, int version_id) return 0; } -static void g364fb_save(QEMUFile *f, void *opaque) +static int g364fb_save(QEMUFile *f, void *opaque) { G364State *s = opaque; int i; @@ -581,6 +581,8 @@ static void g364fb_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->top_of_screen); qemu_put_be32(f, s->width); qemu_put_be32(f, s->height); + + return 0; } int g364fb_mm_init(target_phys_addr_t vram_base, diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c index 91c755f..f6905fb 100644 --- a/hw/grackle_pci.c +++ b/hw/grackle_pci.c @@ -57,11 +57,13 @@ static void pci_grackle_set_irq(void *opaque, int irq_num, int level) qemu_set_irq(pic[irq_num + 0x15], level); } -static void pci_grackle_save(QEMUFile* f, void *opaque) +static int pci_grackle_save(QEMUFile* f, void *opaque) { PCIDevice *d = opaque; pci_device_save(d, f); + + return 0; } static int pci_grackle_load(QEMUFile* f, void *opaque, int version_id) diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c index cabf7ea..7d8c3b3 100644 --- a/hw/gt64xxx.c +++ b/hw/gt64xxx.c @@ -1086,10 +1086,11 @@ static void gt64120_reset(void *opaque) gt64120_pci_mapping(s); } -static void gt64120_save(QEMUFile* f, void *opaque) +static int gt64120_save(QEMUFile* f, void *opaque) { PCIDevice *d = opaque; pci_device_save(d, f); + return 0; } static int gt64120_load(QEMUFile* f, void *opaque, int version_id) diff --git a/hw/heathrow_pic.c b/hw/heathrow_pic.c index cd86121..66473a4 100644 --- a/hw/heathrow_pic.c +++ b/hw/heathrow_pic.c @@ -169,12 +169,14 @@ static void heathrow_pic_save_one(QEMUFile *f, HeathrowPIC *s) qemu_put_be32s(f, &s->level_triggered); } -static void heathrow_pic_save(QEMUFile *f, void *opaque) +static int heathrow_pic_save(QEMUFile *f, void *opaque) { HeathrowPICS *s = (HeathrowPICS *)opaque; heathrow_pic_save_one(f, &s->pics[0]); heathrow_pic_save_one(f, &s->pics[1]); + + return 0; } static void heathrow_pic_load_one(QEMUFile *f, HeathrowPIC *s) diff --git a/hw/hw.h b/hw/hw.h index 4405092..b6f1236 100644 --- a/hw/hw.h +++ b/hw/hw.h @@ -240,7 +240,7 @@ int64_t qemu_ftell(QEMUFile *f); int64_t qemu_fseek(QEMUFile *f, int64_t pos, int whence); typedef void SaveSetParamsHandler(int blk_enable, int shared, void * opaque); -typedef void SaveStateHandler(QEMUFile *f, void *opaque); +typedef int SaveStateHandler(QEMUFile *f, void *opaque); typedef int SaveLiveStateHandler(Monitor *mon, QEMUFile *f, int stage, void *opaque); typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id); diff --git a/hw/ivshmem.c b/hw/ivshmem.c index 06dce70..0919c4e 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -616,7 +616,7 @@ static void ivshmem_setup_msi(IVShmemState * s) { s->eventfd_table = qemu_mallocz(s->vectors * sizeof(EventfdEntry)); } -static void ivshmem_save(QEMUFile* f, void *opaque) +static int ivshmem_save(QEMUFile* f, void *opaque) { IVShmemState *proxy = opaque; @@ -630,6 +630,7 @@ static void ivshmem_save(QEMUFile* f, void *opaque) qemu_put_be32(f, proxy->intrmask); } + return 0; } static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) diff --git a/hw/m48t59.c b/hw/m48t59.c index c7492a6..9e0af08 100644 --- a/hw/m48t59.c +++ b/hw/m48t59.c @@ -585,13 +585,15 @@ static CPUReadMemoryFunc * const nvram_read[] = { &nvram_readl, }; -static void m48t59_save(QEMUFile *f, void *opaque) +static int m48t59_save(QEMUFile *f, void *opaque) { M48t59State *s = opaque; qemu_put_8s(f, &s->lock); qemu_put_be16s(f, &s->addr); qemu_put_buffer(f, s->buffer, s->size); + + return 0; } static int m48t59_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/mac_dbdma.c b/hw/mac_dbdma.c index 03d2d16..ffbe1c0 100644 --- a/hw/mac_dbdma.c +++ b/hw/mac_dbdma.c @@ -804,7 +804,7 @@ static CPUReadMemoryFunc * const dbdma_read[] = { dbdma_readl, }; -static void dbdma_save(QEMUFile *f, void *opaque) +static int dbdma_save(QEMUFile *f, void *opaque) { DBDMA_channel *s = opaque; unsigned int i, j; @@ -812,6 +812,8 @@ static void dbdma_save(QEMUFile *f, void *opaque) for (i = 0; i < DBDMA_CHANNELS; i++) for (j = 0; j < DBDMA_REGS; j++) qemu_put_be32s(f, &s[i].regs[j]); + + return 0; } static int dbdma_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/mac_nvram.c b/hw/mac_nvram.c index ce287c3..989acf3 100644 --- a/hw/mac_nvram.c +++ b/hw/mac_nvram.c @@ -105,11 +105,13 @@ static CPUReadMemoryFunc * const nvram_read[] = { &macio_nvram_readb, }; -static void macio_nvram_save(QEMUFile *f, void *opaque) +static int macio_nvram_save(QEMUFile *f, void *opaque) { MacIONVRAMState *s = (MacIONVRAMState *)opaque; qemu_put_buffer(f, s->data, s->size); + + return 0; } static int macio_nvram_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/max111x.c b/hw/max111x.c index 2844665..d467ad8 100644 --- a/hw/max111x.c +++ b/hw/max111x.c @@ -94,7 +94,7 @@ static uint32_t max111x_transfer(SSISlave *dev, uint32_t value) return max111x_read(s); } -static void max111x_save(QEMUFile *f, void *opaque) +static int max111x_save(QEMUFile *f, void *opaque) { MAX111xState *s = (MAX111xState *) opaque; int i; @@ -106,6 +106,8 @@ static void max111x_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->com); for (i = 0; i < s->inputs; i ++) qemu_put_byte(f, s->input[i]); + + return 0; } static int max111x_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/mipsnet.c b/hw/mipsnet.c index c5e54ff..9f49d1f 100644 --- a/hw/mipsnet.c +++ b/hw/mipsnet.c @@ -202,7 +202,7 @@ static void mipsnet_ioport_write(void *opaque, uint32_t addr, uint32_t val) } } -static void mipsnet_save(QEMUFile *f, void *opaque) +static int mipsnet_save(QEMUFile *f, void *opaque) { MIPSnetState *s = opaque; @@ -214,6 +214,8 @@ static void mipsnet_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &s->intctl); qemu_put_buffer(f, s->rx_buffer, MAX_ETH_FRAME_SIZE); qemu_put_buffer(f, s->tx_buffer, MAX_ETH_FRAME_SIZE); + + return 0; } static int mipsnet_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/mst_fpga.c b/hw/mst_fpga.c index 8fc348f..6402cac 100644 --- a/hw/mst_fpga.c +++ b/hw/mst_fpga.c @@ -175,7 +175,7 @@ static CPUWriteMemoryFunc * const mst_fpga_writefn[] = { mst_fpga_writeb, }; -static void +static int mst_fpga_save(QEMUFile *f, void *opaque) { struct mst_irq_state *s = (mst_irq_state *) opaque; @@ -193,6 +193,7 @@ mst_fpga_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &s->intsetclr); qemu_put_be32s(f, &s->pcmcia0); qemu_put_be32s(f, &s->pcmcia1); + return 0; } static int diff --git a/hw/nand.c b/hw/nand.c index f414aa1..c6b17a2 100644 --- a/hw/nand.c +++ b/hw/nand.c @@ -281,7 +281,7 @@ static void nand_command(NANDFlashState *s) } } -static void nand_save(QEMUFile *f, void *opaque) +static int nand_save(QEMUFile *f, void *opaque) { NANDFlashState *s = (NANDFlashState *) opaque; qemu_put_byte(f, s->cle); @@ -299,6 +299,7 @@ static void nand_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->status); qemu_put_be32(f, s->offset); /* XXX: do we want to save s->storage too? */ + return 0; } static int nand_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/openpic.c b/hw/openpic.c index 01bf15f..4ca4ba3 100644 --- a/hw/openpic.c +++ b/hw/openpic.c @@ -1052,7 +1052,7 @@ static void openpic_save_IRQ_queue(QEMUFile* f, IRQ_queue_t *q) qemu_put_sbe32s(f, &q->priority); } -static void openpic_save(QEMUFile* f, void *opaque) +static int openpic_save(QEMUFile* f, void *opaque) { openpic_t *opp = (openpic_t *)opaque; unsigned int i; @@ -1103,6 +1103,8 @@ static void openpic_save(QEMUFile* f, void *opaque) #endif pci_device_save(&opp->pci_dev, f); + + return 0; } static void openpic_load_IRQ_queue(QEMUFile* f, IRQ_queue_t *q) diff --git a/hw/piix4.c b/hw/piix4.c index 5489386..5209061 100644 --- a/hw/piix4.c +++ b/hw/piix4.c @@ -68,10 +68,11 @@ static void piix4_reset(void *opaque) pci_conf[0xae] = 0x00; } -static void piix_save(QEMUFile* f, void *opaque) +static int piix_save(QEMUFile* f, void *opaque) { PCIDevice *d = opaque; pci_device_save(d, f); + return 0; } static int piix_load(QEMUFile* f, void *opaque, int version_id) diff --git a/hw/pl011.c b/hw/pl011.c index 02cf84a..20bc431 100644 --- a/hw/pl011.c +++ b/hw/pl011.c @@ -235,7 +235,7 @@ static CPUWriteMemoryFunc * const pl011_writefn[] = { pl011_write }; -static void pl011_save(QEMUFile *f, void *opaque) +static int pl011_save(QEMUFile *f, void *opaque) { pl011_state *s = (pl011_state *)opaque; int i; @@ -256,6 +256,8 @@ static void pl011_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->read_pos); qemu_put_be32(f, s->read_count); qemu_put_be32(f, s->read_trigger); + + return 0; } static int pl011_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/pl022.c b/hw/pl022.c index d7862bc..b58c6c3 100644 --- a/hw/pl022.c +++ b/hw/pl022.c @@ -239,7 +239,7 @@ static CPUWriteMemoryFunc * const pl022_writefn[] = { pl022_write }; -static void pl022_save(QEMUFile *f, void *opaque) +static int pl022_save(QEMUFile *f, void *opaque) { pl022_state *s = (pl022_state *)opaque; int i; @@ -259,6 +259,8 @@ static void pl022_save(QEMUFile *f, void *opaque) qemu_put_be16(f, s->tx_fifo[i]); qemu_put_be16(f, s->rx_fifo[i]); } + + return 0; } static int pl022_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/pl061.c b/hw/pl061.c index e4505f5..02240dc 100644 --- a/hw/pl061.c +++ b/hw/pl061.c @@ -235,7 +235,7 @@ static CPUWriteMemoryFunc * const pl061_writefn[] = { pl061_write }; -static void pl061_save(QEMUFile *f, void *opaque) +static int pl061_save(QEMUFile *f, void *opaque) { pl061_state *s = (pl061_state *)opaque; @@ -259,6 +259,8 @@ static void pl061_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->den); qemu_put_be32(f, s->cr); qemu_put_be32(f, s->float_high); + + return 0; } static int pl061_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c index 6e437e7..7507d08 100644 --- a/hw/ppc4xx_pci.c +++ b/hw/ppc4xx_pci.c @@ -298,7 +298,7 @@ static void ppc4xx_pci_set_irq(void *opaque, int irq_num, int level) qemu_set_irq(pci_irqs[irq_num], level); } -static void ppc4xx_pci_save(QEMUFile *f, void *opaque) +static int ppc4xx_pci_save(QEMUFile *f, void *opaque) { PPC4xxPCIState *controller = opaque; int i; @@ -316,6 +316,8 @@ static void ppc4xx_pci_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &controller->ptm[i].ms); qemu_put_be32s(f, &controller->ptm[i].la); } + + return 0; } static int ppc4xx_pci_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c index 8ac99f2..9babe05 100644 --- a/hw/ppce500_pci.c +++ b/hw/ppce500_pci.c @@ -216,7 +216,7 @@ static void mpc85xx_pci_set_irq(void *opaque, int irq_num, int level) qemu_set_irq(pic[irq_num], level); } -static void ppce500_pci_save(QEMUFile *f, void *opaque) +static int ppce500_pci_save(QEMUFile *f, void *opaque) { PPCE500PCIState *controller = opaque; int i; @@ -237,6 +237,8 @@ static void ppce500_pci_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &controller->pib[i].piwar); } qemu_put_be32s(f, &controller->gasket_time); + + return 0; } static int ppce500_pci_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c index 6e04645..306b057 100644 --- a/hw/pxa2xx.c +++ b/hw/pxa2xx.c @@ -147,13 +147,15 @@ static CPUWriteMemoryFunc * const pxa2xx_pm_writefn[] = { pxa2xx_pm_write, }; -static void pxa2xx_pm_save(QEMUFile *f, void *opaque) +static int pxa2xx_pm_save(QEMUFile *f, void *opaque) { PXA2xxState *s = (PXA2xxState *) opaque; int i; for (i = 0; i < 0x40; i ++) qemu_put_be32s(f, &s->pm_regs[i]); + + return 0; } static int pxa2xx_pm_load(QEMUFile *f, void *opaque, int version_id) @@ -228,7 +230,7 @@ static CPUWriteMemoryFunc * const pxa2xx_cm_writefn[] = { pxa2xx_cm_write, }; -static void pxa2xx_cm_save(QEMUFile *f, void *opaque) +static int pxa2xx_cm_save(QEMUFile *f, void *opaque) { PXA2xxState *s = (PXA2xxState *) opaque; int i; @@ -237,6 +239,8 @@ static void pxa2xx_cm_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &s->cm_regs[i]); qemu_put_be32s(f, &s->clkcfg); qemu_put_be32s(f, &s->pmnc); + + return 0; } static int pxa2xx_cm_load(QEMUFile *f, void *opaque, int version_id) @@ -528,13 +532,15 @@ static CPUWriteMemoryFunc * const pxa2xx_mm_writefn[] = { pxa2xx_mm_write, }; -static void pxa2xx_mm_save(QEMUFile *f, void *opaque) +static int pxa2xx_mm_save(QEMUFile *f, void *opaque) { PXA2xxState *s = (PXA2xxState *) opaque; int i; for (i = 0; i < 0x1a; i ++) qemu_put_be32s(f, &s->mm_regs[i]); + + return 0; } static int pxa2xx_mm_load(QEMUFile *f, void *opaque, int version_id) @@ -804,7 +810,7 @@ static CPUWriteMemoryFunc * const pxa2xx_ssp_writefn[] = { pxa2xx_ssp_write, }; -static void pxa2xx_ssp_save(QEMUFile *f, void *opaque) +static int pxa2xx_ssp_save(QEMUFile *f, void *opaque) { PXA2xxSSPState *s = (PXA2xxSSPState *) opaque; int i; @@ -824,6 +830,8 @@ static void pxa2xx_ssp_save(QEMUFile *f, void *opaque) qemu_put_byte(f, s->rx_level); for (i = 0; i < s->rx_level; i ++) qemu_put_byte(f, s->rx_fifo[(s->rx_start + i) & 0xf]); + + return 0; } static int pxa2xx_ssp_load(QEMUFile *f, void *opaque, int version_id) @@ -1198,7 +1206,7 @@ static void pxa2xx_rtc_init(PXA2xxState *s) s->rtc_pi = qemu_new_timer(rt_clock, pxa2xx_rtc_pi_tick, s); } -static void pxa2xx_rtc_save(QEMUFile *f, void *opaque) +static int pxa2xx_rtc_save(QEMUFile *f, void *opaque) { PXA2xxState *s = (PXA2xxState *) opaque; @@ -1224,6 +1232,8 @@ static void pxa2xx_rtc_save(QEMUFile *f, void *opaque) qemu_put_sbe64s(f, &s->last_hz); qemu_put_sbe64s(f, &s->last_sw); qemu_put_sbe64s(f, &s->last_pi); + + return 0; } static int pxa2xx_rtc_load(QEMUFile *f, void *opaque, int version_id) @@ -1679,7 +1689,7 @@ static CPUWriteMemoryFunc * const pxa2xx_i2s_writefn[] = { pxa2xx_i2s_write, }; -static void pxa2xx_i2s_save(QEMUFile *f, void *opaque) +static int pxa2xx_i2s_save(QEMUFile *f, void *opaque) { PXA2xxI2SState *s = (PXA2xxI2SState *) opaque; @@ -1693,6 +1703,8 @@ static void pxa2xx_i2s_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->rx_len); qemu_put_be32(f, s->tx_len); qemu_put_be32(f, s->fifo_len); + + return 0; } static int pxa2xx_i2s_load(QEMUFile *f, void *opaque, int version_id) @@ -1955,7 +1967,7 @@ static void pxa2xx_fir_event(void *opaque, int event) { } -static void pxa2xx_fir_save(QEMUFile *f, void *opaque) +static int pxa2xx_fir_save(QEMUFile *f, void *opaque) { PXA2xxFIrState *s = (PXA2xxFIrState *) opaque; int i; @@ -1971,6 +1983,8 @@ static void pxa2xx_fir_save(QEMUFile *f, void *opaque) qemu_put_byte(f, s->rx_len); for (i = 0; i < s->rx_len; i ++) qemu_put_byte(f, s->rx_fifo[(s->rx_start + i) & 63]); + + return 0; } static int pxa2xx_fir_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/pxa2xx_dma.c b/hw/pxa2xx_dma.c index 9c479df..17d347c 100644 --- a/hw/pxa2xx_dma.c +++ b/hw/pxa2xx_dma.c @@ -428,7 +428,7 @@ static CPUWriteMemoryFunc * const pxa2xx_dma_writefn[] = { pxa2xx_dma_write }; -static void pxa2xx_dma_save(QEMUFile *f, void *opaque) +static int pxa2xx_dma_save(QEMUFile *f, void *opaque) { PXA2xxDMAState *s = (PXA2xxDMAState *) opaque; int i; @@ -452,6 +452,8 @@ static void pxa2xx_dma_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &s->chan[i].state); qemu_put_be32(f, s->chan[i].request); }; + + return 0; } static int pxa2xx_dma_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/pxa2xx_gpio.c b/hw/pxa2xx_gpio.c index 2abcb65..e46fe77 100644 --- a/hw/pxa2xx_gpio.c +++ b/hw/pxa2xx_gpio.c @@ -249,7 +249,7 @@ static CPUWriteMemoryFunc * const pxa2xx_gpio_writefn[] = { pxa2xx_gpio_write }; -static void pxa2xx_gpio_save(QEMUFile *f, void *opaque) +static int pxa2xx_gpio_save(QEMUFile *f, void *opaque) { PXA2xxGPIOInfo *s = (PXA2xxGPIOInfo *) opaque; int i; @@ -268,6 +268,8 @@ static void pxa2xx_gpio_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &s->prev_level[i]); } + + return 0; } static int pxa2xx_gpio_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/pxa2xx_keypad.c b/hw/pxa2xx_keypad.c index dfa8945..212339e 100644 --- a/hw/pxa2xx_keypad.c +++ b/hw/pxa2xx_keypad.c @@ -269,7 +269,7 @@ static CPUWriteMemoryFunc * const pxa2xx_keypad_writefn[] = { pxa2xx_keypad_write }; -static void pxa2xx_keypad_save(QEMUFile *f, void *opaque) +static int pxa2xx_keypad_save(QEMUFile *f, void *opaque) { PXA2xxKeyPadState *s = (PXA2xxKeyPadState *) opaque; @@ -284,6 +284,7 @@ static void pxa2xx_keypad_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &s->kpasmkp3); qemu_put_be32s(f, &s->kpkdi); + return 0; } static int pxa2xx_keypad_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c index 111a0dc..64d2c49 100644 --- a/hw/pxa2xx_lcd.c +++ b/hw/pxa2xx_lcd.c @@ -831,7 +831,7 @@ static void pxa2xx_lcdc_orientation(void *opaque, int angle) pxa2xx_lcdc_resize(s); } -static void pxa2xx_lcdc_save(QEMUFile *f, void *opaque) +static int pxa2xx_lcdc_save(QEMUFile *f, void *opaque) { PXA2xxLCDState *s = (PXA2xxLCDState *) opaque; int i; @@ -864,6 +864,8 @@ static void pxa2xx_lcdc_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &s->dma_ch[i].id); qemu_put_be32s(f, &s->dma_ch[i].command); } + + return 0; } static int pxa2xx_lcdc_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/pxa2xx_mmci.c b/hw/pxa2xx_mmci.c index ca98660..2f05dbc 100644 --- a/hw/pxa2xx_mmci.c +++ b/hw/pxa2xx_mmci.c @@ -439,7 +439,7 @@ static CPUWriteMemoryFunc * const pxa2xx_mmci_writefn[] = { pxa2xx_mmci_writew }; -static void pxa2xx_mmci_save(QEMUFile *f, void *opaque) +static int pxa2xx_mmci_save(QEMUFile *f, void *opaque) { PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque; int i; @@ -471,6 +471,8 @@ static void pxa2xx_mmci_save(QEMUFile *f, void *opaque) qemu_put_byte(f, s->resp_len); for (i = s->resp_len; i < 9; i ++) qemu_put_be16s(f, &s->resp_fifo[i]); + + return 0; } static int pxa2xx_mmci_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/pxa2xx_pic.c b/hw/pxa2xx_pic.c index 4d8944b..d95fa74 100644 --- a/hw/pxa2xx_pic.c +++ b/hw/pxa2xx_pic.c @@ -241,7 +241,7 @@ static CPUWriteMemoryFunc * const pxa2xx_pic_writefn[] = { pxa2xx_pic_mem_write, }; -static void pxa2xx_pic_save(QEMUFile *f, void *opaque) +static int pxa2xx_pic_save(QEMUFile *f, void *opaque) { PXA2xxPICState *s = (PXA2xxPICState *) opaque; int i; @@ -255,6 +255,8 @@ static void pxa2xx_pic_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &s->int_idle); for (i = 0; i < PXA2XX_PIC_SRCS; i ++) qemu_put_be32s(f, &s->priority[i]); + + return 0; } static int pxa2xx_pic_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/pxa2xx_timer.c b/hw/pxa2xx_timer.c index 0f0ffd3..d4dab2b 100644 --- a/hw/pxa2xx_timer.c +++ b/hw/pxa2xx_timer.c @@ -360,7 +360,7 @@ static void pxa2xx_timer_tick4(void *opaque) pxa2xx_timer_update4(i, qemu_get_clock(vm_clock), t->tm.num - 4); } -static void pxa2xx_timer_save(QEMUFile *f, void *opaque) +static int pxa2xx_timer_save(QEMUFile *f, void *opaque) { pxa2xx_timer_info *s = (pxa2xx_timer_info *) opaque; int i; @@ -388,6 +388,8 @@ static void pxa2xx_timer_save(QEMUFile *f, void *opaque) qemu_put_be32s(f, &s->irq_enabled); qemu_put_be32s(f, &s->reset3); qemu_put_be32s(f, &s->snapshot); + + return 0; } static int pxa2xx_timer_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/rc4030.c b/hw/rc4030.c index abbc3eb..e0b9610 100644 --- a/hw/rc4030.c +++ b/hw/rc4030.c @@ -650,7 +650,7 @@ static int rc4030_load(QEMUFile *f, void *opaque, int version_id) return 0; } -static void rc4030_save(QEMUFile *f, void *opaque) +static int rc4030_save(QEMUFile *f, void *opaque) { rc4030State* s = opaque; int i, j; @@ -675,6 +675,8 @@ static void rc4030_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->imr_jazz); qemu_put_be32(f, s->isr_jazz); qemu_put_be32(f, s->itr); + + return 0; } void rc4030_dma_memory_rw(void *opaque, target_phys_addr_t addr, uint8_t *buf, int len, int is_write) diff --git a/hw/spitz.c b/hw/spitz.c index a064460..c8228a6 100644 --- a/hw/spitz.c +++ b/hw/spitz.c @@ -130,12 +130,14 @@ static void sl_writeb(void *opaque, target_phys_addr_t addr, } } -static void sl_save(QEMUFile *f, void *opaque) +static int sl_save(QEMUFile *f, void *opaque) { SLNANDState *s = (SLNANDState *) opaque; qemu_put_8s(f, &s->ctl); ecc_put(f, &s->ecc); + + return 0; } static int sl_load(QEMUFile *f, void *opaque, int version_id) @@ -447,7 +449,7 @@ static void spitz_keyboard_pre_map(SpitzKeyboardState *s) #undef CTRL #undef FN -static void spitz_keyboard_save(QEMUFile *f, void *opaque) +static int spitz_keyboard_save(QEMUFile *f, void *opaque) { SpitzKeyboardState *s = (SpitzKeyboardState *) opaque; int i; @@ -456,6 +458,8 @@ static void spitz_keyboard_save(QEMUFile *f, void *opaque) qemu_put_be16s(f, &s->strobe_state); for (i = 0; i < 5; i ++) qemu_put_byte(f, spitz_gpio_invert[i]); + + return 0; } static int spitz_keyboard_load(QEMUFile *f, void *opaque, int version_id) @@ -591,11 +595,12 @@ static uint32_t spitz_lcdtg_transfer(SSISlave *dev, uint32_t value) return 0; } -static void spitz_lcdtg_save(QEMUFile *f, void *opaque) +static int spitz_lcdtg_save(QEMUFile *f, void *opaque) { SpitzLCDTG *s = (SpitzLCDTG *)opaque; qemu_put_be32(f, s->bl_intensity); qemu_put_be32(f, s->bl_power); + return 0; } static int spitz_lcdtg_load(QEMUFile *f, void *opaque, int version_id) @@ -676,7 +681,7 @@ static void spitz_adc_temp_on(void *opaque, int line, int level) max111x_set_input(max1111, MAX1111_BATT_TEMP, 0); } -static void spitz_ssp_save(QEMUFile *f, void *opaque) +static int spitz_ssp_save(QEMUFile *f, void *opaque) { CorgiSSPState *s = (CorgiSSPState *)opaque; int i; @@ -684,6 +689,7 @@ static void spitz_ssp_save(QEMUFile *f, void *opaque) for (i = 0; i < 3; i++) { qemu_put_be32(f, s->enable[i]); } + return 0; } static int spitz_ssp_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/ssd0323.c b/hw/ssd0323.c index 8643961..6058b94 100644 --- a/hw/ssd0323.c +++ b/hw/ssd0323.c @@ -275,7 +275,7 @@ static void ssd0323_cd(void *opaque, int n, int level) s->mode = level ? SSD0323_DATA : SSD0323_CMD; } -static void ssd0323_save(QEMUFile *f, void *opaque) +static int ssd0323_save(QEMUFile *f, void *opaque) { ssd0323_state *s = (ssd0323_state *)opaque; int i; @@ -294,6 +294,8 @@ static void ssd0323_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->remap); qemu_put_be32(f, s->mode); qemu_put_buffer(f, s->framebuffer, sizeof(s->framebuffer)); + + return 0; } static int ssd0323_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c index a1a63b2..99302d8 100644 --- a/hw/ssi-sd.c +++ b/hw/ssi-sd.c @@ -191,7 +191,7 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val) return 0xff; } -static void ssi_sd_save(QEMUFile *f, void *opaque) +static int ssi_sd_save(QEMUFile *f, void *opaque) { ssi_sd_state *s = (ssi_sd_state *)opaque; int i; @@ -205,6 +205,8 @@ static void ssi_sd_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->arglen); qemu_put_be32(f, s->response_pos); qemu_put_be32(f, s->stopping); + + return 0; } static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/stellaris.c b/hw/stellaris.c index ccad134..fbbf58d 100644 --- a/hw/stellaris.c +++ b/hw/stellaris.c @@ -280,7 +280,7 @@ static CPUWriteMemoryFunc * const gptm_writefn[] = { gptm_write }; -static void gptm_save(QEMUFile *f, void *opaque) +static int gptm_save(QEMUFile *f, void *opaque) { gptm_state *s = (gptm_state *)opaque; @@ -305,6 +305,8 @@ static void gptm_save(QEMUFile *f, void *opaque) qemu_put_be64(f, s->tick[1]); qemu_put_timer(f, s->timer[0]); qemu_put_timer(f, s->timer[1]); + + return 0; } static int gptm_load(QEMUFile *f, void *opaque, int version_id) @@ -604,7 +606,7 @@ static void ssys_reset(void *opaque) s->dcgc[0] = 1; } -static void ssys_save(QEMUFile *f, void *opaque) +static int ssys_save(QEMUFile *f, void *opaque) { ssys_state *s = (ssys_state *)opaque; @@ -625,6 +627,8 @@ static void ssys_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->dcgc[2]); qemu_put_be32(f, s->clkvclr); qemu_put_be32(f, s->ldoarst); + + return 0; } static int ssys_load(QEMUFile *f, void *opaque, int version_id) @@ -842,7 +846,7 @@ static CPUWriteMemoryFunc * const stellaris_i2c_writefn[] = { stellaris_i2c_write }; -static void stellaris_i2c_save(QEMUFile *f, void *opaque) +static int stellaris_i2c_save(QEMUFile *f, void *opaque) { stellaris_i2c_state *s = (stellaris_i2c_state *)opaque; @@ -853,6 +857,8 @@ static void stellaris_i2c_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->mimr); qemu_put_be32(f, s->mris); qemu_put_be32(f, s->mcr); + + return 0; } static int stellaris_i2c_load(QEMUFile *f, void *opaque, int version_id) @@ -1127,7 +1133,7 @@ static CPUWriteMemoryFunc * const stellaris_adc_writefn[] = { stellaris_adc_write }; -static void stellaris_adc_save(QEMUFile *f, void *opaque) +static int stellaris_adc_save(QEMUFile *f, void *opaque) { stellaris_adc_state *s = (stellaris_adc_state *)opaque; int i; @@ -1150,6 +1156,8 @@ static void stellaris_adc_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->ssctl[i]); } qemu_put_be32(f, s->noise); + + return 0; } static int stellaris_adc_load(QEMUFile *f, void *opaque, int version_id) @@ -1230,11 +1238,13 @@ static uint32_t stellaris_ssi_bus_transfer(SSISlave *dev, uint32_t val) return ssi_transfer(s->bus[s->current_dev], val); } -static void stellaris_ssi_bus_save(QEMUFile *f, void *opaque) +static int stellaris_ssi_bus_save(QEMUFile *f, void *opaque) { stellaris_ssi_bus_state *s = (stellaris_ssi_bus_state *)opaque; qemu_put_be32(f, s->current_dev); + + return 0; } static int stellaris_ssi_bus_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/stellaris_enet.c b/hw/stellaris_enet.c index 330a9d6..acca1ad 100644 --- a/hw/stellaris_enet.c +++ b/hw/stellaris_enet.c @@ -324,7 +324,7 @@ static void stellaris_enet_reset(stellaris_enet_state *s) s->tx_frame_len = -1; } -static void stellaris_enet_save(QEMUFile *f, void *opaque) +static int stellaris_enet_save(QEMUFile *f, void *opaque) { stellaris_enet_state *s = (stellaris_enet_state *)opaque; int i; @@ -350,6 +350,8 @@ static void stellaris_enet_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->next_packet); qemu_put_be32(f, s->rx_fifo - s->rx[s->next_packet].data); qemu_put_be32(f, s->rx_fifo_len); + + return 0; } static int stellaris_enet_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/stellaris_input.c b/hw/stellaris_input.c index 16aae96..b036045 100644 --- a/hw/stellaris_input.c +++ b/hw/stellaris_input.c @@ -47,7 +47,7 @@ static void stellaris_gamepad_put_key(void * opaque, int keycode) s->extension = 0; } -static void stellaris_gamepad_save(QEMUFile *f, void *opaque) +static int stellaris_gamepad_save(QEMUFile *f, void *opaque) { gamepad_state *s = (gamepad_state *)opaque; int i; @@ -55,6 +55,8 @@ static void stellaris_gamepad_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->extension); for (i = 0; i < s->num_buttons; i++) qemu_put_byte(f, s->buttons[i].pressed); + + return 0; } static int stellaris_gamepad_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/syborg_fb.c b/hw/syborg_fb.c index ed57203..94c3e5d 100644 --- a/hw/syborg_fb.c +++ b/hw/syborg_fb.c @@ -457,7 +457,7 @@ static CPUWriteMemoryFunc * const syborg_fb_writefn[] = { syborg_fb_write }; -static void syborg_fb_save(QEMUFile *f, void *opaque) +static int syborg_fb_save(QEMUFile *f, void *opaque) { SyborgFBState *s = opaque; int i; @@ -475,6 +475,8 @@ static void syborg_fb_save(QEMUFile *f, void *opaque) for (i = 0; i < 256; i++) { qemu_put_be32(f, s->raw_palette[i]); } + + return 0; } static int syborg_fb_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/syborg_interrupt.c b/hw/syborg_interrupt.c index 30140fb..271715b 100644 --- a/hw/syborg_interrupt.c +++ b/hw/syborg_interrupt.c @@ -168,7 +168,7 @@ static CPUWriteMemoryFunc * const syborg_int_writefn[] = { syborg_int_write }; -static void syborg_int_save(QEMUFile *f, void *opaque) +static int syborg_int_save(QEMUFile *f, void *opaque) { SyborgIntState *s = (SyborgIntState *)opaque; int i; @@ -179,6 +179,7 @@ static void syborg_int_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->flags[i].enabled | ((unsigned)s->flags[i].level << 1)); } + return 0; } static int syborg_int_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/syborg_keyboard.c b/hw/syborg_keyboard.c index 7709100..e08f4db 100644 --- a/hw/syborg_keyboard.c +++ b/hw/syborg_keyboard.c @@ -165,7 +165,7 @@ static void syborg_keyboard_event(void *opaque, int keycode) syborg_keyboard_update(s); } -static void syborg_keyboard_save(QEMUFile *f, void *opaque) +static int syborg_keyboard_save(QEMUFile *f, void *opaque) { SyborgKeyboardState *s = (SyborgKeyboardState *)opaque; int i; @@ -178,6 +178,7 @@ static void syborg_keyboard_save(QEMUFile *f, void *opaque) for (i = 0; i < s->fifo_size; i++) { qemu_put_be32(f, s->key_fifo[i]); } + return 0; } static int syborg_keyboard_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/syborg_pointer.c b/hw/syborg_pointer.c index 69b8d96..d8142a5 100644 --- a/hw/syborg_pointer.c +++ b/hw/syborg_pointer.c @@ -152,7 +152,7 @@ static void syborg_pointer_event(void *opaque, int dx, int dy, int dz, syborg_pointer_update(s); } -static void syborg_pointer_save(QEMUFile *f, void *opaque) +static int syborg_pointer_save(QEMUFile *f, void *opaque) { SyborgPointerState *s = (SyborgPointerState *)opaque; int i; @@ -168,6 +168,7 @@ static void syborg_pointer_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->event_fifo[i].z); qemu_put_be32(f, s->event_fifo[i].pointer_buttons); } + return 0; } static int syborg_pointer_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/syborg_rtc.c b/hw/syborg_rtc.c index 78d5edb..506f727 100644 --- a/hw/syborg_rtc.c +++ b/hw/syborg_rtc.c @@ -102,12 +102,14 @@ static CPUWriteMemoryFunc * const syborg_rtc_writefn[] = { syborg_rtc_write }; -static void syborg_rtc_save(QEMUFile *f, void *opaque) +static int syborg_rtc_save(QEMUFile *f, void *opaque) { SyborgRTCState *s = opaque; qemu_put_be64(f, s->offset); qemu_put_be64(f, s->data); + + return 0; } static int syborg_rtc_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/syborg_serial.c b/hw/syborg_serial.c index 8c42956..0f4e646 100644 --- a/hw/syborg_serial.c +++ b/hw/syborg_serial.c @@ -273,7 +273,7 @@ static CPUWriteMemoryFunc * const syborg_serial_writefn[] = { syborg_serial_write }; -static void syborg_serial_save(QEMUFile *f, void *opaque) +static int syborg_serial_save(QEMUFile *f, void *opaque) { SyborgSerialState *s = opaque; int i; @@ -288,6 +288,8 @@ static void syborg_serial_save(QEMUFile *f, void *opaque) for (i = 0; i < s->fifo_size; i++) { qemu_put_be32(f, s->read_fifo[i]); } + + return 0; } static int syborg_serial_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/syborg_timer.c b/hw/syborg_timer.c index 95e07d7..f0bf280 100644 --- a/hw/syborg_timer.c +++ b/hw/syborg_timer.c @@ -174,7 +174,7 @@ static CPUWriteMemoryFunc * const syborg_timer_writefn[] = { syborg_timer_write }; -static void syborg_timer_save(QEMUFile *f, void *opaque) +static int syborg_timer_save(QEMUFile *f, void *opaque) { SyborgTimerState *s = opaque; @@ -184,6 +184,8 @@ static void syborg_timer_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->int_level); qemu_put_be32(f, s->int_enabled); qemu_put_ptimer(f, s->timer); + + return 0; } static int syborg_timer_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/tsc2005.c b/hw/tsc2005.c index a55853c..4ae92e1 100644 --- a/hw/tsc2005.c +++ b/hw/tsc2005.c @@ -432,7 +432,7 @@ static void tsc2005_touchscreen_event(void *opaque, tsc2005_pin_update(s); } -static void tsc2005_save(QEMUFile *f, void *opaque) +static int tsc2005_save(QEMUFile *f, void *opaque) { TSC2005State *s = (TSC2005State *) opaque; int i; @@ -471,6 +471,8 @@ static void tsc2005_save(QEMUFile *f, void *opaque) for (i = 0; i < 8; i ++) qemu_put_be32(f, s->tr[i]); + + return 0; } static int tsc2005_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/tsc210x.c b/hw/tsc210x.c index fca73f1..bb4ee14 100644 --- a/hw/tsc210x.c +++ b/hw/tsc210x.c @@ -1002,7 +1002,7 @@ static void tsc210x_i2s_set_rate(TSC210xState *s, int in, int out) s->i2s_rx_rate = in; } -static void tsc210x_save(QEMUFile *f, void *opaque) +static int tsc210x_save(QEMUFile *f, void *opaque) { TSC210xState *s = (TSC210xState *) opaque; int64_t now = qemu_get_clock(vm_clock); @@ -1046,6 +1046,8 @@ static void tsc210x_save(QEMUFile *f, void *opaque) for (i = 0; i < 0x14; i ++) qemu_put_be16s(f, &s->filter_data[i]); + + return 0; } static int tsc210x_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/unin_pci.c b/hw/unin_pci.c index 1310211..ca15a00 100644 --- a/hw/unin_pci.c +++ b/hw/unin_pci.c @@ -63,11 +63,13 @@ static void pci_unin_set_irq(void *opaque, int irq_num, int level) qemu_set_irq(pic[unin_irq_line[irq_num]], level); } -static void pci_unin_save(QEMUFile* f, void *opaque) +static int pci_unin_save(QEMUFile* f, void *opaque) { PCIDevice *d = opaque; pci_device_save(d, f); + + return 0; } static int pci_unin_load(QEMUFile* f, void *opaque, int version_id) diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c index 1e74674..719e72c 100644 --- a/hw/virtio-balloon.c +++ b/hw/virtio-balloon.c @@ -227,7 +227,7 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target, } } -static void virtio_balloon_save(QEMUFile *f, void *opaque) +static int virtio_balloon_save(QEMUFile *f, void *opaque) { VirtIOBalloon *s = opaque; @@ -235,6 +235,7 @@ static void virtio_balloon_save(QEMUFile *f, void *opaque) qemu_put_be32(f, s->num_pages); qemu_put_be32(f, s->actual); + return 0; } static int virtio_balloon_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index a1df26d..3770901 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -460,7 +460,7 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features) return features; } -static void virtio_blk_save(QEMUFile *f, void *opaque) +static int virtio_blk_save(QEMUFile *f, void *opaque) { VirtIOBlock *s = opaque; VirtIOBlockReq *req = s->rq; @@ -473,6 +473,8 @@ static void virtio_blk_save(QEMUFile *f, void *opaque) req = req->next; } qemu_put_sbyte(f, 0); + + return 0; } static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 0a9cae2..1b683d9 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -779,7 +779,7 @@ static void virtio_net_tx_bh(void *opaque) } } -static void virtio_net_save(QEMUFile *f, void *opaque) +static int virtio_net_save(QEMUFile *f, void *opaque) { VirtIONet *n = opaque; @@ -808,6 +808,8 @@ static void virtio_net_save(QEMUFile *f, void *opaque) qemu_put_byte(f, n->nouni); qemu_put_byte(f, n->nobcast); qemu_put_byte(f, n->has_ufo); + + return 0; } static int virtio_net_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 74ba5ec..7f00fcf 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -453,7 +453,7 @@ static void set_config(VirtIODevice *vdev, const uint8_t *config_data) memcpy(&config, config_data, sizeof(config)); } -static void virtio_serial_save(QEMUFile *f, void *opaque) +static int virtio_serial_save(QEMUFile *f, void *opaque) { VirtIOSerial *s = opaque; VirtIOSerialPort *port; @@ -492,6 +492,8 @@ static void virtio_serial_save(QEMUFile *f, void *opaque) qemu_put_byte(f, port->guest_connected); qemu_put_byte(f, port->host_connected); } + + return 0; } static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id) diff --git a/hw/zaurus.c b/hw/zaurus.c index dd999d7..a3468b4 100644 --- a/hw/zaurus.c +++ b/hw/zaurus.c @@ -178,7 +178,7 @@ void scoop_gpio_out_set(ScoopInfo *s, int line, s->handler[line] = handler; } -static void scoop_save(QEMUFile *f, void *opaque) +static int scoop_save(QEMUFile *f, void *opaque) { ScoopInfo *s = (ScoopInfo *) opaque; qemu_put_be16s(f, &s->status); @@ -192,6 +192,8 @@ static void scoop_save(QEMUFile *f, void *opaque) qemu_put_be16s(f, &s->irr); qemu_put_be16s(f, &s->imr); qemu_put_be16s(f, &s->isr); + + return 0; } static int scoop_load(QEMUFile *f, void *opaque, int version_id) diff --git a/qemu-common.h b/qemu-common.h index 81aafa0..f847cda 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -256,7 +256,7 @@ typedef enum { void cpu_exec_init_all(unsigned long tb_size); /* CPU save/load. */ -void cpu_save(QEMUFile *f, void *opaque); +int cpu_save(QEMUFile *f, void *opaque); int cpu_load(QEMUFile *f, void *opaque, int version_id); /* Force QEMU to stop what it's doing and service IO */ diff --git a/savevm.c b/savevm.c index 6fa7a5f..f8a7819 100644 --- a/savevm.c +++ b/savevm.c @@ -1387,8 +1387,7 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se) } if (!se->vmsd) { /* Old style */ - se->save_state(f, se->opaque); - return 0; + return se->save_state(f, se->opaque); } vmstate_save_state(f,se->vmsd, se->opaque); diff --git a/slirp/slirp.c b/slirp/slirp.c index 332d83b..b9d0347 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -193,7 +193,7 @@ static void slirp_init_once(void) loopback_addr.s_addr = htonl(INADDR_LOOPBACK); } -static void slirp_state_save(QEMUFile *f, void *opaque); +static int slirp_state_save(QEMUFile *f, void *opaque); static int slirp_state_load(QEMUFile *f, void *opaque, int version_id); Slirp *slirp_init(int restricted, struct in_addr vnetwork, @@ -940,7 +940,7 @@ static void slirp_bootp_save(QEMUFile *f, Slirp *slirp) } } -static void slirp_state_save(QEMUFile *f, void *opaque) +static int slirp_state_save(QEMUFile *f, void *opaque) { Slirp *slirp = opaque; struct ex_list *ex_ptr; @@ -961,6 +961,8 @@ static void slirp_state_save(QEMUFile *f, void *opaque) qemu_put_be16(f, slirp->ip_id); slirp_bootp_save(f, slirp); + + return 0; } static void slirp_tcp_load(QEMUFile *f, struct tcpcb *tp) diff --git a/target-arm/machine.c b/target-arm/machine.c index 3925d3a..744ad3c 100644 --- a/target-arm/machine.c +++ b/target-arm/machine.c @@ -1,7 +1,7 @@ #include "hw/hw.h" #include "hw/boards.h" -void cpu_save(QEMUFile *f, void *opaque) +int cpu_save(QEMUFile *f, void *opaque) { int i; CPUARMState *env = (CPUARMState *)opaque; @@ -99,6 +99,7 @@ void cpu_save(QEMUFile *f, void *opaque) qemu_put_be32(f, env->teecr); qemu_put_be32(f, env->teehbr); } + return 0; } int cpu_load(QEMUFile *f, void *opaque, int version_id) diff --git a/target-cris/machine.c b/target-cris/machine.c index 8f9c0dd..66cb613 100644 --- a/target-cris/machine.c +++ b/target-cris/machine.c @@ -1,7 +1,7 @@ #include "hw/hw.h" #include "hw/boards.h" -void cpu_save(QEMUFile *f, void *opaque) +int cpu_save(QEMUFile *f, void *opaque) { CPUCRISState *env = opaque; int i; @@ -42,6 +42,7 @@ void cpu_save(QEMUFile *f, void *opaque) } } } + return 0; } int cpu_load(QEMUFile *f, void *opaque, int version_id) diff --git a/target-i386/machine.c b/target-i386/machine.c index 5f8376c..540292f 100644 --- a/target-i386/machine.c +++ b/target-i386/machine.c @@ -478,9 +478,10 @@ static const VMStateDescription vmstate_cpu = { } }; -void cpu_save(QEMUFile *f, void *opaque) +int cpu_save(QEMUFile *f, void *opaque) { vmstate_save_state(f, &vmstate_cpu, opaque); + return 0; } int cpu_load(QEMUFile *f, void *opaque, int version_id) diff --git a/target-microblaze/machine.c b/target-microblaze/machine.c index 1be1c35..e5f9031 100644 --- a/target-microblaze/machine.c +++ b/target-microblaze/machine.c @@ -1,8 +1,9 @@ #include "hw/hw.h" #include "hw/boards.h" -void cpu_save(QEMUFile *f, void *opaque) +int cpu_save(QEMUFile *f, void *opaque) { + return 0; } int cpu_load(QEMUFile *f, void *opaque, int version_id) diff --git a/target-mips/machine.c b/target-mips/machine.c index 9ffac71..7d8a67c 100644 --- a/target-mips/machine.c +++ b/target-mips/machine.c @@ -40,7 +40,7 @@ static void save_fpu(QEMUFile *f, CPUMIPSFPUContext *fpu) qemu_put_be32s(f, &fpu->fcr31); } -void cpu_save(QEMUFile *f, void *opaque) +int cpu_save(QEMUFile *f, void *opaque) { CPUState *env = opaque; int i; @@ -149,6 +149,7 @@ void cpu_save(QEMUFile *f, void *opaque) save_tc(f, &env->tcs[i]); for (i = 0; i < MIPS_FPU_MAX; i++) save_fpu(f, &env->fpus[i]); + return 0; } static void load_tc(QEMUFile *f, TCState *tc) diff --git a/target-ppc/machine.c b/target-ppc/machine.c index 67de951..df8a085 100644 --- a/target-ppc/machine.c +++ b/target-ppc/machine.c @@ -2,7 +2,7 @@ #include "hw/boards.h" #include "kvm.h" -void cpu_save(QEMUFile *f, void *opaque) +int cpu_save(QEMUFile *f, void *opaque) { CPUState *env = (CPUState *)opaque; unsigned int i, j; @@ -87,6 +87,7 @@ void cpu_save(QEMUFile *f, void *opaque) qemu_put_betls(f, &env->hflags_nmsr); qemu_put_sbe32s(f, &env->mmu_idx); qemu_put_sbe32s(f, &env->power_mode); + return 0; } int cpu_load(QEMUFile *f, void *opaque, int version_id) diff --git a/target-s390x/machine.c b/target-s390x/machine.c index 3e79be6..11161fd 100644 --- a/target-s390x/machine.c +++ b/target-s390x/machine.c @@ -20,8 +20,9 @@ #include "hw/hw.h" #include "hw/boards.h" -void cpu_save(QEMUFile *f, void *opaque) +int cpu_save(QEMUFile *f, void *opaque) { + return 0; } int cpu_load(QEMUFile *f, void *opaque, int version_id) diff --git a/target-sparc/machine.c b/target-sparc/machine.c index 752e431..c574ad8 100644 --- a/target-sparc/machine.c +++ b/target-sparc/machine.c @@ -4,7 +4,7 @@ #include "exec-all.h" -void cpu_save(QEMUFile *f, void *opaque) +int cpu_save(QEMUFile *f, void *opaque) { CPUState *env = opaque; int i; @@ -98,6 +98,7 @@ void cpu_save(QEMUFile *f, void *opaque) qemu_put_be64s(f, &env->ssr); cpu_put_timer(f, env->hstick); #endif + return 0; } int cpu_load(QEMUFile *f, void *opaque, int version_id) ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 2/6] savevm: Allow vmsd->pre_save to return error 2010-10-06 20:58 [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 1/6] savevm: Allow SaveStateHandler() to return error Alex Williamson @ 2010-10-06 20:59 ` Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 3/6] pci: Allow pci_device_save() " Alex Williamson ` (6 subsequent siblings) 8 siblings, 0 replies; 26+ messages in thread From: Alex Williamson @ 2010-10-06 20:59 UTC (permalink / raw) To: qemu-devel; +Cc: alex.williamson, cam, kvm, quintela This allows vmsd based saves to also have a way to signal that they can't be saved or migrated. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- hw/fdc.c | 3 ++- hw/hpet.c | 3 ++- hw/hw.h | 6 +++--- hw/i2c.c | 3 ++- hw/ide/core.c | 4 +++- hw/lsi53c895a.c | 4 +++- hw/rtl8139.c | 4 +++- hw/serial.c | 3 ++- hw/twl92230.c | 3 ++- hw/usb-uhci.c | 3 ++- hw/wm8750.c | 3 ++- savevm.c | 36 +++++++++++++++++++++++------------- target-i386/machine.c | 6 +++--- 13 files changed, 52 insertions(+), 29 deletions(-) diff --git a/hw/fdc.c b/hw/fdc.c index c159dcb..ff48c70 100644 --- a/hw/fdc.c +++ b/hw/fdc.c @@ -643,11 +643,12 @@ static const VMStateDescription vmstate_fdrive = { } }; -static void fdc_pre_save(void *opaque) +static int fdc_pre_save(void *opaque) { FDCtrl *s = opaque; s->dor_vmstate = s->dor | GET_CUR_DRV(s); + return 0; } static int fdc_post_load(void *opaque, int version_id) diff --git a/hw/hpet.c b/hw/hpet.c index d5c406c..e586e68 100644 --- a/hw/hpet.c +++ b/hw/hpet.c @@ -204,12 +204,13 @@ static void update_irq(struct HPETTimer *timer, int set) } } -static void hpet_pre_save(void *opaque) +static int hpet_pre_save(void *opaque) { HPETState *s = opaque; /* save current counter value */ s->hpet_counter = hpet_get_ticks(s); + return 0; } static int hpet_pre_load(void *opaque) diff --git a/hw/hw.h b/hw/hw.h index b6f1236..91a60ca 100644 --- a/hw/hw.h +++ b/hw/hw.h @@ -328,7 +328,7 @@ struct VMStateDescription { LoadStateHandler *load_state_old; int (*pre_load)(void *opaque); int (*post_load)(void *opaque, int version_id); - void (*pre_save)(void *opaque); + int (*pre_save)(void *opaque); VMStateField *fields; const VMStateSubsection *subsections; }; @@ -773,8 +773,8 @@ extern const VMStateDescription vmstate_i2c_slave; extern int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, void *opaque, int version_id); -extern void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, - void *opaque); +extern int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, + void *opaque); extern int vmstate_register(DeviceState *dev, int instance_id, const VMStateDescription *vmsd, void *base); extern int vmstate_register_with_alias_id(DeviceState *dev, diff --git a/hw/i2c.c b/hw/i2c.c index f80d12d..f05c2ef 100644 --- a/hw/i2c.c +++ b/hw/i2c.c @@ -26,11 +26,12 @@ static struct BusInfo i2c_bus_info = { } }; -static void i2c_bus_pre_save(void *opaque) +static int i2c_bus_pre_save(void *opaque) { i2c_bus *bus = opaque; bus->saved_address = bus->current_dev ? bus->current_dev->address : -1; + return 0; } static int i2c_bus_post_load(void *opaque, int version_id) diff --git a/hw/ide/core.c b/hw/ide/core.c index 06b6e14..eb5f095 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -2792,7 +2792,7 @@ static int ide_drive_pio_post_load(void *opaque, int version_id) return 0; } -static void ide_drive_pio_pre_save(void *opaque) +static int ide_drive_pio_pre_save(void *opaque) { IDEState *s = opaque; int idx; @@ -2808,6 +2808,8 @@ static void ide_drive_pio_pre_save(void *opaque) } else { s->end_transfer_fn_idx = idx; } + + return 0; } static bool ide_drive_pio_state_needed(void *opaque) diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c index 5eaf69e..7315a3f 100644 --- a/hw/lsi53c895a.c +++ b/hw/lsi53c895a.c @@ -2045,7 +2045,7 @@ static void lsi_scsi_reset(DeviceState *dev) lsi_soft_reset(s); } -static void lsi_pre_save(void *opaque) +static int lsi_pre_save(void *opaque) { LSIState *s = opaque; @@ -2054,6 +2054,8 @@ static void lsi_pre_save(void *opaque) assert(s->current->dma_len == 0); } assert(QTAILQ_EMPTY(&s->queue)); + + return 0; } static const VMStateDescription vmstate_lsi_scsi = { diff --git a/hw/rtl8139.c b/hw/rtl8139.c index d92981d..56271fb 100644 --- a/hw/rtl8139.c +++ b/hw/rtl8139.c @@ -3173,7 +3173,7 @@ static int rtl8139_post_load(void *opaque, int version_id) return 0; } -static void rtl8139_pre_save(void *opaque) +static int rtl8139_pre_save(void *opaque) { RTL8139State* s = opaque; int64_t current_time = qemu_get_clock(vm_clock); @@ -3182,6 +3182,8 @@ static void rtl8139_pre_save(void *opaque) rtl8139_set_next_tctr_time(s, current_time); s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY, get_ticks_per_sec()); + + return 0; } static const VMStateDescription vmstate_rtl8139 = { diff --git a/hw/serial.c b/hw/serial.c index 9ebc452..edfdd4d 100644 --- a/hw/serial.c +++ b/hw/serial.c @@ -659,10 +659,11 @@ static void serial_event(void *opaque, int event) serial_receive_break(s); } -static void serial_pre_save(void *opaque) +static int serial_pre_save(void *opaque) { SerialState *s = opaque; s->fcr_vmstate = s->fcr; + return 0; } static int serial_post_load(void *opaque, int version_id) diff --git a/hw/twl92230.c b/hw/twl92230.c index e61f17f..0d6f3b6 100644 --- a/hw/twl92230.c +++ b/hw/twl92230.c @@ -782,11 +782,12 @@ static const VMStateDescription vmstate_menelaus_tm = { } }; -static void menelaus_pre_save(void *opaque) +static int menelaus_pre_save(void *opaque) { MenelausState *s = opaque; /* Should be <= 1000 */ s->rtc_next_vmstate = s->rtc.next - qemu_get_clock(rt_clock); + return 0; } static int menelaus_post_load(void *opaque, int version_id) diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c index 1d83400..ea00386 100644 --- a/hw/usb-uhci.c +++ b/hw/usb-uhci.c @@ -355,11 +355,12 @@ static void uhci_reset(void *opaque) uhci_async_cancel_all(s); } -static void uhci_pre_save(void *opaque) +static int uhci_pre_save(void *opaque) { UHCIState *s = opaque; uhci_async_cancel_all(s); + return 0; } static const VMStateDescription vmstate_uhci_port = { diff --git a/hw/wm8750.c b/hw/wm8750.c index ce43c23..f41a6f0 100644 --- a/hw/wm8750.c +++ b/hw/wm8750.c @@ -564,11 +564,12 @@ static int wm8750_rx(i2c_slave *i2c) return 0x00; } -static void wm8750_pre_save(void *opaque) +static int wm8750_pre_save(void *opaque) { WM8750State *s = opaque; s->rate_vmstate = (s->rate - wm_rate_table) / sizeof(*s->rate); + return 0; } static int wm8750_post_load(void *opaque, int version_id) diff --git a/savevm.c b/savevm.c index f8a7819..89c5fac 100644 --- a/savevm.c +++ b/savevm.c @@ -1244,8 +1244,8 @@ void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd, } } -static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, - void *opaque); +static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, + void *opaque); static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd, void *opaque); @@ -1323,13 +1323,17 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, return 0; } -void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, - void *opaque) +int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, + void *opaque) { VMStateField *field = vmsd->fields; + int ret; if (vmsd->pre_save) { - vmsd->pre_save(opaque); + ret = vmsd->pre_save(opaque); + if (ret < 0) { + return ret; + } } while(field->name) { if (!field->field_exists || @@ -1361,7 +1365,10 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, addr = *(void **)addr; } if (field->flags & VMS_STRUCT) { - vmstate_save_state(f, field->vmsd, addr); + ret = vmstate_save_state(f, field->vmsd, addr); + if (ret < 0) { + return ret; + } } else { field->info->put(f, addr, size); } @@ -1369,7 +1376,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, } field++; } - vmstate_subsection_save(f, vmsd, opaque); + return vmstate_subsection_save(f, vmsd, opaque); } static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id) @@ -1389,9 +1396,7 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se) if (!se->vmsd) { /* Old style */ return se->save_state(f, se->opaque); } - vmstate_save_state(f,se->vmsd, se->opaque); - - return 0; + return vmstate_save_state(f, se->vmsd, se->opaque); } #define QEMU_VM_FILE_MAGIC 0x5145564d @@ -1635,8 +1640,8 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd, return 0; } -static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, - void *opaque) +static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, + void *opaque) { const VMStateSubsection *sub = vmsd->subsections; @@ -1644,16 +1649,21 @@ static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, if (sub->needed(opaque)) { const VMStateDescription *vmsd = sub->vmsd; uint8_t len; + int ret; qemu_put_byte(f, QEMU_VM_SUBSECTION); len = strlen(vmsd->name); qemu_put_byte(f, len); qemu_put_buffer(f, (uint8_t *)vmsd->name, len); qemu_put_be32(f, vmsd->version_id); - vmstate_save_state(f, vmsd, opaque); + ret = vmstate_save_state(f, vmsd, opaque); + if (ret < 0) { + return ret; + } } sub++; } + return 0; } typedef struct LoadStateEntry { diff --git a/target-i386/machine.c b/target-i386/machine.c index 540292f..e47206c 100644 --- a/target-i386/machine.c +++ b/target-i386/machine.c @@ -332,7 +332,7 @@ static const VMStateInfo vmstate_hack_uint64_as_uint32 = { VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_hack_uint64_as_uint32, uint64_t) #endif -static void cpu_pre_save(void *opaque) +static int cpu_pre_save(void *opaque) { CPUState *env = opaque; int i; @@ -349,6 +349,7 @@ static void cpu_pre_save(void *opaque) #else env->fpregs_format_vmstate = 1; #endif + return 0; } static int cpu_post_load(void *opaque, int version_id) @@ -480,8 +481,7 @@ static const VMStateDescription vmstate_cpu = { int cpu_save(QEMUFile *f, void *opaque) { - vmstate_save_state(f, &vmstate_cpu, opaque); - return 0; + return vmstate_save_state(f, &vmstate_cpu, opaque); } int cpu_load(QEMUFile *f, void *opaque, int version_id) ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 3/6] pci: Allow pci_device_save() to return error 2010-10-06 20:58 [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 1/6] savevm: Allow SaveStateHandler() to return error Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 2/6] savevm: Allow vmsd->pre_save " Alex Williamson @ 2010-10-06 20:59 ` Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 4/6] virtio: Allow virtio_save() errors Alex Williamson ` (5 subsequent siblings) 8 siblings, 0 replies; 26+ messages in thread From: Alex Williamson @ 2010-10-06 20:59 UTC (permalink / raw) To: qemu-devel; +Cc: alex.williamson, cam, kvm, quintela Carry the vmsd pre_save error reporting through pci_device_save(). Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- hw/grackle_pci.c | 4 +--- hw/gt64xxx.c | 3 +-- hw/ivshmem.c | 7 ++++++- hw/openpic.c | 4 +--- hw/pci.c | 9 +++++++-- hw/pci.h | 2 +- hw/piix4.c | 3 +-- hw/ppc4xx_pci.c | 7 +++++-- hw/ppce500_pci.c | 7 +++++-- hw/unin_pci.c | 4 +--- 10 files changed, 29 insertions(+), 21 deletions(-) diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c index f6905fb..c7164c5 100644 --- a/hw/grackle_pci.c +++ b/hw/grackle_pci.c @@ -61,9 +61,7 @@ static int pci_grackle_save(QEMUFile* f, void *opaque) { PCIDevice *d = opaque; - pci_device_save(d, f); - - return 0; + return pci_device_save(d, f); } static int pci_grackle_load(QEMUFile* f, void *opaque, int version_id) diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c index 7d8c3b3..21a0e57 100644 --- a/hw/gt64xxx.c +++ b/hw/gt64xxx.c @@ -1089,8 +1089,7 @@ static void gt64120_reset(void *opaque) static int gt64120_save(QEMUFile* f, void *opaque) { PCIDevice *d = opaque; - pci_device_save(d, f); - return 0; + return pci_device_save(d, f); } static int gt64120_load(QEMUFile* f, void *opaque, int version_id) diff --git a/hw/ivshmem.c b/hw/ivshmem.c index 0919c4e..3726a7f 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -619,9 +619,14 @@ static void ivshmem_setup_msi(IVShmemState * s) { static int ivshmem_save(QEMUFile* f, void *opaque) { IVShmemState *proxy = opaque; + int ret; IVSHMEM_DPRINTF("ivshmem_save\n"); - pci_device_save(&proxy->dev, f); + + ret = pci_device_save(&proxy->dev, f); + if (ret < 0) { + return ret; + } if (ivshmem_has_feature(proxy, IVSHMEM_MSI)) { msix_save(&proxy->dev, f); diff --git a/hw/openpic.c b/hw/openpic.c index 4ca4ba3..4537239 100644 --- a/hw/openpic.c +++ b/hw/openpic.c @@ -1102,9 +1102,7 @@ static int openpic_save(QEMUFile* f, void *opaque) } #endif - pci_device_save(&opp->pci_dev, f); - - return 0; + return pci_device_save(&opp->pci_dev, f); } static void openpic_load_IRQ_queue(QEMUFile* f, IRQ_queue_t *q) diff --git a/hw/pci.c b/hw/pci.c index 15416dd..a30f6ec 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -434,16 +434,21 @@ static inline const VMStateDescription *pci_get_vmstate(PCIDevice *s) return pci_is_express(s) ? &vmstate_pcie_device : &vmstate_pci_device; } -void pci_device_save(PCIDevice *s, QEMUFile *f) +int pci_device_save(PCIDevice *s, QEMUFile *f) { + int ret; /* Clear interrupt status bit: it is implicit * in irq_state which we are saving. * This makes us compatible with old devices * which never set or clear this bit. */ s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT; - vmstate_save_state(f, pci_get_vmstate(s), s); + ret = vmstate_save_state(f, pci_get_vmstate(s), s); + if (ret < 0) { + return ret; + } /* Restore the interrupt status bit. */ pci_update_irq_status(s); + return 0; } int pci_device_load(PCIDevice *s, QEMUFile *f) diff --git a/hw/pci.h b/hw/pci.h index 3d23f03..bb9ad79 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -198,7 +198,7 @@ uint32_t pci_default_read_config(PCIDevice *d, uint32_t address, int len); void pci_default_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len); -void pci_device_save(PCIDevice *s, QEMUFile *f); +int pci_device_save(PCIDevice *s, QEMUFile *f); int pci_device_load(PCIDevice *s, QEMUFile *f); typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level); diff --git a/hw/piix4.c b/hw/piix4.c index 5209061..9f560ac 100644 --- a/hw/piix4.c +++ b/hw/piix4.c @@ -71,8 +71,7 @@ static void piix4_reset(void *opaque) static int piix_save(QEMUFile* f, void *opaque) { PCIDevice *d = opaque; - pci_device_save(d, f); - return 0; + return pci_device_save(d, f); } static int piix_load(QEMUFile* f, void *opaque, int version_id) diff --git a/hw/ppc4xx_pci.c b/hw/ppc4xx_pci.c index 7507d08..3499270 100644 --- a/hw/ppc4xx_pci.c +++ b/hw/ppc4xx_pci.c @@ -301,9 +301,12 @@ static void ppc4xx_pci_set_irq(void *opaque, int irq_num, int level) static int ppc4xx_pci_save(QEMUFile *f, void *opaque) { PPC4xxPCIState *controller = opaque; - int i; + int i, ret; - pci_device_save(controller->pci_dev, f); + ret = pci_device_save(controller->pci_dev, f); + if (ret < 0) { + return ret; + } for (i = 0; i < PPC4xx_PCI_NR_PMMS; i++) { qemu_put_be32s(f, &controller->pmm[i].la); diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c index 9babe05..97a7743 100644 --- a/hw/ppce500_pci.c +++ b/hw/ppce500_pci.c @@ -219,9 +219,12 @@ static void mpc85xx_pci_set_irq(void *opaque, int irq_num, int level) static int ppce500_pci_save(QEMUFile *f, void *opaque) { PPCE500PCIState *controller = opaque; - int i; + int i, ret; - pci_device_save(controller->pci_dev, f); + ret = pci_device_save(controller->pci_dev, f); + if (ret < 0) { + return ret; + } for (i = 0; i < PPCE500_PCI_NR_POBS; i++) { qemu_put_be32s(f, &controller->pob[i].potar); diff --git a/hw/unin_pci.c b/hw/unin_pci.c index ca15a00..28b9836 100644 --- a/hw/unin_pci.c +++ b/hw/unin_pci.c @@ -67,9 +67,7 @@ static int pci_unin_save(QEMUFile* f, void *opaque) { PCIDevice *d = opaque; - pci_device_save(d, f); - - return 0; + return pci_device_save(d, f); } static int pci_unin_load(QEMUFile* f, void *opaque, int version_id) ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 4/6] virtio: Allow virtio_save() errors 2010-10-06 20:58 [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson ` (2 preceding siblings ...) 2010-10-06 20:59 ` [Qemu-devel] [PATCH 3/6] pci: Allow pci_device_save() " Alex Williamson @ 2010-10-06 20:59 ` Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 5/6] savevm: Allow set_params and save_live_state to error Alex Williamson ` (4 subsequent siblings) 8 siblings, 0 replies; 26+ messages in thread From: Alex Williamson @ 2010-10-06 20:59 UTC (permalink / raw) To: qemu-devel; +Cc: alex.williamson, cam, kvm, quintela Carry pci_device_save() error through to virtio_save(). Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- hw/virtio-balloon.c | 6 +++++- hw/virtio-blk.c | 6 +++++- hw/virtio-net.c | 7 ++++++- hw/virtio-pci.c | 10 ++++++++-- hw/virtio-serial-bus.c | 6 +++++- hw/virtio.c | 14 ++++++++++---- hw/virtio.h | 4 ++-- 7 files changed, 41 insertions(+), 12 deletions(-) diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c index 719e72c..2bf009d 100644 --- a/hw/virtio-balloon.c +++ b/hw/virtio-balloon.c @@ -230,8 +230,12 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target, static int virtio_balloon_save(QEMUFile *f, void *opaque) { VirtIOBalloon *s = opaque; + int ret; - virtio_save(&s->vdev, f); + ret = virtio_save(&s->vdev, f); + if (ret < 0) { + return ret; + } qemu_put_be32(f, s->num_pages); qemu_put_be32(f, s->actual); diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c index 3770901..b4772bf 100644 --- a/hw/virtio-blk.c +++ b/hw/virtio-blk.c @@ -464,8 +464,12 @@ static int virtio_blk_save(QEMUFile *f, void *opaque) { VirtIOBlock *s = opaque; VirtIOBlockReq *req = s->rq; + int ret; - virtio_save(&s->vdev, f); + ret = virtio_save(&s->vdev, f); + if (ret < 0) { + return ret; + } while (req) { qemu_put_sbyte(f, 1); diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 1b683d9..6673320 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -782,6 +782,7 @@ static void virtio_net_tx_bh(void *opaque) static int virtio_net_save(QEMUFile *f, void *opaque) { VirtIONet *n = opaque; + int ret; if (n->vhost_started) { /* TODO: should we really stop the backend? @@ -789,7 +790,11 @@ static int virtio_net_save(QEMUFile *f, void *opaque) vhost_net_stop(tap_get_vhost_net(n->nic->nc.peer), &n->vdev); n->vhost_started = 0; } - virtio_save(&n->vdev, f); + + ret = virtio_save(&n->vdev, f); + if (ret < 0) { + return ret; + } qemu_put_buffer(f, n->mac, ETH_ALEN); qemu_put_be32(f, n->tx_waiting); diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 86e6b0a..a7603bb 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -121,13 +121,19 @@ static void virtio_pci_notify(void *opaque, uint16_t vector) qemu_set_irq(proxy->pci_dev.irq[0], proxy->vdev->isr & 1); } -static void virtio_pci_save_config(void * opaque, QEMUFile *f) +static int virtio_pci_save_config(void * opaque, QEMUFile *f) { VirtIOPCIProxy *proxy = opaque; - pci_device_save(&proxy->pci_dev, f); + int ret; + + ret = pci_device_save(&proxy->pci_dev, f); + if (ret < 0) { + return ret; + } msix_save(&proxy->pci_dev, f); if (msix_present(&proxy->pci_dev)) qemu_put_be16(f, proxy->vdev->config_vector); + return 0; } static void virtio_pci_save_queue(void * opaque, int n, QEMUFile *f) diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 7f00fcf..ca57dda 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -459,9 +459,13 @@ static int virtio_serial_save(QEMUFile *f, void *opaque) VirtIOSerialPort *port; uint32_t nr_active_ports; unsigned int i; + int ret; /* The virtio device */ - virtio_save(&s->vdev, f); + ret = virtio_save(&s->vdev, f); + if (ret < 0) { + return ret; + } /* The config space */ qemu_put_be16s(f, &s->config.cols); diff --git a/hw/virtio.c b/hw/virtio.c index fbef788..27b0e84 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -640,12 +640,16 @@ void virtio_notify_config(VirtIODevice *vdev) virtio_notify_vector(vdev, vdev->config_vector); } -void virtio_save(VirtIODevice *vdev, QEMUFile *f) +int virtio_save(VirtIODevice *vdev, QEMUFile *f) { - int i; + int i, ret; - if (vdev->binding->save_config) - vdev->binding->save_config(vdev->binding_opaque, f); + if (vdev->binding->save_config) { + ret = vdev->binding->save_config(vdev->binding_opaque, f); + if (ret < 0) { + return ret; + } + } qemu_put_8s(f, &vdev->status); qemu_put_8s(f, &vdev->isr); @@ -671,6 +675,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) if (vdev->binding->save_queue) vdev->binding->save_queue(vdev->binding_opaque, i, f); } + + return 0; } int virtio_load(VirtIODevice *vdev, QEMUFile *f) diff --git a/hw/virtio.h b/hw/virtio.h index 96514e6..5c5da3a 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -88,7 +88,7 @@ typedef struct VirtQueueElement typedef struct { void (*notify)(void * opaque, uint16_t vector); - void (*save_config)(void * opaque, QEMUFile *f); + int (*save_config)(void * opaque, QEMUFile *f); void (*save_queue)(void * opaque, int n, QEMUFile *f); int (*load_config)(void * opaque, QEMUFile *f); int (*load_queue)(void * opaque, int n, QEMUFile *f); @@ -150,7 +150,7 @@ int virtqueue_avail_bytes(VirtQueue *vq, int in_bytes, int out_bytes); void virtio_notify(VirtIODevice *vdev, VirtQueue *vq); -void virtio_save(VirtIODevice *vdev, QEMUFile *f); +int virtio_save(VirtIODevice *vdev, QEMUFile *f); int virtio_load(VirtIODevice *vdev, QEMUFile *f); ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 5/6] savevm: Allow set_params and save_live_state to error 2010-10-06 20:58 [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson ` (3 preceding siblings ...) 2010-10-06 20:59 ` [Qemu-devel] [PATCH 4/6] virtio: Allow virtio_save() errors Alex Williamson @ 2010-10-06 20:59 ` Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 6/6] savevm: Remove register_device_unmigratable() Alex Williamson ` (3 subsequent siblings) 8 siblings, 0 replies; 26+ messages in thread From: Alex Williamson @ 2010-10-06 20:59 UTC (permalink / raw) To: qemu-devel; +Cc: alex.williamson, cam, kvm, quintela This lets a save state handler NAK a migration or cancel if it runs into problems. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- block-migration.c | 4 +++- hw/hw.h | 2 +- savevm.c | 18 +++++++++++++++--- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/block-migration.c b/block-migration.c index 0bfdb73..5fb3b72 100644 --- a/block-migration.c +++ b/block-migration.c @@ -628,13 +628,15 @@ static int block_load(QEMUFile *f, void *opaque, int version_id) return 0; } -static void block_set_params(int blk_enable, int shared_base, void *opaque) +static int block_set_params(int blk_enable, int shared_base, void *opaque) { block_mig_state.blk_enable = blk_enable; block_mig_state.shared_base = shared_base; /* shared base means that blk_enable = 1 */ block_mig_state.blk_enable |= shared_base; + + return 0; } void blk_mig_init(void) diff --git a/hw/hw.h b/hw/hw.h index 91a60ca..95f2d52 100644 --- a/hw/hw.h +++ b/hw/hw.h @@ -239,7 +239,7 @@ static inline void qemu_get_sbe64s(QEMUFile *f, int64_t *pv) int64_t qemu_ftell(QEMUFile *f); int64_t qemu_fseek(QEMUFile *f, int64_t pos, int whence); -typedef void SaveSetParamsHandler(int blk_enable, int shared, void * opaque); +typedef int SaveSetParamsHandler(int blk_enable, int shared, void * opaque); typedef int SaveStateHandler(QEMUFile *f, void *opaque); typedef int SaveLiveStateHandler(Monitor *mon, QEMUFile *f, int stage, void *opaque); diff --git a/savevm.c b/savevm.c index 89c5fac..ad3ab86 100644 --- a/savevm.c +++ b/savevm.c @@ -1414,12 +1414,16 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable, int shared) { SaveStateEntry *se; + int ret; QTAILQ_FOREACH(se, &savevm_handlers, entry) { if(se->set_params == NULL) { continue; } - se->set_params(blk_enable, shared, se->opaque); + ret = se->set_params(blk_enable, shared, se->opaque); + if (ret < 0) { + return ret; + } } qemu_put_be32(f, QEMU_VM_FILE_MAGIC); @@ -1443,7 +1447,10 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable, qemu_put_be32(f, se->instance_id); qemu_put_be32(f, se->version_id); - se->save_live_state(mon, f, QEMU_VM_SECTION_START, se->opaque); + ret = se->save_live_state(mon, f, QEMU_VM_SECTION_START, se->opaque); + if (ret < 0) { + return ret; + } } if (qemu_file_has_error(f)) { @@ -1474,6 +1481,8 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f) and reduces the probability that a faster changing state is synchronized over and over again. */ break; + } else if (ret < 0) { + return ret; } } @@ -1503,7 +1512,10 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f) qemu_put_byte(f, QEMU_VM_SECTION_END); qemu_put_be32(f, se->section_id); - se->save_live_state(mon, f, QEMU_VM_SECTION_END, se->opaque); + r = se->save_live_state(mon, f, QEMU_VM_SECTION_END, se->opaque); + if (r < 0) { + return r; + } } QTAILQ_FOREACH(se, &savevm_handlers, entry) { ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] [PATCH 6/6] savevm: Remove register_device_unmigratable() 2010-10-06 20:58 [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson ` (4 preceding siblings ...) 2010-10-06 20:59 ` [Qemu-devel] [PATCH 5/6] savevm: Allow set_params and save_live_state to error Alex Williamson @ 2010-10-06 20:59 ` Alex Williamson 2010-10-07 16:55 ` [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson ` (2 subsequent siblings) 8 siblings, 0 replies; 26+ messages in thread From: Alex Williamson @ 2010-10-06 20:59 UTC (permalink / raw) To: qemu-devel; +Cc: alex.williamson, cam, kvm, quintela Now that the save state handlers can return error, individual drivers can cancel a migration if they hit an error or don't support it. This makes the unmigratable callback redundant. Remove it and change the only user to cancel the migration in a set_params callback, which actually happens much earlier in the migration than the unmigratable flag was checked. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- hw/hw.h | 2 -- hw/ivshmem.c | 20 ++++++++++++++------ savevm.c | 31 ------------------------------- 3 files changed, 14 insertions(+), 39 deletions(-) diff --git a/hw/hw.h b/hw/hw.h index 95f2d52..6c0aefe 100644 --- a/hw/hw.h +++ b/hw/hw.h @@ -264,8 +264,6 @@ int register_savevm_live(DeviceState *dev, void *opaque); void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque); -void register_device_unmigratable(DeviceState *dev, const char *idstr, - void *opaque); typedef void QEMUResetHandler(void *opaque); diff --git a/hw/ivshmem.c b/hw/ivshmem.c index 3726a7f..4164861 100644 --- a/hw/ivshmem.c +++ b/hw/ivshmem.c @@ -616,6 +616,18 @@ static void ivshmem_setup_msi(IVShmemState * s) { s->eventfd_table = qemu_mallocz(s->vectors * sizeof(EventfdEntry)); } +static int ivshmem_set_param(int blk_enable, int shared, void *opaque) +{ + IVShmemState *proxy = opaque; + + if (proxy->role_val == IVSHMEM_PEER) { + fprintf(stderr, + "ivshmem device in peer role, cannot be migrated or saved\n"); + return -EINVAL; + } + return 0; +} + static int ivshmem_save(QEMUFile* f, void *opaque) { IVShmemState *proxy = opaque; @@ -683,8 +695,8 @@ static int pci_ivshmem_init(PCIDevice *dev) s->ivshmem_size = ivshmem_get_size(s); } - register_savevm(&s->dev.qdev, "ivshmem", 0, 0, ivshmem_save, ivshmem_load, - dev); + register_savevm_live(&s->dev.qdev, "ivshmem", 0, 0, ivshmem_set_param, + NULL, ivshmem_save, ivshmem_load, dev); /* IRQFD requires MSI */ if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) && @@ -707,10 +719,6 @@ static int pci_ivshmem_init(PCIDevice *dev) s->role_val = IVSHMEM_MASTER; /* default */ } - if (s->role_val == IVSHMEM_PEER) { - register_device_unmigratable(&s->dev.qdev, "ivshmem", s); - } - pci_conf = s->dev.config; pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_REDHAT_QUMRANET); pci_conf[0x02] = 0x10; diff --git a/savevm.c b/savevm.c index ad3ab86..1b4ee08 100644 --- a/savevm.c +++ b/savevm.c @@ -1018,7 +1018,6 @@ typedef struct SaveStateEntry { const VMStateDescription *vmsd; void *opaque; CompatEntry *compat; - int no_migrate; } SaveStateEntry; @@ -1082,7 +1081,6 @@ int register_savevm_live(DeviceState *dev, se->load_state = load_state; se->opaque = opaque; se->vmsd = NULL; - se->no_migrate = 0; if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) { char *id = dev->parent_bus->info->get_dev_path(dev); @@ -1149,31 +1147,6 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque) } } -/* mark a device as not to be migrated, that is the device should be - unplugged before migration */ -void register_device_unmigratable(DeviceState *dev, const char *idstr, - void *opaque) -{ - SaveStateEntry *se; - char id[256] = ""; - - if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) { - char *path = dev->parent_bus->info->get_dev_path(dev); - if (path) { - pstrcpy(id, sizeof(id), path); - pstrcat(id, sizeof(id), "/"); - qemu_free(path); - } - } - pstrcat(id, sizeof(id), idstr); - - QTAILQ_FOREACH(se, &savevm_handlers, entry) { - if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) { - se->no_migrate = 1; - } - } -} - int vmstate_register_with_alias_id(DeviceState *dev, int instance_id, const VMStateDescription *vmsd, void *opaque, int alias_id, @@ -1389,10 +1362,6 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id) static int vmstate_save(QEMUFile *f, SaveStateEntry *se) { - if (se->no_migrate) { - return -1; - } - if (!se->vmsd) { /* Old style */ return se->save_state(f, se->opaque); } ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-10-06 20:58 [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson ` (5 preceding siblings ...) 2010-10-06 20:59 ` [Qemu-devel] [PATCH 6/6] savevm: Remove register_device_unmigratable() Alex Williamson @ 2010-10-07 16:55 ` Alex Williamson 2010-11-08 11:40 ` Michael S. Tsirkin 2010-11-16 10:23 ` Juan Quintela 8 siblings, 0 replies; 26+ messages in thread From: Alex Williamson @ 2010-10-07 16:55 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, cam, kvm, quintela [-- 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) ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-10-06 20:58 [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson ` (6 preceding siblings ...) 2010-10-07 16:55 ` [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson @ 2010-11-08 11:40 ` Michael S. Tsirkin 2010-11-08 14:59 ` Alex Williamson 2010-11-16 10:23 ` Juan Quintela 8 siblings, 1 reply; 26+ messages in thread From: Michael S. Tsirkin @ 2010-11-08 11:40 UTC (permalink / raw) To: Alex Williamson; +Cc: cam, qemu-devel, kvm, quintela On Wed, Oct 06, 2010 at 02:58:57PM -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 Well error handling is always tricky: it seems easier to require save handlers to never fail. So there's a bunch of code here but what exactly is the benefit? Since save handlers have no idea what does the remote do, what is the compatibility you mention? > --- > > 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(-) > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-08 11:40 ` Michael S. Tsirkin @ 2010-11-08 14:59 ` Alex Williamson 2010-11-08 16:54 ` Michael S. Tsirkin 0 siblings, 1 reply; 26+ messages in thread From: Alex Williamson @ 2010-11-08 14:59 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: cam, qemu-devel, kvm, quintela On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > Well error handling is always tricky: it seems easier to > require save handlers to never fail. Sure it's easier, but does that make it robust? > So there's a bunch of code here but what exactly is the benefit? > Since save handlers have no idea what does the remote do, > what is the compatibility you mention? There are two users I currently have in mind. ivshmem currently makes use of the register_device_unmigratable() because it makes use of host specific resources and connections (aiui). This sets the no_migrate flag, which is not dynamic and a bit of a band-aide. The other is device assignment, which needs a way to NAK a migration since physical devices are never migratable. I imagine we could at some point have devices with state tied to other features that can't always be detached from the host, this tries to provide the infrastructure for that to happen. Alex ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-08 14:59 ` Alex Williamson @ 2010-11-08 16:54 ` Michael S. Tsirkin 2010-11-08 17:20 ` Alex Williamson 0 siblings, 1 reply; 26+ messages in thread From: Michael S. Tsirkin @ 2010-11-08 16:54 UTC (permalink / raw) To: Alex Williamson; +Cc: cam, qemu-devel, kvm, quintela On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > > > Well error handling is always tricky: it seems easier to > > require save handlers to never fail. > > Sure it's easier, but does that make it robust? More robust in the face of wwhat kind of failure? > > So there's a bunch of code here but what exactly is the benefit? > > Since save handlers have no idea what does the remote do, > > what is the compatibility you mention? > > There are two users I currently have in mind. ivshmem currently makes > use of the register_device_unmigratable() because it makes use of host > specific resources and connections (aiui). This sets the no_migrate > flag, which is not dynamic and a bit of a band-aide. > The other is > device assignment, which needs a way to NAK a migration since physical > devices are never migratable. Well since all these can't be migrated ever, a fixed property actually seems a good match. Sure it's not dynamic but all the easier to debug. > I imagine we could at some point have > devices with state tied to other features that can't always be detached > from the host, this tries to provide the infrastructure for that to > happen. > > Alex Let guest control whether you can migrate? Sounds like something that is more likely to be abused than used constructively. -- MST ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-08 16:54 ` Michael S. Tsirkin @ 2010-11-08 17:20 ` Alex Williamson 2010-11-08 20:59 ` Michael S. Tsirkin 0 siblings, 1 reply; 26+ messages in thread From: Alex Williamson @ 2010-11-08 17:20 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: cam, qemu-devel, kvm, quintela On Mon, 2010-11-08 at 18:54 +0200, Michael S. Tsirkin wrote: > On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > > > > > Well error handling is always tricky: it seems easier to > > > require save handlers to never fail. > > > > Sure it's easier, but does that make it robust? > > More robust in the face of wwhat kind of failure? I really don't understand why we're having a discussion about whether providing a means to return an error is a good thing or not. These patches touch a lot of files, but the change is dead simple. > > > So there's a bunch of code here but what exactly is the benefit? > > > Since save handlers have no idea what does the remote do, > > > what is the compatibility you mention? > > > > There are two users I currently have in mind. ivshmem currently makes > > use of the register_device_unmigratable() because it makes use of host > > specific resources and connections (aiui). This sets the no_migrate > > flag, which is not dynamic and a bit of a band-aide. > > The other is > > device assignment, which needs a way to NAK a migration since physical > > devices are never migratable. > > Well since all these can't be migrated ever, a fixed property actually seems > a good match. Sure it's not dynamic but all the easier to debug. > > > I imagine we could at some point have > > devices with state tied to other features that can't always be detached > > from the host, this tries to provide the infrastructure for that to > > happen. > > > > Alex > > Let guest control whether you can migrate? > Sounds like something that is more likely to be abused > than used constructively. s/guest/device/ So you would rather the migration failed on the incoming side where it may not be detected or it may be detected too late to stop the migration? Alex ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-08 17:20 ` Alex Williamson @ 2010-11-08 20:59 ` Michael S. Tsirkin 2010-11-08 21:23 ` Alex Williamson 0 siblings, 1 reply; 26+ messages in thread From: Michael S. Tsirkin @ 2010-11-08 20:59 UTC (permalink / raw) To: Alex Williamson; +Cc: cam, qemu-devel, kvm, quintela On Mon, Nov 08, 2010 at 10:20:46AM -0700, Alex Williamson wrote: > On Mon, 2010-11-08 at 18:54 +0200, Michael S. Tsirkin wrote: > > On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > > > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > > > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > > > > > > > Well error handling is always tricky: it seems easier to > > > > require save handlers to never fail. > > > > > > Sure it's easier, but does that make it robust? > > > > More robust in the face of wwhat kind of failure? > > I really don't understand why we're having a discussion about whether > providing a means to return an error is a good thing or not. These > patches touch a lot of files, but the change is dead simple. I just don't see the motivation. Presumably your patches are there to achieve some kind of goal, right? I am trying to figure out what that goal is. Currently savevm callbacks never fail. So they return void. Why is returing 0 and adding a bunch of code to test the condition that never happens a good idea? It just seems to create more ways for devices to shoot themselves in the foot. > > > > So there's a bunch of code here but what exactly is the benefit? > > > > Since save handlers have no idea what does the remote do, > > > > what is the compatibility you mention? > > > > > > There are two users I currently have in mind. ivshmem currently makes > > > use of the register_device_unmigratable() because it makes use of host > > > specific resources and connections (aiui). This sets the no_migrate > > > flag, which is not dynamic and a bit of a band-aide. > > > The other is > > > device assignment, which needs a way to NAK a migration since physical > > > devices are never migratable. > > > > Well since all these can't be migrated ever, a fixed property actually seems > > a good match. Sure it's not dynamic but all the easier to debug. > > > > > I imagine we could at some point have > > > devices with state tied to other features that can't always be detached > > > from the host, this tries to provide the infrastructure for that to > > > happen. > > > > > > Alex > > > > Let guest control whether you can migrate? > > Sounds like something that is more likely to be abused > > than used constructively. > > s/guest/device/ So you would rather the migration failed on the > incoming side where it may not be detected And incoming migration handlers *must* validate the input, anyway. We should not plaster over this with checks on outgoing side. > or it may be detected too > late to stop the migration? > > Alex So there's a bug and device is in an unexpected state. What can we do? Assert, print an error, notify guest - all these come to mind. But stop migration? Seems arbitrary. -- MST ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-08 20:59 ` Michael S. Tsirkin @ 2010-11-08 21:23 ` Alex Williamson 2010-11-09 12:00 ` Michael S. Tsirkin 0 siblings, 1 reply; 26+ messages in thread From: Alex Williamson @ 2010-11-08 21:23 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: cam, qemu-devel, kvm, quintela On Mon, 2010-11-08 at 22:59 +0200, Michael S. Tsirkin wrote: > On Mon, Nov 08, 2010 at 10:20:46AM -0700, Alex Williamson wrote: > > On Mon, 2010-11-08 at 18:54 +0200, Michael S. Tsirkin wrote: > > > On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > > > > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > > > > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > > > > > > > > > Well error handling is always tricky: it seems easier to > > > > > require save handlers to never fail. > > > > > > > > Sure it's easier, but does that make it robust? > > > > > > More robust in the face of wwhat kind of failure? > > > > I really don't understand why we're having a discussion about whether > > providing a means to return an error is a good thing or not. These > > patches touch a lot of files, but the change is dead simple. > > I just don't see the motivation. Presumably your patches are > there to achieve some kind of goal, right? I am trying to > figure out what that goal is. My goal is that I want to be able to NAK a migration when devices are assigned, and I think we can do it more generically than the no_migrate flag so that it supports this application and any other reason that saves might fail in the future. > Currently savevm callbacks never fail. So they > return void. Why is returing 0 and adding a bunch of code to test the > condition that never happens a good idea? It just seems to create more > ways for devices to shoot themselves in the foot. And more ways to indicate something bad happened and keep running. We already have far too many abort() calls in the code. > > > > > So there's a bunch of code here but what exactly is the benefit? > > > > > Since save handlers have no idea what does the remote do, > > > > > what is the compatibility you mention? > > > > > > > > There are two users I currently have in mind. ivshmem currently makes > > > > use of the register_device_unmigratable() because it makes use of host > > > > specific resources and connections (aiui). This sets the no_migrate > > > > flag, which is not dynamic and a bit of a band-aide. > > > > The other is > > > > device assignment, which needs a way to NAK a migration since physical > > > > devices are never migratable. > > > > > > Well since all these can't be migrated ever, a fixed property actually seems > > > a good match. Sure it's not dynamic but all the easier to debug. > > > > > > > I imagine we could at some point have > > > > devices with state tied to other features that can't always be detached > > > > from the host, this tries to provide the infrastructure for that to > > > > happen. > > > > > > > > Alex > > > > > > Let guest control whether you can migrate? > > > Sounds like something that is more likely to be abused > > > than used constructively. > > > > s/guest/device/ So you would rather the migration failed on the > > incoming side where it may not be detected > > And incoming migration handlers *must* validate the input, anyway. > We should not plaster over this with checks on outgoing side. I'm not in any way suggesting incoming shouldn't do validation. > > or it may be detected too > > late to stop the migration? > > > > Alex > > So there's a bug and device is in an unexpected state. > What can we do? Assert, print an error, notify guest - all these > come to mind. But stop migration? Seems arbitrary. Perhaps the problem is that either an assert or an fprintf are the first things that come to mind. We shouldn't have guests randomly blowing up or telling users to go scan through their log files to find errors. It's not very hard to allow simple error handling, so why shouldn't our first plan of attack be to return an error so that the human/qmp monitor can detect it and inform the user. For the current candidates for this interface, there's no point notifying the guest, it's the interface attempting to do the migration that needs to know there's something blocking it. Alex ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-08 21:23 ` Alex Williamson @ 2010-11-09 12:00 ` Michael S. Tsirkin 2010-11-09 14:58 ` Alex Williamson 0 siblings, 1 reply; 26+ messages in thread From: Michael S. Tsirkin @ 2010-11-09 12:00 UTC (permalink / raw) To: Alex Williamson; +Cc: cam, qemu-devel, kvm, quintela On Mon, Nov 08, 2010 at 02:23:37PM -0700, Alex Williamson wrote: > On Mon, 2010-11-08 at 22:59 +0200, Michael S. Tsirkin wrote: > > On Mon, Nov 08, 2010 at 10:20:46AM -0700, Alex Williamson wrote: > > > On Mon, 2010-11-08 at 18:54 +0200, Michael S. Tsirkin wrote: > > > > On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > > > > > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > > > > > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > > > > > > > > > > > Well error handling is always tricky: it seems easier to > > > > > > require save handlers to never fail. > > > > > > > > > > Sure it's easier, but does that make it robust? > > > > > > > > More robust in the face of wwhat kind of failure? > > > > > > I really don't understand why we're having a discussion about whether > > > providing a means to return an error is a good thing or not. These > > > patches touch a lot of files, but the change is dead simple. > > > > I just don't see the motivation. Presumably your patches are > > there to achieve some kind of goal, right? I am trying to > > figure out what that goal is. > > My goal is that I want to be able to NAK a migration when devices are > assigned, and I think we can do it more generically than the no_migrate > flag so that it supports this application and any other reason that > saves might fail in the future. More generically but harder to understand and debug, IMO. > > Currently savevm callbacks never fail. So they > > return void. Why is returing 0 and adding a bunch of code to test the > > condition that never happens a good idea? It just seems to create more > > ways for devices to shoot themselves in the foot. > > And more ways to indicate something bad happened and keep running. We > already have far too many abort() calls in the code. If you can keep running why can't you migrate? > > > > > > So there's a bunch of code here but what exactly is the benefit? > > > > > > Since save handlers have no idea what does the remote do, > > > > > > what is the compatibility you mention? > > > > > > > > > > There are two users I currently have in mind. ivshmem currently makes > > > > > use of the register_device_unmigratable() because it makes use of host > > > > > specific resources and connections (aiui). This sets the no_migrate > > > > > flag, which is not dynamic and a bit of a band-aide. > > > > > The other is > > > > > device assignment, which needs a way to NAK a migration since physical > > > > > devices are never migratable. > > > > > > > > Well since all these can't be migrated ever, a fixed property actually seems > > > > a good match. Sure it's not dynamic but all the easier to debug. > > > > > > > > > I imagine we could at some point have > > > > > devices with state tied to other features that can't always be detached > > > > > from the host, this tries to provide the infrastructure for that to > > > > > happen. > > > > > > > > > > Alex > > > > > > > > Let guest control whether you can migrate? > > > > Sounds like something that is more likely to be abused > > > > than used constructively. > > > > > > s/guest/device/ So you would rather the migration failed on the > > > incoming side where it may not be detected > > > > And incoming migration handlers *must* validate the input, anyway. > > We should not plaster over this with checks on outgoing side. > > I'm not in any way suggesting incoming shouldn't do validation. So that's enough to detect the problem. > > > or it may be detected too > > > late to stop the migration? > > > > > > Alex > > > > So there's a bug and device is in an unexpected state. > > What can we do? Assert, print an error, notify guest - all these > > come to mind. But stop migration? Seems arbitrary. > > Perhaps the problem is that either an assert or an fprintf are the first > things that come to mind. We shouldn't have guests randomly blowing up > or telling users to go scan through their log files to find errors. > It's not very hard to allow simple error handling, so why shouldn't our > first plan of attack be to return an error so that the human/qmp monitor > can detect it and inform the user. For the current candidates for this > interface, there's no point notifying the guest, it's the interface > attempting to do the migration that needs to know there's something > blocking it. > > Alex I still don't understand, I am sorry. When will migration fail? Assigned devices always fail migration so it's not a good example. -- MST ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-09 12:00 ` Michael S. Tsirkin @ 2010-11-09 14:58 ` Alex Williamson 2010-11-09 15:07 ` Michael S. Tsirkin 0 siblings, 1 reply; 26+ messages in thread From: Alex Williamson @ 2010-11-09 14:58 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: cam, qemu-devel, kvm, quintela On Tue, 2010-11-09 at 14:00 +0200, Michael S. Tsirkin wrote: > On Mon, Nov 08, 2010 at 02:23:37PM -0700, Alex Williamson wrote: > > On Mon, 2010-11-08 at 22:59 +0200, Michael S. Tsirkin wrote: > > > On Mon, Nov 08, 2010 at 10:20:46AM -0700, Alex Williamson wrote: > > > > On Mon, 2010-11-08 at 18:54 +0200, Michael S. Tsirkin wrote: > > > > > On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > > > > > > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > > > > > > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > > > > > > > > > > > > > Well error handling is always tricky: it seems easier to > > > > > > > require save handlers to never fail. > > > > > > > > > > > > Sure it's easier, but does that make it robust? > > > > > > > > > > More robust in the face of wwhat kind of failure? > > > > > > > > I really don't understand why we're having a discussion about whether > > > > providing a means to return an error is a good thing or not. These > > > > patches touch a lot of files, but the change is dead simple. > > > > > > I just don't see the motivation. Presumably your patches are > > > there to achieve some kind of goal, right? I am trying to > > > figure out what that goal is. > > > > My goal is that I want to be able to NAK a migration when devices are > > assigned, and I think we can do it more generically than the no_migrate > > flag so that it supports this application and any other reason that > > saves might fail in the future. > > More generically but harder to understand and debug, IMO. How is returning an error condition hard to understand? Debugging seems easier to me, especially if drivers follow the precedent set in the last patch and fprintf the reason for the failure. Ideally this would be some kind of push out to qmp, but it still seems easier than figuring out which driver called register_device_unmigratable(). > > > Currently savevm callbacks never fail. So they > > > return void. Why is returing 0 and adding a bunch of code to test the > > > condition that never happens a good idea? It just seems to create more > > > ways for devices to shoot themselves in the foot. > > > > And more ways to indicate something bad happened and keep running. We > > already have far too many abort() calls in the code. > > If you can keep running why can't you migrate? Well, as you know device assignment is tied to the hardware, so can't migrate, but can always keep running. The ivshmem driver has a peer role, where it's tied to the host memory, so can't migrate, but can keep running. > > > > > > > So there's a bunch of code here but what exactly is the benefit? > > > > > > > Since save handlers have no idea what does the remote do, > > > > > > > what is the compatibility you mention? > > > > > > > > > > > > There are two users I currently have in mind. ivshmem currently makes > > > > > > use of the register_device_unmigratable() because it makes use of host > > > > > > specific resources and connections (aiui). This sets the no_migrate > > > > > > flag, which is not dynamic and a bit of a band-aide. > > > > > > The other is > > > > > > device assignment, which needs a way to NAK a migration since physical > > > > > > devices are never migratable. > > > > > > > > > > Well since all these can't be migrated ever, a fixed property actually seems > > > > > a good match. Sure it's not dynamic but all the easier to debug. > > > > > > > > > > > I imagine we could at some point have > > > > > > devices with state tied to other features that can't always be detached > > > > > > from the host, this tries to provide the infrastructure for that to > > > > > > happen. > > > > > > > > > > > > Alex > > > > > > > > > > Let guest control whether you can migrate? > > > > > Sounds like something that is more likely to be abused > > > > > than used constructively. > > > > > > > > s/guest/device/ So you would rather the migration failed on the > > > > incoming side where it may not be detected > > > > > > And incoming migration handlers *must* validate the input, anyway. > > > We should not plaster over this with checks on outgoing side. > > > > I'm not in any way suggesting incoming shouldn't do validation. > > So that's enough to detect the problem. No. Let's say I have a migration source with an assigned device (rombar=0 to even avoid ramblock migration issues), the migration target is identical except it doesn't include the assigned device. pci-assign on the source can't NAK a migration because save doesn't currently allow error returns. The target doesn't even have the driver loaded, so there's nothing to NAK the load... migration happens and the device disappeared, wait for crash. Maybe we could assume that the user did something sane and used pci-assign on the target to match the source, then we could NAK the load, but only after we wait for the entire memory state of the guest to be transferred. > > > > or it may be detected too > > > > late to stop the migration? > > > > > > > > Alex > > > > > > So there's a bug and device is in an unexpected state. > > > What can we do? Assert, print an error, notify guest - all these > > > come to mind. But stop migration? Seems arbitrary. > > > > Perhaps the problem is that either an assert or an fprintf are the first > > things that come to mind. We shouldn't have guests randomly blowing up > > or telling users to go scan through their log files to find errors. > > It's not very hard to allow simple error handling, so why shouldn't our > > first plan of attack be to return an error so that the human/qmp monitor > > can detect it and inform the user. For the current candidates for this > > interface, there's no point notifying the guest, it's the interface > > attempting to do the migration that needs to know there's something > > blocking it. > > > > Alex > > I still don't understand, I am sorry. When will migration fail? > Assigned devices always fail migration so it's not a good example. Seems like the perfect example, especially in the scenario above where load failure is insufficient. This is why the no_migrate flag was introduced and why ivshmem makes use of it today. This series starts from the assumption that we need a way to NAK a migration, can we do it better than the no_migrate flag, generically, and as early as possible in the process. Alex ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-09 14:58 ` Alex Williamson @ 2010-11-09 15:07 ` Michael S. Tsirkin 2010-11-09 15:34 ` Alex Williamson 0 siblings, 1 reply; 26+ messages in thread From: Michael S. Tsirkin @ 2010-11-09 15:07 UTC (permalink / raw) To: Alex Williamson; +Cc: cam, qemu-devel, kvm, quintela On Tue, Nov 09, 2010 at 07:58:23AM -0700, Alex Williamson wrote: > On Tue, 2010-11-09 at 14:00 +0200, Michael S. Tsirkin wrote: > > On Mon, Nov 08, 2010 at 02:23:37PM -0700, Alex Williamson wrote: > > > On Mon, 2010-11-08 at 22:59 +0200, Michael S. Tsirkin wrote: > > > > On Mon, Nov 08, 2010 at 10:20:46AM -0700, Alex Williamson wrote: > > > > > On Mon, 2010-11-08 at 18:54 +0200, Michael S. Tsirkin wrote: > > > > > > On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > > > > > > > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > > > > > > > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > > > > > > > > > > > > > > > Well error handling is always tricky: it seems easier to > > > > > > > > require save handlers to never fail. > > > > > > > > > > > > > > Sure it's easier, but does that make it robust? > > > > > > > > > > > > More robust in the face of wwhat kind of failure? > > > > > > > > > > I really don't understand why we're having a discussion about whether > > > > > providing a means to return an error is a good thing or not. These > > > > > patches touch a lot of files, but the change is dead simple. > > > > > > > > I just don't see the motivation. Presumably your patches are > > > > there to achieve some kind of goal, right? I am trying to > > > > figure out what that goal is. > > > > > > My goal is that I want to be able to NAK a migration when devices are > > > assigned, and I think we can do it more generically than the no_migrate > > > flag so that it supports this application and any other reason that > > > saves might fail in the future. > > > > More generically but harder to understand and debug, IMO. > > How is returning an error condition hard to understand? Debugging seems > easier to me, especially if drivers follow the precedent set in the last > patch and fprintf the reason for the failure. Ideally this would be > some kind of push out to qmp, but it still seems easier than figuring > out which driver called register_device_unmigratable(). > > > > > Currently savevm callbacks never fail. So they > > > > return void. Why is returing 0 and adding a bunch of code to test the > > > > condition that never happens a good idea? It just seems to create more > > > > ways for devices to shoot themselves in the foot. > > > > > > And more ways to indicate something bad happened and keep running. We > > > already have far too many abort() calls in the code. > > > > If you can keep running why can't you migrate? > > Well, as you know device assignment is tied to the hardware, so can't > migrate, but can always keep running. The ivshmem driver has a peer > role, where it's tied to the host memory, so can't migrate, but can keep > running. Right. All these are covered with no_migrate flag well enough. Their inability to migrate does not change at runtime. > > > > > > > > So there's a bunch of code here but what exactly is the benefit? > > > > > > > > Since save handlers have no idea what does the remote do, > > > > > > > > what is the compatibility you mention? > > > > > > > > > > > > > > There are two users I currently have in mind. ivshmem currently makes > > > > > > > use of the register_device_unmigratable() because it makes use of host > > > > > > > specific resources and connections (aiui). This sets the no_migrate > > > > > > > flag, which is not dynamic and a bit of a band-aide. > > > > > > > The other is > > > > > > > device assignment, which needs a way to NAK a migration since physical > > > > > > > devices are never migratable. > > > > > > > > > > > > Well since all these can't be migrated ever, a fixed property actually seems > > > > > > a good match. Sure it's not dynamic but all the easier to debug. > > > > > > > > > > > > > I imagine we could at some point have > > > > > > > devices with state tied to other features that can't always be detached > > > > > > > from the host, this tries to provide the infrastructure for that to > > > > > > > happen. > > > > > > > > > > > > > > Alex > > > > > > > > > > > > Let guest control whether you can migrate? > > > > > > Sounds like something that is more likely to be abused > > > > > > than used constructively. > > > > > > > > > > s/guest/device/ So you would rather the migration failed on the > > > > > incoming side where it may not be detected > > > > > > > > And incoming migration handlers *must* validate the input, anyway. > > > > We should not plaster over this with checks on outgoing side. > > > > > > I'm not in any way suggesting incoming shouldn't do validation. > > > > So that's enough to detect the problem. > > No. Let's say I have a migration source with an assigned device > (rombar=0 to even avoid ramblock migration issues), the migration target > is identical except it doesn't include the assigned device. pci-assign > on the source can't NAK a migration because save doesn't currently allow > error returns. The target doesn't even have the driver loaded, so > there's nothing to NAK the load... migration happens and the device > disappeared, wait for crash. Maybe we could assume that the user did > something sane and used pci-assign on the target to match the source, > then we could NAK the load, but only after we wait for the entire memory > state of the guest to be transferred. So set no_migrate flag and that should be enough. > > > > > or it may be detected too > > > > > late to stop the migration? > > > > > > > > > > Alex > > > > > > > > So there's a bug and device is in an unexpected state. > > > > What can we do? Assert, print an error, notify guest - all these > > > > come to mind. But stop migration? Seems arbitrary. > > > > > > Perhaps the problem is that either an assert or an fprintf are the first > > > things that come to mind. We shouldn't have guests randomly blowing up > > > or telling users to go scan through their log files to find errors. > > > It's not very hard to allow simple error handling, so why shouldn't our > > > first plan of attack be to return an error so that the human/qmp monitor > > > can detect it and inform the user. For the current candidates for this > > > interface, there's no point notifying the guest, it's the interface > > > attempting to do the migration that needs to know there's something > > > blocking it. > > > > > > Alex > > > > I still don't understand, I am sorry. When will migration fail? > > Assigned devices always fail migration so it's not a good example. > > Seems like the perfect example, especially in the scenario above where > load failure is insufficient. This is why the no_migrate flag was > introduced and why ivshmem makes use of it today. This series starts > from the assumption that we need a way to NAK a migration, can we do it > better than the no_migrate flag, generically, and as early as possible > in the process. > > Alex no_migrate seems better in that we can check it at any point, unlike tying it to save callback which can only be invoked with VM stopped. ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-09 15:07 ` Michael S. Tsirkin @ 2010-11-09 15:34 ` Alex Williamson 2010-11-09 15:42 ` Michael S. Tsirkin 0 siblings, 1 reply; 26+ messages in thread From: Alex Williamson @ 2010-11-09 15:34 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: cam, qemu-devel, kvm, quintela On Tue, 2010-11-09 at 17:07 +0200, Michael S. Tsirkin wrote: > On Tue, Nov 09, 2010 at 07:58:23AM -0700, Alex Williamson wrote: > > On Tue, 2010-11-09 at 14:00 +0200, Michael S. Tsirkin wrote: > > > On Mon, Nov 08, 2010 at 02:23:37PM -0700, Alex Williamson wrote: > > > > On Mon, 2010-11-08 at 22:59 +0200, Michael S. Tsirkin wrote: > > > > > On Mon, Nov 08, 2010 at 10:20:46AM -0700, Alex Williamson wrote: > > > > > > On Mon, 2010-11-08 at 18:54 +0200, Michael S. Tsirkin wrote: > > > > > > > On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > > > > > > > > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > > > > > > > > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > > > > > > > > > > > > > > > > > Well error handling is always tricky: it seems easier to > > > > > > > > > require save handlers to never fail. > > > > > > > > > > > > > > > > Sure it's easier, but does that make it robust? > > > > > > > > > > > > > > More robust in the face of wwhat kind of failure? > > > > > > > > > > > > I really don't understand why we're having a discussion about whether > > > > > > providing a means to return an error is a good thing or not. These > > > > > > patches touch a lot of files, but the change is dead simple. > > > > > > > > > > I just don't see the motivation. Presumably your patches are > > > > > there to achieve some kind of goal, right? I am trying to > > > > > figure out what that goal is. > > > > > > > > My goal is that I want to be able to NAK a migration when devices are > > > > assigned, and I think we can do it more generically than the no_migrate > > > > flag so that it supports this application and any other reason that > > > > saves might fail in the future. > > > > > > More generically but harder to understand and debug, IMO. > > > > How is returning an error condition hard to understand? Debugging seems > > easier to me, especially if drivers follow the precedent set in the last > > patch and fprintf the reason for the failure. Ideally this would be > > some kind of push out to qmp, but it still seems easier than figuring > > out which driver called register_device_unmigratable(). > > > > > > > Currently savevm callbacks never fail. So they > > > > > return void. Why is returing 0 and adding a bunch of code to test the > > > > > condition that never happens a good idea? It just seems to create more > > > > > ways for devices to shoot themselves in the foot. > > > > > > > > And more ways to indicate something bad happened and keep running. We > > > > already have far too many abort() calls in the code. > > > > > > If you can keep running why can't you migrate? > > > > Well, as you know device assignment is tied to the hardware, so can't > > migrate, but can always keep running. The ivshmem driver has a peer > > role, where it's tied to the host memory, so can't migrate, but can keep > > running. > > Right. All these are covered with no_migrate flag well enough. > Their inability to migrate does not change at runtime. But it could. What if ivshmem is acting in a peer role, but has no clients, could it migrate? What if ivshmem is migratable when the migration begins, but while the migration continues, a connection is setup and it becomes unmigratable. Using this series, ivshmem would have multiple options how to support this. It could a) NAK the migration, b) drop connections and prevent new connections until the migration finishes, c) detect that new connections have happened since the migration started and cancel. And probably more. no_migrate can only do a). And in fact, we can only test no_migrate after the VM is stopped (after all memory is migrated) because otherwise it could race with devices setting no_migrate during migration. > > > > > > > > > So there's a bunch of code here but what exactly is the benefit? > > > > > > > > > Since save handlers have no idea what does the remote do, > > > > > > > > > what is the compatibility you mention? > > > > > > > > > > > > > > > > There are two users I currently have in mind. ivshmem currently makes > > > > > > > > use of the register_device_unmigratable() because it makes use of host > > > > > > > > specific resources and connections (aiui). This sets the no_migrate > > > > > > > > flag, which is not dynamic and a bit of a band-aide. > > > > > > > > The other is > > > > > > > > device assignment, which needs a way to NAK a migration since physical > > > > > > > > devices are never migratable. > > > > > > > > > > > > > > Well since all these can't be migrated ever, a fixed property actually seems > > > > > > > a good match. Sure it's not dynamic but all the easier to debug. > > > > > > > > > > > > > > > I imagine we could at some point have > > > > > > > > devices with state tied to other features that can't always be detached > > > > > > > > from the host, this tries to provide the infrastructure for that to > > > > > > > > happen. > > > > > > > > > > > > > > > > Alex > > > > > > > > > > > > > > Let guest control whether you can migrate? > > > > > > > Sounds like something that is more likely to be abused > > > > > > > than used constructively. > > > > > > > > > > > > s/guest/device/ So you would rather the migration failed on the > > > > > > incoming side where it may not be detected > > > > > > > > > > And incoming migration handlers *must* validate the input, anyway. > > > > > We should not plaster over this with checks on outgoing side. > > > > > > > > I'm not in any way suggesting incoming shouldn't do validation. > > > > > > So that's enough to detect the problem. > > > > No. Let's say I have a migration source with an assigned device > > (rombar=0 to even avoid ramblock migration issues), the migration target > > is identical except it doesn't include the assigned device. pci-assign > > on the source can't NAK a migration because save doesn't currently allow > > error returns. The target doesn't even have the driver loaded, so > > there's nothing to NAK the load... migration happens and the device > > disappeared, wait for crash. Maybe we could assume that the user did > > something sane and used pci-assign on the target to match the source, > > then we could NAK the load, but only after we wait for the entire memory > > state of the guest to be transferred. > > So set no_migrate flag and that should be enough. no_migrate is sufficient for today's usage, but undesirable IMO. > > > > > > or it may be detected too > > > > > > late to stop the migration? > > > > > > > > > > > > Alex > > > > > > > > > > So there's a bug and device is in an unexpected state. > > > > > What can we do? Assert, print an error, notify guest - all these > > > > > come to mind. But stop migration? Seems arbitrary. > > > > > > > > Perhaps the problem is that either an assert or an fprintf are the first > > > > things that come to mind. We shouldn't have guests randomly blowing up > > > > or telling users to go scan through their log files to find errors. > > > > It's not very hard to allow simple error handling, so why shouldn't our > > > > first plan of attack be to return an error so that the human/qmp monitor > > > > can detect it and inform the user. For the current candidates for this > > > > interface, there's no point notifying the guest, it's the interface > > > > attempting to do the migration that needs to know there's something > > > > blocking it. > > > > > > > > Alex > > > > > > I still don't understand, I am sorry. When will migration fail? > > > Assigned devices always fail migration so it's not a good example. > > > > Seems like the perfect example, especially in the scenario above where > > load failure is insufficient. This is why the no_migrate flag was > > introduced and why ivshmem makes use of it today. This series starts > > from the assumption that we need a way to NAK a migration, can we do it > > better than the no_migrate flag, generically, and as early as possible > > in the process. > > > > Alex > > no_migrate seems better in that we can check it at any point, > unlike tying it to save callback which can only be invoked > with VM stopped. Please see the patches, I switched ivshmem over to NAK set_params, which happens before the VM is stopped and before memory is migrated. An interface could be added to call this at any point, it's dynamic, and it keeps all of the tests for when and why a driver might NAK in the driver itself. Alex ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-09 15:34 ` Alex Williamson @ 2010-11-09 15:42 ` Michael S. Tsirkin 2010-11-09 15:47 ` Alex Williamson 0 siblings, 1 reply; 26+ messages in thread From: Michael S. Tsirkin @ 2010-11-09 15:42 UTC (permalink / raw) To: Alex Williamson; +Cc: cam, qemu-devel, kvm, quintela On Tue, Nov 09, 2010 at 08:34:54AM -0700, Alex Williamson wrote: > On Tue, 2010-11-09 at 17:07 +0200, Michael S. Tsirkin wrote: > > On Tue, Nov 09, 2010 at 07:58:23AM -0700, Alex Williamson wrote: > > > On Tue, 2010-11-09 at 14:00 +0200, Michael S. Tsirkin wrote: > > > > On Mon, Nov 08, 2010 at 02:23:37PM -0700, Alex Williamson wrote: > > > > > On Mon, 2010-11-08 at 22:59 +0200, Michael S. Tsirkin wrote: > > > > > > On Mon, Nov 08, 2010 at 10:20:46AM -0700, Alex Williamson wrote: > > > > > > > On Mon, 2010-11-08 at 18:54 +0200, Michael S. Tsirkin wrote: > > > > > > > > On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > > > > > > > > > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > > > > > > > > > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > > > > > > > > > > > > > > > > > > > Well error handling is always tricky: it seems easier to > > > > > > > > > > require save handlers to never fail. > > > > > > > > > > > > > > > > > > Sure it's easier, but does that make it robust? > > > > > > > > > > > > > > > > More robust in the face of wwhat kind of failure? > > > > > > > > > > > > > > I really don't understand why we're having a discussion about whether > > > > > > > providing a means to return an error is a good thing or not. These > > > > > > > patches touch a lot of files, but the change is dead simple. > > > > > > > > > > > > I just don't see the motivation. Presumably your patches are > > > > > > there to achieve some kind of goal, right? I am trying to > > > > > > figure out what that goal is. > > > > > > > > > > My goal is that I want to be able to NAK a migration when devices are > > > > > assigned, and I think we can do it more generically than the no_migrate > > > > > flag so that it supports this application and any other reason that > > > > > saves might fail in the future. > > > > > > > > More generically but harder to understand and debug, IMO. > > > > > > How is returning an error condition hard to understand? Debugging seems > > > easier to me, especially if drivers follow the precedent set in the last > > > patch and fprintf the reason for the failure. Ideally this would be > > > some kind of push out to qmp, but it still seems easier than figuring > > > out which driver called register_device_unmigratable(). > > > > > > > > > Currently savevm callbacks never fail. So they > > > > > > return void. Why is returing 0 and adding a bunch of code to test the > > > > > > condition that never happens a good idea? It just seems to create more > > > > > > ways for devices to shoot themselves in the foot. > > > > > > > > > > And more ways to indicate something bad happened and keep running. We > > > > > already have far too many abort() calls in the code. > > > > > > > > If you can keep running why can't you migrate? > > > > > > Well, as you know device assignment is tied to the hardware, so can't > > > migrate, but can always keep running. The ivshmem driver has a peer > > > role, where it's tied to the host memory, so can't migrate, but can keep > > > running. > > > > Right. All these are covered with no_migrate flag well enough. > > Their inability to migrate does not change at runtime. > > But it could. What if ivshmem is acting in a peer role, but has no > clients, could it migrate? What if ivshmem is migratable when the > migration begins, but while the migration continues, a connection is > setup and it becomes unmigratable. Sounds like something we should work to prevent, not support :) > Using this series, ivshmem would > have multiple options how to support this. It could a) NAK the > migration, b) drop connections and prevent new connections until the > migration finishes, c) detect that new connections have happened since > the migration started and cancel. And probably more. no_migrate can > only do a). And in fact, we can only test no_migrate after the VM is > stopped (after all memory is migrated) because otherwise it could race > with devices setting no_migrate during migration. We really want no_migrate to be static. changing it is abusing the infrastructure. -- MST ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-09 15:42 ` Michael S. Tsirkin @ 2010-11-09 15:47 ` Alex Williamson 2010-11-09 16:15 ` Michael S. Tsirkin 0 siblings, 1 reply; 26+ messages in thread From: Alex Williamson @ 2010-11-09 15:47 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: cam, qemu-devel, kvm, quintela On Tue, 2010-11-09 at 17:42 +0200, Michael S. Tsirkin wrote: > On Tue, Nov 09, 2010 at 08:34:54AM -0700, Alex Williamson wrote: > > On Tue, 2010-11-09 at 17:07 +0200, Michael S. Tsirkin wrote: > > > On Tue, Nov 09, 2010 at 07:58:23AM -0700, Alex Williamson wrote: > > > > On Tue, 2010-11-09 at 14:00 +0200, Michael S. Tsirkin wrote: > > > > > On Mon, Nov 08, 2010 at 02:23:37PM -0700, Alex Williamson wrote: > > > > > > On Mon, 2010-11-08 at 22:59 +0200, Michael S. Tsirkin wrote: > > > > > > > On Mon, Nov 08, 2010 at 10:20:46AM -0700, Alex Williamson wrote: > > > > > > > > On Mon, 2010-11-08 at 18:54 +0200, Michael S. Tsirkin wrote: > > > > > > > > > On Mon, Nov 08, 2010 at 07:59:57AM -0700, Alex Williamson wrote: > > > > > > > > > > On Mon, 2010-11-08 at 13:40 +0200, Michael S. Tsirkin wrote: > > > > > > > > > > > On Wed, Oct 06, 2010 at 02:58:57PM -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 > > > > > > > > > > > > > > > > > > > > > > Well error handling is always tricky: it seems easier to > > > > > > > > > > > require save handlers to never fail. > > > > > > > > > > > > > > > > > > > > Sure it's easier, but does that make it robust? > > > > > > > > > > > > > > > > > > More robust in the face of wwhat kind of failure? > > > > > > > > > > > > > > > > I really don't understand why we're having a discussion about whether > > > > > > > > providing a means to return an error is a good thing or not. These > > > > > > > > patches touch a lot of files, but the change is dead simple. > > > > > > > > > > > > > > I just don't see the motivation. Presumably your patches are > > > > > > > there to achieve some kind of goal, right? I am trying to > > > > > > > figure out what that goal is. > > > > > > > > > > > > My goal is that I want to be able to NAK a migration when devices are > > > > > > assigned, and I think we can do it more generically than the no_migrate > > > > > > flag so that it supports this application and any other reason that > > > > > > saves might fail in the future. > > > > > > > > > > More generically but harder to understand and debug, IMO. > > > > > > > > How is returning an error condition hard to understand? Debugging seems > > > > easier to me, especially if drivers follow the precedent set in the last > > > > patch and fprintf the reason for the failure. Ideally this would be > > > > some kind of push out to qmp, but it still seems easier than figuring > > > > out which driver called register_device_unmigratable(). > > > > > > > > > > > Currently savevm callbacks never fail. So they > > > > > > > return void. Why is returing 0 and adding a bunch of code to test the > > > > > > > condition that never happens a good idea? It just seems to create more > > > > > > > ways for devices to shoot themselves in the foot. > > > > > > > > > > > > And more ways to indicate something bad happened and keep running. We > > > > > > already have far too many abort() calls in the code. > > > > > > > > > > If you can keep running why can't you migrate? > > > > > > > > Well, as you know device assignment is tied to the hardware, so can't > > > > migrate, but can always keep running. The ivshmem driver has a peer > > > > role, where it's tied to the host memory, so can't migrate, but can keep > > > > running. > > > > > > Right. All these are covered with no_migrate flag well enough. > > > Their inability to migrate does not change at runtime. > > > > But it could. What if ivshmem is acting in a peer role, but has no > > clients, could it migrate? What if ivshmem is migratable when the > > migration begins, but while the migration continues, a connection is > > setup and it becomes unmigratable. > > Sounds like something we should work to prevent, not support :) s/:)/:(/ why? > > Using this series, ivshmem would > > have multiple options how to support this. It could a) NAK the > > migration, b) drop connections and prevent new connections until the > > migration finishes, c) detect that new connections have happened since > > the migration started and cancel. And probably more. no_migrate can > > only do a). And in fact, we can only test no_migrate after the VM is > > stopped (after all memory is migrated) because otherwise it could race > > with devices setting no_migrate during migration. > > We really want no_migrate to be static. changing it is abusing > the infrastructure. You call it abusing, I call it making use of the infrastructure. Why unnecessarily restrict ourselves? Is return 0/-1 really that scary, unmaintainable, undebuggable? I don't understand the resistance. Alex ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-09 15:47 ` Alex Williamson @ 2010-11-09 16:15 ` Michael S. Tsirkin 2010-11-09 16:30 ` Alex Williamson 0 siblings, 1 reply; 26+ messages in thread From: Michael S. Tsirkin @ 2010-11-09 16:15 UTC (permalink / raw) To: Alex Williamson; +Cc: cam, qemu-devel, kvm, quintela On Tue, Nov 09, 2010 at 08:47:00AM -0700, Alex Williamson wrote: > > > But it could. What if ivshmem is acting in a peer role, but has no > > > clients, could it migrate? What if ivshmem is migratable when the > > > migration begins, but while the migration continues, a connection is > > > setup and it becomes unmigratable. > > > > Sounds like something we should work to prevent, not support :) > > s/:)/:(/ why? It will just confuse everyone. Also if it happens after sending all of memory, it's pretty painful. > > > Using this series, ivshmem would > > > have multiple options how to support this. It could a) NAK the > > > migration, b) drop connections and prevent new connections until the > > > migration finishes, c) detect that new connections have happened since > > > the migration started and cancel. And probably more. no_migrate can > > > only do a). And in fact, we can only test no_migrate after the VM is > > > stopped (after all memory is migrated) because otherwise it could race > > > with devices setting no_migrate during migration. > > > > We really want no_migrate to be static. changing it is abusing > > the infrastructure. > > You call it abusing, I call it making use of the infrastructure. Why > unnecessarily restrict ourselves? Is return 0/-1 really that scary, > unmaintainable, undebuggable? I don't understand the resistance. > > Alex management really does not know how to handle unexpected migration failures. They must be avoided. There are some very special cases that fail migration. They are currently easy to find with grep register_device_unmigratable. I prefer to keep it that way. -- MST ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-09 16:15 ` Michael S. Tsirkin @ 2010-11-09 16:30 ` Alex Williamson 2010-11-09 16:49 ` Michael S. Tsirkin 0 siblings, 1 reply; 26+ messages in thread From: Alex Williamson @ 2010-11-09 16:30 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: cam, qemu-devel, kvm, quintela On Tue, 2010-11-09 at 18:15 +0200, Michael S. Tsirkin wrote: > On Tue, Nov 09, 2010 at 08:47:00AM -0700, Alex Williamson wrote: > > > > But it could. What if ivshmem is acting in a peer role, but has no > > > > clients, could it migrate? What if ivshmem is migratable when the > > > > migration begins, but while the migration continues, a connection is > > > > setup and it becomes unmigratable. > > > > > > Sounds like something we should work to prevent, not support :) > > > > s/:)/:(/ why? > > It will just confuse everyone. Also if it happens after sending > all of memory, it's pretty painful. It happens after sending all of memory with no_migrate, and I think pushing that earlier might introduce some races around when register_device_unmigratable() can be called. > > > > Using this series, ivshmem would > > > > have multiple options how to support this. It could a) NAK the > > > > migration, b) drop connections and prevent new connections until the > > > > migration finishes, c) detect that new connections have happened since > > > > the migration started and cancel. And probably more. no_migrate can > > > > only do a). And in fact, we can only test no_migrate after the VM is > > > > stopped (after all memory is migrated) because otherwise it could race > > > > with devices setting no_migrate during migration. > > > > > > We really want no_migrate to be static. changing it is abusing > > > the infrastructure. > > > > You call it abusing, I call it making use of the infrastructure. Why > > unnecessarily restrict ourselves? Is return 0/-1 really that scary, > > unmaintainable, undebuggable? I don't understand the resistance. > > > > Alex > > management really does not know how to handle unexpected > migration failures. They must be avoided. > > There are some very special cases that fail migration. They are > currently easy to find with grep register_device_unmigratable. > I prefer to keep it that way. How can management tools be improved to better handle unexpected migration failures when the only way for qemu to fail is an abort? We need the infrastructure to at least return an error first. Do we just need to add some fprintfs to the save core to print the id string of the device that failed to save? I just can't buy the "code is easier to grep" as an argument against adding better error handling to the save code path. Anyone else want to chime in? Alex ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-09 16:30 ` Alex Williamson @ 2010-11-09 16:49 ` Michael S. Tsirkin 2010-11-09 17:44 ` Alex Williamson 0 siblings, 1 reply; 26+ messages in thread From: Michael S. Tsirkin @ 2010-11-09 16:49 UTC (permalink / raw) To: Alex Williamson; +Cc: cam, qemu-devel, kvm, quintela On Tue, Nov 09, 2010 at 09:30:45AM -0700, Alex Williamson wrote: > On Tue, 2010-11-09 at 18:15 +0200, Michael S. Tsirkin wrote: > > On Tue, Nov 09, 2010 at 08:47:00AM -0700, Alex Williamson wrote: > > > > > But it could. What if ivshmem is acting in a peer role, but has no > > > > > clients, could it migrate? What if ivshmem is migratable when the > > > > > migration begins, but while the migration continues, a connection is > > > > > setup and it becomes unmigratable. > > > > > > > > Sounds like something we should work to prevent, not support :) > > > > > > s/:)/:(/ why? > > > > It will just confuse everyone. Also if it happens after sending > > all of memory, it's pretty painful. > > It happens after sending all of memory with no_migrate, and I think > pushing that earlier might introduce some races around when > register_device_unmigratable() can be called. Good point. I guess we could check it twice just to speed things up. > > > > > Using this series, ivshmem would > > > > > have multiple options how to support this. It could a) NAK the > > > > > migration, b) drop connections and prevent new connections until the > > > > > migration finishes, c) detect that new connections have happened since > > > > > the migration started and cancel. And probably more. no_migrate can > > > > > only do a). And in fact, we can only test no_migrate after the VM is > > > > > stopped (after all memory is migrated) because otherwise it could race > > > > > with devices setting no_migrate during migration. > > > > > > > > We really want no_migrate to be static. changing it is abusing > > > > the infrastructure. > > > > > > You call it abusing, I call it making use of the infrastructure. Why > > > unnecessarily restrict ourselves? Is return 0/-1 really that scary, > > > unmaintainable, undebuggable? I don't understand the resistance. > > > > > > Alex > > > > management really does not know how to handle unexpected > > migration failures. They must be avoided. > > > > There are some very special cases that fail migration. They are > > currently easy to find with grep register_device_unmigratable. > > I prefer to keep it that way. > > How can management tools be improved to better handle unexpected > migration failures when the only way for qemu to fail is an abort? > We need the infrastructure to at least return an error first. Do we just > need to add some fprintfs to the save core to print the id string of the > device that failed to save? I just can't buy the "code is easier to > grep" as an argument against adding better error handling to the save > code path. I just don't buy the 'we'll return meaningless error codes at random point in time and management will figure it out' as an argument :) > Anyone else want to chime in? > > Alex Maybe try coding up some user using the new infrastructure to do something useful, that register_device_unmigratable can't do. -- MST ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-09 16:49 ` Michael S. Tsirkin @ 2010-11-09 17:44 ` Alex Williamson 2010-11-09 19:35 ` Alex Williamson 0 siblings, 1 reply; 26+ messages in thread From: Alex Williamson @ 2010-11-09 17:44 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: cam, qemu-devel, kvm, quintela On Tue, 2010-11-09 at 18:49 +0200, Michael S. Tsirkin wrote: > On Tue, Nov 09, 2010 at 09:30:45AM -0700, Alex Williamson wrote: > > On Tue, 2010-11-09 at 18:15 +0200, Michael S. Tsirkin wrote: > > > On Tue, Nov 09, 2010 at 08:47:00AM -0700, Alex Williamson wrote: > > > > > > But it could. What if ivshmem is acting in a peer role, but has no > > > > > > clients, could it migrate? What if ivshmem is migratable when the > > > > > > migration begins, but while the migration continues, a connection is > > > > > > setup and it becomes unmigratable. > > > > > > > > > > Sounds like something we should work to prevent, not support :) > > > > > > > > s/:)/:(/ why? > > > > > > It will just confuse everyone. Also if it happens after sending > > > all of memory, it's pretty painful. > > > > It happens after sending all of memory with no_migrate, and I think > > pushing that earlier might introduce some races around when > > register_device_unmigratable() can be called. > > Good point. I guess we could check it twice just to speed things up. > > > > > > > Using this series, ivshmem would > > > > > > have multiple options how to support this. It could a) NAK the > > > > > > migration, b) drop connections and prevent new connections until the > > > > > > migration finishes, c) detect that new connections have happened since > > > > > > the migration started and cancel. And probably more. no_migrate can > > > > > > only do a). And in fact, we can only test no_migrate after the VM is > > > > > > stopped (after all memory is migrated) because otherwise it could race > > > > > > with devices setting no_migrate during migration. > > > > > > > > > > We really want no_migrate to be static. changing it is abusing > > > > > the infrastructure. > > > > > > > > You call it abusing, I call it making use of the infrastructure. Why > > > > unnecessarily restrict ourselves? Is return 0/-1 really that scary, > > > > unmaintainable, undebuggable? I don't understand the resistance. > > > > > > > > Alex > > > > > > management really does not know how to handle unexpected > > > migration failures. They must be avoided. > > > > > > There are some very special cases that fail migration. They are > > > currently easy to find with grep register_device_unmigratable. > > > I prefer to keep it that way. > > > > How can management tools be improved to better handle unexpected > > migration failures when the only way for qemu to fail is an abort? > > We need the infrastructure to at least return an error first. Do we just > > need to add some fprintfs to the save core to print the id string of the > > device that failed to save? I just can't buy the "code is easier to > > grep" as an argument against adding better error handling to the save > > code path. > > I just don't buy the 'we'll return meaningless error codes at random > point in time and management will figure it out' as an argument :) Why is the error code meaningless? The error code stops the migration in qemu and hopefully prints an error message (we could easily add an fprintf to the core save code to ensure we know the device responsible for the NAK). From there we can figure out how to return the error to monitors and management tools, but we need to have a way to know there's an error first. > > Anyone else want to chime in? > > > > Alex > > Maybe try coding up some user using the new infrastructure to do > something useful, that register_device_unmigratable can't do. With the number of people I hear complaining about how qemu has too many aborts, no error checking, and no way to return errors, I'm a little dumbfounded that there's such a roadblock to actually add some simple error handling. Is it the error handling you're opposed to, or the way I'm using it to NAK a migration? Alex ^ permalink raw reply [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-11-09 17:44 ` Alex Williamson @ 2010-11-09 19:35 ` Alex Williamson 0 siblings, 0 replies; 26+ messages in thread From: Alex Williamson @ 2010-11-09 19:35 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: cam, qemu-devel, kvm, quintela On Tue, 2010-11-09 at 10:44 -0700, Alex Williamson wrote: > On Tue, 2010-11-09 at 18:49 +0200, Michael S. Tsirkin wrote: > > On Tue, Nov 09, 2010 at 09:30:45AM -0700, Alex Williamson wrote: > > > On Tue, 2010-11-09 at 18:15 +0200, Michael S. Tsirkin wrote: > > > > On Tue, Nov 09, 2010 at 08:47:00AM -0700, Alex Williamson wrote: > > > > > > > But it could. What if ivshmem is acting in a peer role, but has no > > > > > > > clients, could it migrate? What if ivshmem is migratable when the > > > > > > > migration begins, but while the migration continues, a connection is > > > > > > > setup and it becomes unmigratable. > > > > > > > > > > > > Sounds like something we should work to prevent, not support :) > > > > > > > > > > s/:)/:(/ why? > > > > > > > > It will just confuse everyone. Also if it happens after sending > > > > all of memory, it's pretty painful. > > > > > > It happens after sending all of memory with no_migrate, and I think > > > pushing that earlier might introduce some races around when > > > register_device_unmigratable() can be called. > > > > Good point. I guess we could check it twice just to speed things up. > > > > > > > > > Using this series, ivshmem would > > > > > > > have multiple options how to support this. It could a) NAK the > > > > > > > migration, b) drop connections and prevent new connections until the > > > > > > > migration finishes, c) detect that new connections have happened since > > > > > > > the migration started and cancel. And probably more. no_migrate can > > > > > > > only do a). And in fact, we can only test no_migrate after the VM is > > > > > > > stopped (after all memory is migrated) because otherwise it could race > > > > > > > with devices setting no_migrate during migration. > > > > > > > > > > > > We really want no_migrate to be static. changing it is abusing > > > > > > the infrastructure. > > > > > > > > > > You call it abusing, I call it making use of the infrastructure. Why > > > > > unnecessarily restrict ourselves? Is return 0/-1 really that scary, > > > > > unmaintainable, undebuggable? I don't understand the resistance. > > > > > > > > > > Alex > > > > > > > > management really does not know how to handle unexpected > > > > migration failures. They must be avoided. > > > > > > > > There are some very special cases that fail migration. They are > > > > currently easy to find with grep register_device_unmigratable. > > > > I prefer to keep it that way. > > > > > > How can management tools be improved to better handle unexpected > > > migration failures when the only way for qemu to fail is an abort? > > > We need the infrastructure to at least return an error first. Do we just > > > need to add some fprintfs to the save core to print the id string of the > > > device that failed to save? I just can't buy the "code is easier to > > > grep" as an argument against adding better error handling to the save > > > code path. > > > > I just don't buy the 'we'll return meaningless error codes at random > > point in time and management will figure it out' as an argument :) > > Why is the error code meaningless? The error code stops the migration > in qemu and hopefully prints an error message (we could easily add an > fprintf to the core save code to ensure we know the device responsible > for the NAK). From there we can figure out how to return the error to > monitors and management tools, but we need to have a way to know there's > an error first. > > > > Anyone else want to chime in? > > > > > > Alex > > > > Maybe try coding up some user using the new infrastructure to do > > something useful, that register_device_unmigratable can't do. > > With the number of people I hear complaining about how qemu has too many > aborts, no error checking, and no way to return errors, I'm a little > dumbfounded that there's such a roadblock to actually add some simple > error handling. Is it the error handling you're opposed to, or the way > I'm using it to NAK a migration? Would something like this in place of 6/6 ease your grep'ability and debug'ability concerns? Unfortunately it adds an assert, but if we stomp on the vmsd, then the save state entry can't be unregistered. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- diff --git a/savevm.c b/savevm.c index 521edc8..b0aaa46 100644 --- a/savevm.c +++ b/savevm.c @@ -1039,7 +1039,6 @@ typedef struct SaveStateEntry { const VMStateDescription *vmsd; void *opaque; CompatEntry *compat; - int no_migrate; } SaveStateEntry; @@ -1103,7 +1102,6 @@ int register_savevm_live(DeviceState *dev, se->load_state = load_state; se->opaque = opaque; se->vmsd = NULL; - se->no_migrate = 0; if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) { char *id = dev->parent_bus->info->get_dev_path(dev); @@ -1170,6 +1168,22 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque) } } +static int nomigrate_set_params(int blk_enable, int shared, void *opaque) +{ + return -EFAULT; +} + +static int nomigrate_save_live_state(Monitor *mon, QEMUFile *f, int stage, + void *opaque) +{ + return -EFAULT; +} + +static int nomigrate_save_state(QEMUFile *f, void *opaque) +{ + return -EFAULT; +} + /* mark a device as not to be migrated, that is the device should be unplugged before migration */ void register_device_unmigratable(DeviceState *dev, const char *idstr, @@ -1190,7 +1204,10 @@ void register_device_unmigratable(DeviceState *dev, const char *idstr, QTAILQ_FOREACH(se, &savevm_handlers, entry) { if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) { - se->no_migrate = 1; + se->set_params = nomigrate_set_params; + se->save_live_state = nomigrate_save_live_state; + se->save_state = nomigrate_save_state; + assert(!se->vmsd); } } } @@ -1410,10 +1427,6 @@ static int vmstate_load(QEMUFile *f, SaveStateEntry *se, int version_id) static int vmstate_save(QEMUFile *f, SaveStateEntry *se) { - if (se->no_migrate) { - return -1; - } - if (!se->vmsd) { /* Old style */ return se->save_state(f, se->opaque); } @@ -1443,6 +1456,9 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable, } ret = se->set_params(blk_enable, shared, se->opaque); if (ret < 0) { + monitor_printf(mon, + "Save state begin blocked by device '%s', error:" + " %s\n", se->idstr, strerror(-ret)); return ret; } } @@ -1470,6 +1486,9 @@ int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable, ret = se->save_live_state(mon, f, QEMU_VM_SECTION_START, se->opaque); if (ret < 0) { + monitor_printf(mon, + "Save state begin blocked by device '%s', error:" + " %s\n", se->idstr, strerror(-ret)); return ret; } } @@ -1503,6 +1522,9 @@ int qemu_savevm_state_iterate(Monitor *mon, QEMUFile *f) synchronized over and over again. */ break; } else if (ret < 0) { + monitor_printf(mon, + "Save state iterate blocked by device '%s', error:" + " %s\n", se->idstr, strerror(-ret)); return ret; } } @@ -1535,6 +1557,9 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f) r = se->save_live_state(mon, f, QEMU_VM_SECTION_END, se->opaque); if (r < 0) { + monitor_printf(mon, + "Save state complete blocked by device '%s', error:" + " %s\n", se->idstr, strerror(-r)); return r; } } @@ -1559,7 +1584,9 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f) r = vmstate_save(f, se); if (r < 0) { - monitor_printf(mon, "cannot migrate with device '%s'\n", se->idstr); + monitor_printf(mon, + "Save state complete blocked by device '%s', error:" + " %s\n", se->idstr, strerror(-r)); return r; } } ^ permalink raw reply related [flat|nested] 26+ messages in thread
* [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) 2010-10-06 20:58 [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson ` (7 preceding siblings ...) 2010-11-08 11:40 ` Michael S. Tsirkin @ 2010-11-16 10:23 ` Juan Quintela 8 siblings, 0 replies; 26+ messages in thread From: Juan Quintela @ 2010-11-16 10:23 UTC (permalink / raw) To: Alex Williamson; +Cc: cam, qemu-devel, kvm Alex Williamson <alex.williamson@redhat.com> 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 > Reviewed-by: Juan Quintela <quintela@redhat.com> Just to address some of mst concerns: - no_migrate was wrong from the beggining. We have enough setup to disable tihngs. - I did save handlers that didn't return any error because they dind't have it when I started, it would have been way better if I had done it the other way around. I was going to need this change done _anyways_, didn't start for there because there were other things to fix. - we really need to be able to return errors in save paths: * ihvm device, it can migrate some times, and no others (we can discuss the details) * device assignment: we can't migrate, and we need a way to say so. * if we want reliable migration & machine definitions, we are going to have to implement device versions at some point. This clearly requires failure of save migration (i.e. we ask to save a device with version n-1 (or without some subsection) and it finds that this would breaks. So I woh Later, Juan. ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2010-11-16 10:23 UTC | newest] Thread overview: 26+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-06 20:58 [Qemu-devel] [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 1/6] savevm: Allow SaveStateHandler() to return error Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 2/6] savevm: Allow vmsd->pre_save " Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 3/6] pci: Allow pci_device_save() " Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 4/6] virtio: Allow virtio_save() errors Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 5/6] savevm: Allow set_params and save_live_state to error Alex Williamson 2010-10-06 20:59 ` [Qemu-devel] [PATCH 6/6] savevm: Remove register_device_unmigratable() Alex Williamson 2010-10-07 16:55 ` [Qemu-devel] Re: [PATCH 0/6] Save state error handling (kill off no_migrate) Alex Williamson 2010-11-08 11:40 ` Michael S. Tsirkin 2010-11-08 14:59 ` Alex Williamson 2010-11-08 16:54 ` Michael S. Tsirkin 2010-11-08 17:20 ` Alex Williamson 2010-11-08 20:59 ` Michael S. Tsirkin 2010-11-08 21:23 ` Alex Williamson 2010-11-09 12:00 ` Michael S. Tsirkin 2010-11-09 14:58 ` Alex Williamson 2010-11-09 15:07 ` Michael S. Tsirkin 2010-11-09 15:34 ` Alex Williamson 2010-11-09 15:42 ` Michael S. Tsirkin 2010-11-09 15:47 ` Alex Williamson 2010-11-09 16:15 ` Michael S. Tsirkin 2010-11-09 16:30 ` Alex Williamson 2010-11-09 16:49 ` Michael S. Tsirkin 2010-11-09 17:44 ` Alex Williamson 2010-11-09 19:35 ` Alex Williamson 2010-11-16 10:23 ` Juan Quintela
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).