migration: let pre_save fail
v2 cover migration: let pre_save fail
From: "Dr. David Alan Gilbert" <[email protected]>
At the moment the pre-save method on VMStateDescription returns void which means there's no sane way of a pre_save routine failing. There are quite a few pre_save's that abort or do other nasty things, and it would be better if they could fail cleanly.
This patch series doesn't fix any of those pre_save's - it just changes pre_save to return an error value and routes that back up so the migration fails. I'll leave that up to the individual device owners.
There are a bunch of corner cases that I've not fixed yet; these are mostly places that call vmstate_save_state which have been partially converted to modern migration code; often the functions they're called from return void as well - they can get fixed later.
v2
test: Split assert/side effects [Fam]
new patch 3: Changed to also check the return of the ->put methods
that should have been checked previously. [Peter]
Dr. David Alan Gilbert (6):
migration: pre_save return int
migration: check pre_save return in vmstate_save_state
migration: Check field save returns
migration: wire vmstate_save_state errors up to
vmstate_subsection_save
migration: Route errors up through vmstate_save
migration: Route more error paths
docs/devel/migration.txt | 2 +- hw/arm/pxa2xx.c | 4 +++- hw/arm/strongarm.c | 4 +++- hw/audio/wm8750.c | 4 +++- hw/block/fdc.c | 4 +++- hw/block/m25p80.c | 4 +++- hw/block/nand.c | 4 +++- hw/block/onenand.c | 4 +++- hw/char/serial.c | 4 +++- hw/display/qxl.c | 4 +++- hw/display/virtio-gpu.c | 4 +--- hw/i2c/core.c | 4 +++- hw/i386/kvm/clock.c | 4 +++- hw/ide/core.c | 4 +++- hw/ide/pci.c | 4 +++- hw/input/ps2.c | 8 ++++++-- hw/input/tsc210x.c | 4 +++- hw/intc/apic_common.c | 4 +++- hw/intc/arm_gic_common.c | 4 +++- hw/intc/arm_gicv3_common.c | 4 +++- hw/intc/arm_gicv3_its_common.c | 4 +++- hw/intc/i8259_common.c | 4 +++- hw/intc/ioapic_common.c | 4 +++- hw/intc/s390_flic_kvm.c | 6 ++++-- hw/intc/xics.c | 8 ++++++-- hw/net/e1000.c | 4 +++- hw/net/e1000e.c | 4 +++- hw/net/rtl8139.c | 4 +++- hw/net/virtio-net.c | 16 ++++++++++++---- hw/net/vmxnet3.c | 4 +++- hw/pci-host/piix.c | 4 +++- hw/ppc/ppc.c | 4 +++- hw/ppc/spapr_iommu.c | 4 +++- hw/ppc/spapr_pci.c | 6 ++++-- hw/s390x/css.c | 10 +++++++--- hw/s390x/virtio-ccw.c | 4 +++- hw/scsi/lsi53c895a.c | 4 +++- hw/scsi/vmw_pvscsi.c | 4 +++- hw/timer/cadence_ttc.c | 4 +++- hw/timer/hpet.c | 4 +++- hw/timer/i8254_common.c | 4 +++- hw/timer/mc146818rtc.c | 4 +++- hw/timer/pl031.c | 4 +++- hw/timer/twl92230.c | 4 +++- hw/usb/dev-smartcard-reader.c | 4 +++- hw/usb/hcd-ehci.c | 4 +++- hw/usb/redirect.c | 4 +++- hw/virtio/vhost-vsock.c | 4 +++- hw/virtio/virtio.c | 13 +++++++------ include/hw/virtio/virtio.h | 2 +- include/migration/vmstate.h | 6 +++--- migration/colo-comm.c | 4 +++- migration/global_state.c | 4 +++- migration/savevm.c | 23 ++++++++++++++++------ migration/trace-events | 1 + migration/vmstate-types.c | 11 ++++++++--- migration/vmstate.c | 43 +++++++++++++++++++++++++++++++----------- replay/replay-snapshot.c | 4 +++- slirp/slirp.c | 8 ++++++-- target/arm/machine.c | 4 +++- target/i386/machine.c | 7 +++++-- target/ppc/machine.c | 4 +++- target/s390x/machine.c | 4 +++- target/sparc/machine.c | 4 +++- tests/test-vmstate.c | 13 +++++++++---- 65 files changed, 271 insertions(+), 104 deletions(-)
Signature · 1 line
2.13.5
PATCH v2 1/6 migration: pre_save return int
From: "Dr. David Alan Gilbert" <[email protected]>
Modify the pre_save method on VMStateDescription to return an int rather than void so that it potentially can fail.
Changed zillions of devices to make them return 0; the only case I've made it return non-0 is hw/intc/s390_flic_kvm.c that already had an error_report/return case.
Note: If you add an error exit in your pre_save you must emit an error_report to say why.
--- docs/devel/migration.txt | 2 +- hw/arm/pxa2xx.c | 4 +++- hw/arm/strongarm.c | 4 +++- hw/audio/wm8750.c | 4 +++- hw/block/fdc.c | 4 +++- hw/block/m25p80.c | 4 +++- hw/block/nand.c | 4 +++- hw/block/onenand.c | 4 +++- hw/char/serial.c | 4 +++- hw/display/qxl.c | 4 +++- hw/i2c/core.c | 4 +++- hw/i386/kvm/clock.c | 4 +++- hw/ide/core.c | 4 +++- hw/ide/pci.c | 4 +++- hw/input/ps2.c | 8 ++++++-- hw/input/tsc210x.c | 4 +++- hw/intc/apic_common.c | 4 +++- hw/intc/arm_gic_common.c | 4 +++- hw/intc/arm_gicv3_common.c | 4 +++- hw/intc/arm_gicv3_its_common.c | 4 +++- hw/intc/i8259_common.c | 4 +++- hw/intc/ioapic_common.c | 4 +++- hw/intc/s390_flic_kvm.c | 6 ++++-- hw/intc/xics.c | 8 ++++++-- hw/net/e1000.c | 4 +++- hw/net/e1000e.c | 4 +++- hw/net/rtl8139.c | 4 +++- hw/net/virtio-net.c | 16 ++++++++++++---- hw/net/vmxnet3.c | 4 +++- hw/pci-host/piix.c | 4 +++- hw/ppc/ppc.c | 4 +++- hw/ppc/spapr_iommu.c | 4 +++- hw/ppc/spapr_pci.c | 6 ++++-- hw/s390x/css.c | 10 +++++++--- hw/s390x/virtio-ccw.c | 4 +++- hw/scsi/lsi53c895a.c | 4 +++- hw/scsi/vmw_pvscsi.c | 4 +++- hw/timer/cadence_ttc.c | 4 +++- hw/timer/hpet.c | 4 +++- hw/timer/i8254_common.c | 4 +++- hw/timer/mc146818rtc.c | 4 +++- hw/timer/pl031.c | 4 +++- hw/timer/twl92230.c | 4 +++- hw/usb/dev-smartcard-reader.c | 4 +++- hw/usb/hcd-ehci.c | 4 +++- hw/usb/redirect.c | 4 +++- hw/virtio/vhost-vsock.c | 4 +++- include/migration/vmstate.h | 2 +- migration/colo-comm.c | 4 +++- migration/global_state.c | 4 +++- migration/savevm.c | 4 +++- replay/replay-snapshot.c | 4 +++- slirp/slirp.c | 8 ++++++-- target/arm/machine.c | 4 +++- target/i386/machine.c | 7 +++++-- target/ppc/machine.c | 4 +++- target/s390x/machine.c | 4 +++- target/sparc/machine.c | 4 +++- tests/test-vmstate.c | 4 +++- 59 files changed, 199 insertions(+), 70 deletions(-)
docs/devel/migration.txt +1 -1
@@ -202,7 +202,7 @@ The functions to do that are inside a vmstate definition, and are called: This function is called after we load the state of one device. -- void (*pre_save)(void *opaque); +- int (*pre_save)(void *opaque); This function is called before we save the state of one device.
hw/arm/pxa2xx.c +3 -1
@@ -1143,13 +1143,15 @@ static void pxa2xx_rtc_init(Object *obj)sysbus_init_mmio (dev, &s->iomem); } -static void pxa2xx_rtc_pre_save (void *opaque) +static int pxa2xx_rtc_pre_save (void *opaque) {PXA2xxRTCState *s = (PXA2xxRTCState *) opaque;pxa2xx_rtc_hzupdate (s);pxa2xx_rtc_piupdate (s);pxa2xx_rtc_swupdate (s); + +return 0 ; }static int pxa2xx_rtc_post_load (void *opaque,int version_id)
hw/arm/strongarm.c +3 -1
@@ -406,11 +406,13 @@ static void strongarm_rtc_init(Object *obj)sysbus_init_mmio (dev, &s->iomem); } -static void strongarm_rtc_pre_save (void *opaque) +static int strongarm_rtc_pre_save (void *opaque) {StrongARMRTCState *s = opaque;strongarm_rtc_hzupdate (s); + +return 0 ; }static int strongarm_rtc_post_load (void *opaque,int version_id)
hw/audio/wm8750.c +3 -1
@@ -567,11 +567,13 @@ static int wm8750_rx(I2CSlave *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; + +return 0 ; }static int wm8750_post_load (void *opaque,int version_id)
hw/block/fdc.c +3 -1
@@ -1101,11 +1101,13 @@ static int reconstruct_phase(FDCtrl *fdctrl) } } -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_pre_load (void *opaque)
hw/block/m25p80.c +3 -1
@@ -1251,9 +1251,11 @@ static void m25p80_reset(DeviceState *d)reset_memory (s); } -static void m25p80_pre_save (void *opaque) +static int m25p80_pre_save (void *opaque) {flash_sync_dirty ((Flash *)opaque,-1 ); + +return 0 ; }static Property m25p80_properties[] = {
hw/block/nand.c +3 -1
@@ -325,11 +325,13 @@ static void nand_command(NANDFlashState *s) } } -static void nand_pre_save (void *opaque) +static int nand_pre_save (void *opaque) {NANDFlashState *s =NAND (opaque); s->ioaddr_vmstate = s->ioaddr - s->io; + +return 0 ; }static int nand_post_load (void *opaque,int version_id)
hw/block/onenand.c +3 -1
@@ -137,7 +137,7 @@ static void onenand_intr_update(OneNANDState *s)qemu_set_irq (s->intr, ((s->intstatus >>15 ) ^ (~s->config[0 ] >>6 )) &1 ); } -static void onenand_pre_save (void *opaque) +static int onenand_pre_save (void *opaque) {OneNANDState *s = opaque;if (s->current == s->otp) {
@@ -147,6 +147,8 @@ static void onenand_pre_save(void *opaque) }else { s->current_direction =0 ; } + +return 0 ; }static int onenand_post_load (void *opaque,int version_id)
hw/char/serial.c +3 -1
@@ -630,10 +630,12 @@ 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_pre_load (void *opaque)
hw/display/qxl.c +3 -1
@@ -2204,7 +2204,7 @@ static void qxl_realize_secondary(PCIDevice *dev, Error **errp)qxl_realize_common (qxl, errp); } -static void qxl_pre_save (void *opaque) +static int qxl_pre_save (void *opaque) {PCIQXLDevice * d = opaque;uint8_t *ram_start = d->vga.vram_ptr;
@@ -2216,6 +2216,8 @@ static void qxl_pre_save(void *opaque) d->last_release_offset = (uint8_t *)d->last_release - ram_start; }assert (d->last_release_offset < d->vga.vram_size); + +return 0 ; }static int qxl_pre_load (void *opaque)
hw/i2c/core.c +3 -1
@@ -41,7 +41,7 @@ static const TypeInfo i2c_bus_info = { .instance_size = sizeof(I2CBus), }; -static void i2c_bus_pre_save (void *opaque) +static int i2c_bus_pre_save (void *opaque) {I2CBus *bus = opaque;
@@ -53,6 +53,8 @@ static void i2c_bus_pre_save(void *opaque) bus->saved_address =I2C_BROADCAST ; } } + +return 0 ; }static const VMStateDescription vmstate_i2c_bus = {
hw/i386/kvm/clock.c +3 -1
@@ -254,11 +254,13 @@ static const VMStateDescription kvmclock_reliable_get_clock = {* final pages of memory (which happens between vm_stop() * and pre_save()) takes max_downtime. */ -static void kvmclock_pre_save (void *opaque) +static int kvmclock_pre_save (void *opaque) {KVMClockState *s = opaque;kvm_update_clock (s); + +return 0 ; }static const VMStateDescription kvmclock_vmsd = {
hw/ide/core.c +3 -1
@@ -2752,7 +2752,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;
@@ -2768,6 +2768,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)
hw/ide/pci.c +3 -1
@@ -296,7 +296,7 @@ static bool ide_bmdma_status_needed(void *opaque)return ((bm->status & abused_bits) !=0 ); } -static void ide_bmdma_pre_save (void *opaque) +static int ide_bmdma_pre_save (void *opaque) {BMDMAState *bm = opaque;uint8_t abused_bits =BM_MIGRATION_COMPAT_STATUS_BITS ;
@@ -310,6 +310,8 @@ static void ide_bmdma_pre_save(void *opaque) bm->migration_retry_nsector = bm->bus->retry_nsector; bm->migration_compat_status = (bm->status & ~abused_bits) | (bm->bus->error_status & abused_bits); + +return 0 ; }/* This function accesses bm->bus->error_status which is loaded only after
hw/input/ps2.c +6 -2
@@ -1216,12 +1216,14 @@ static int ps2_kbd_post_load(void* opaque, int version_id)return 0 ; } -static void ps2_kbd_pre_save (void *opaque) +static int ps2_kbd_pre_save (void *opaque) {PS2KbdState *s = (PS2KbdState *)opaque;PS2State *ps2 = &s->common;ps2_common_post_load (ps2); + +return 0 ; }static const VMStateDescription vmstate_ps2_keyboard = {
@@ -1254,12 +1256,14 @@ static int ps2_mouse_post_load(void *opaque, int version_id)return 0 ; } -static void ps2_mouse_pre_save (void *opaque) +static int ps2_mouse_pre_save (void *opaque) {PS2MouseState *s = (PS2MouseState *)opaque;PS2State *ps2 = &s->common;ps2_common_post_load (ps2); + +return 0 ; }static const VMStateDescription vmstate_ps2_mouse = {
hw/input/tsc210x.c +3 -1
@@ -976,10 +976,12 @@ static void tsc210x_i2s_set_rate(TSC210xState *s, int in, int out) s->i2s_rx_rate = in; } -static void tsc210x_pre_save (void *opaque) +static int tsc210x_pre_save (void *opaque) {TSC210xState *s = (TSC210xState *) opaque; s->now =qemu_clock_get_ns (QEMU_CLOCK_VIRTUAL ); + +return 0 ; }static int tsc210x_post_load (void *opaque,int version_id)
hw/intc/apic_common.c +3 -1
@@ -360,7 +360,7 @@ static int apic_pre_load(void *opaque)return 0 ; } -static void apic_dispatch_pre_save (void *opaque) +static int apic_dispatch_pre_save (void *opaque) {APICCommonState *s =APIC_COMMON (opaque);APICCommonClass *info =APIC_COMMON_GET_CLASS (s);
@@ -368,6 +368,8 @@ static void apic_dispatch_pre_save(void *opaque)if (info->pre_save) { info->pre_save (s); } + +return 0 ; }static int apic_dispatch_post_load (void *opaque,int version_id)
hw/intc/arm_gic_common.c +3 -1
@@ -23,7 +23,7 @@#include "gic_internal.h" #include "hw/arm/linux-boot-if.h" -static void gic_pre_save (void *opaque) +static int gic_pre_save (void *opaque) {GICState *s = (GICState *)opaque;ARMGICCommonClass *c =ARM_GIC_COMMON_GET_CLASS (s);
@@ -31,6 +31,8 @@ static void gic_pre_save(void *opaque)if (c->pre_save) { c->pre_save (s); } + +return 0 ; }static int gic_post_load (void *opaque,int version_id)
hw/intc/arm_gicv3_common.c +3 -1
@@ -28,7 +28,7 @@#include "gicv3_internal.h" #include "hw/arm/linux-boot-if.h" -static void gicv3_pre_save (void *opaque) +static int gicv3_pre_save (void *opaque) {GICv3State *s = (GICv3State *)opaque;ARMGICv3CommonClass *c =ARM_GICV3_COMMON_GET_CLASS (s);
@@ -36,6 +36,8 @@ static void gicv3_pre_save(void *opaque)if (c->pre_save) { c->pre_save (s); } + +return 0 ; }static int gicv3_post_load (void *opaque,int version_id)
hw/intc/arm_gicv3_its_common.c +3 -1
@@ -23,7 +23,7 @@#include "hw/intc/arm_gicv3_its_common.h" #include "qemu/log.h" -static void gicv3_its_pre_save (void *opaque) +static int gicv3_its_pre_save (void *opaque) {GICv3ITSState *s = (GICv3ITSState *)opaque;GICv3ITSCommonClass *c =ARM_GICV3_ITS_COMMON_GET_CLASS (s);
@@ -31,6 +31,8 @@ static void gicv3_its_pre_save(void *opaque)if (c->pre_save) { c->pre_save (s); } + +return 0 ; }static int gicv3_its_post_load (void *opaque,int version_id)
hw/intc/i8259_common.c +3 -1
@@ -46,7 +46,7 @@ void pic_reset_common(PICCommonState *s)/* Note: ELCR is not reset */ } -static void pic_dispatch_pre_save (void *opaque) +static int pic_dispatch_pre_save (void *opaque) {PICCommonState *s = opaque;PICCommonClass *info =PIC_COMMON_GET_CLASS (s);
@@ -54,6 +54,8 @@ static void pic_dispatch_pre_save(void *opaque)if (info->pre_save) { info->pre_save (s); } + +return 0 ; }static int pic_dispatch_post_load (void *opaque,int version_id)
hw/intc/ioapic_common.c +3 -1
@@ -102,7 +102,7 @@ void ioapic_reset_common(DeviceState *dev) } } -static void ioapic_dispatch_pre_save (void *opaque) +static int ioapic_dispatch_pre_save (void *opaque) {IOAPICCommonState *s =IOAPIC_COMMON (opaque);IOAPICCommonClass *info =IOAPIC_COMMON_GET_CLASS (s);
@@ -110,6 +110,8 @@ static void ioapic_dispatch_pre_save(void *opaque)if (info->pre_save) { info->pre_save (s); } + +return 0 ; }static int ioapic_dispatch_post_load (void *opaque,int version_id)
hw/intc/s390_flic_kvm.c +4 -2
@@ -420,7 +420,7 @@ typedef struct KVMS390FLICStateMigTmp {uint8_t nimm; } KVMS390FLICStateMigTmp; -static void kvm_flic_ais_pre_save (void *opaque) +static int kvm_flic_ais_pre_save (void *opaque) {KVMS390FLICStateMigTmp *tmp = opaque;KVMS390FLICState *flic = tmp->parent;
@@ -433,11 +433,13 @@ static void kvm_flic_ais_pre_save(void *opaque)if (ioctl (flic->fd,KVM_GET_DEVICE_ATTR , &attr)) {error_report ("Failed to retrieve kvm flic ais states" ); -return ; +return -EINVAL ; } tmp->simm = ais.simm; tmp->nimm = ais.nimm; + +return 0 ; }static int kvm_flic_ais_post_load (void *opaque,int version_id)
hw/intc/xics.c +6 -2
@@ -241,7 +241,7 @@ static void icp_irq(ICSState *ics, int server, int nr, uint8_t priority) } } -static void icp_dispatch_pre_save (void *opaque) +static int icp_dispatch_pre_save (void *opaque) {ICPState *icp = opaque;ICPStateClass *info =ICP_GET_CLASS (icp);
@@ -249,6 +249,8 @@ static void icp_dispatch_pre_save(void *opaque)if (info->pre_save) { info->pre_save (icp); } + +return 0 ; }static int icp_dispatch_post_load (void *opaque,int version_id)
@@ -533,7 +535,7 @@ static void ics_simple_reset(void *dev) } } -static void ics_simple_dispatch_pre_save (void *opaque) +static int ics_simple_dispatch_pre_save (void *opaque) {ICSState *ics = opaque;ICSStateClass *info =ICS_BASE_GET_CLASS (ics);
@@ -541,6 +543,8 @@ static void ics_simple_dispatch_pre_save(void *opaque)if (info->pre_save) { info->pre_save (ics); } + +return 0 ; }static int ics_simple_dispatch_post_load (void *opaque,int version_id)
hw/net/e1000.c +3 -1
@@ -1343,7 +1343,7 @@ static bool is_version_1(void *opaque, int version_id)return version_id ==1 ; } -static void e1000_pre_save (void *opaque) +static int e1000_pre_save (void *opaque) {E1000State *s = opaque;NetClientState *nc =qemu_get_queue (s->nic);
@@ -1361,6 +1361,8 @@ static void e1000_pre_save(void *opaque)if (nc->link_down &&have_autoneg (s)) { s->phy_reg[PHY_STATUS ] |=MII_SR_AUTONEG_COMPLETE ; } + +return 0 ; }static int e1000_post_load (void *opaque,int version_id)
hw/net/e1000e.c +3 -1
@@ -523,13 +523,15 @@ static void e1000e_qdev_reset(DeviceState *dev)e1000e_core_reset (&s->core); } -static void e1000e_pre_save (void *opaque) +static int e1000e_pre_save (void *opaque) {E1000EState *s = opaque;trace_e1000e_cb_pre_save ();e1000e_core_pre_save (&s->core); + +return 0 ; }static int e1000e_post_load (void *opaque,int version_id)
hw/net/rtl8139.c +3 -1
@@ -3162,7 +3162,7 @@ static const VMStateDescription vmstate_rtl8139_hotplug_ready ={ } }; -static void rtl8139_pre_save (void *opaque) +static int rtl8139_pre_save (void *opaque) {RTL8139State * s = opaque;int64_t current_time =qemu_clock_get_ns (QEMU_CLOCK_VIRTUAL );
@@ -3170,6 +3170,8 @@ static void rtl8139_pre_save(void *opaque)/* for migration to older versions */ s->TCTR = (current_time - s->TCTR_base) /PCI_PERIOD ; s->rtl8139_mmio_io_addr_dummy =0 ; + +return 0 ; }static const VMStateDescription vmstate_rtl8139 = {
hw/net/virtio-net.c +12 -4
@@ -1712,7 +1712,7 @@ struct VirtIONetMigTmp {* pointer and count and also validate the count. */ -static void virtio_net_tx_waiting_pre_save (void *opaque) +static int virtio_net_tx_waiting_pre_save (void *opaque) {struct VirtIONetMigTmp *tmp = opaque;
@@ -1721,6 +1721,8 @@ static void virtio_net_tx_waiting_pre_save(void *opaque)if (tmp->parent->curr_queues ==0 ) { tmp->curr_queues_1 =0 ; } + +return 0 ; }static int virtio_net_tx_waiting_pre_load (void *opaque)
@@ -1768,11 +1770,13 @@ static int virtio_net_ufo_post_load(void *opaque, int version_id)return 0 ; } -static void virtio_net_ufo_pre_save (void *opaque) +static int virtio_net_ufo_pre_save (void *opaque) {struct VirtIONetMigTmp *tmp = opaque; tmp->has_ufo = tmp->parent->has_ufo; + +return 0 ; }static const VMStateDescription vmstate_virtio_net_has_ufo = {
@@ -1800,11 +1804,13 @@ static int virtio_net_vnet_post_load(void *opaque, int version_id)return 0 ; } -static void virtio_net_vnet_pre_save (void *opaque) +static int virtio_net_vnet_pre_save (void *opaque) {struct VirtIONetMigTmp *tmp = opaque; tmp->has_vnet_hdr = tmp->parent->has_vnet_hdr; + +return 0 ; }static const VMStateDescription vmstate_virtio_net_has_vnet = {
@@ -2079,13 +2085,15 @@ static void virtio_net_instance_init(Object *obj) DEVICE(n), NULL); } -static void virtio_net_pre_save (void *opaque) +static int virtio_net_pre_save (void *opaque) {VirtIONet *n = opaque;/* At this point, backend must be stopped, otherwise * it might keep writing to memory. */ assert (!n->vhost_started); + +return 0 ; }static const VMStateDescription vmstate_virtio_net = {
hw/net/vmxnet3.c +3 -1
@@ -2388,11 +2388,13 @@ static int vmxnet3_mcast_list_pre_load(void *opaque) } -static void vmxnet3_pre_save (void *opaque) +static int vmxnet3_pre_save (void *opaque) {VMXNET3State *s = opaque; s->mcast_list_buff_size = s->mcast_list_len *sizeof (MACAddr); + +return 0 ; }static const VMStateDescription vmxstate_vmxnet3_mcast_list = {
hw/pci-host/piix.c +3 -1
@@ -579,7 +579,7 @@ static int piix3_post_load(void *opaque, int version_id)return 0 ; } -static void piix3_pre_save (void *opaque) +static int piix3_pre_save (void *opaque) {int i;PIIX3State *piix3 = opaque;
@@ -588,6 +588,8 @@ static void piix3_pre_save(void *opaque) piix3->pci_irq_levels_vmstate[i] =pci_bus_get_irq_level (piix3->dev.bus, i); } + +return 0 ; }static bool piix3_rcr_needed (void *opaque)
hw/ppc/ppc.c +3 -1
@@ -937,11 +937,13 @@ void cpu_ppc_clock_vm_state_change(void *opaque, int running,* final pages of memory (which happens between vm_stop() * and pre_save()) takes max_downtime. */ -static void timebase_pre_save (void *opaque) +static int timebase_pre_save (void *opaque) {PPCTimebase *tb = opaque;timebase_save (tb); + +return 0 ; }const VMStateDescription vmstate_ppc_timebase = {
hw/ppc/spapr_iommu.c +3 -1
@@ -140,7 +140,7 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(IOMMUMemoryRegion *iommu,return ret; } -static void spapr_tce_table_pre_save (void *opaque) +static int spapr_tce_table_pre_save (void *opaque) {sPAPRTCETable *tcet =SPAPR_TCE_TABLE (opaque);
@@ -149,6 +149,8 @@ static void spapr_tce_table_pre_save(void *opaque)trace_spapr_iommu_pre_save (tcet->liobn, tcet->mig_nb_table, tcet->bus_offset, tcet->page_shift); + +return 0 ; }static uint64_t spapr_tce_get_min_page_size (IOMMUMemoryRegion *iommu)
hw/ppc/spapr_pci.c +4 -2
@@ -1844,7 +1844,7 @@ static const VMStateDescription vmstate_spapr_pci_msi = { }, }; -static void spapr_pci_pre_save (void *opaque) +static int spapr_pci_pre_save (void *opaque) {sPAPRPHBState *sphb = opaque;GHashTableIter iter;
@@ -1869,7 +1869,7 @@ static void spapr_pci_pre_save(void *opaque) sphb->msi_devs =NULL ; sphb->msi_devs_num =g_hash_table_size (sphb->msi);if (!sphb->msi_devs_num) { -return ; +return 0 ; } sphb->msi_devs =g_malloc (sphb->msi_devs_num *sizeof (spapr_pci_msi_mig));
@@ -1878,6 +1878,8 @@ static void spapr_pci_pre_save(void *opaque) sphb->msi_devs[i].key = *(uint32_t *) key; sphb->msi_devs[i].value = *(spapr_pci_msi *) value; } + +return 0 ; }static int spapr_pci_post_load (void *opaque,int version_id)
hw/s390x/css.c +7 -3
@@ -196,7 +196,7 @@ static const VMStateDescription vmstate_schdev_orb = { };static int subch_dev_post_load (void *opaque,int version_id); -static void subch_dev_pre_save (void *opaque); +static int subch_dev_pre_save (void *opaque);const char err_hint_devno[] ="Devno mismatch, tried to load wrong section!" " Likely reason: some sequences of plug and unplug can break"
@@ -249,7 +249,7 @@ static int post_load_ind_addr(void *opaque, int version_id)return 0 ; } -static void pre_save_ind_addr (void *opaque) +static int pre_save_ind_addr (void *opaque) {IndAddrPtrTmp *ptmp = opaque;IndAddr *ind_addr = *(ptmp->parent);
@@ -261,6 +261,8 @@ static void pre_save_ind_addr(void *opaque) ptmp->len =0 ; ptmp->addr =0L ; } + +return 0 ; }const VMStateDescription vmstate_ind_addr_tmp = {
@@ -358,12 +360,14 @@ static ChannelSubSys channel_subsys = { QTAILQ_HEAD_INITIALIZER(channel_subsys.indicator_addresses), }; -static void subch_dev_pre_save (void *opaque) +static int subch_dev_pre_save (void *opaque) {SubchDev *s = opaque;/* Prepare remote_schid for save */ s->migrated_schid = s->schid; + +return 0 ; }static int subch_dev_post_load (void *opaque,int version_id)
hw/s390x/virtio-ccw.c +3 -1
@@ -62,13 +62,15 @@ typedef struct VirtioCcwDeviceTmp {uint16_t config_vector; } VirtioCcwDeviceTmp; -static void virtio_ccw_dev_tmp_pre_save (void *opaque) +static int virtio_ccw_dev_tmp_pre_save (void *opaque) {VirtioCcwDeviceTmp *tmp = opaque;VirtioCcwDevice *dev = tmp->parent;VirtIODevice *vdev =virtio_bus_get_device (&dev->bus); tmp->config_vector = vdev->config_vector; + +return 0 ; }static int virtio_ccw_dev_tmp_post_load (void *opaque,int version_id)
hw/scsi/lsi53c895a.c +3 -1
@@ -2083,7 +2083,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;
@@ -2092,6 +2092,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 = {
hw/scsi/vmw_pvscsi.c +3 -1
@@ -1167,7 +1167,7 @@ pvscsi_reset(DeviceState *dev)pvscsi_reset_adapter (s); } -static void +static int pvscsi_pre_save (void *opaque) {PVSCSIState *s = (PVSCSIState *) opaque;
@@ -1176,6 +1176,8 @@ pvscsi_pre_save(void *opaque)assert (QTAILQ_EMPTY (&s->pending_queue));assert (QTAILQ_EMPTY (&s->completion_queue)); + +return 0 ; } static int
hw/timer/cadence_ttc.c +3 -1
@@ -421,9 +421,11 @@ static void cadence_ttc_init(Object *obj)sysbus_init_mmio (SYS_BUS_DEVICE (obj), &s->iomem); } -static void cadence_timer_pre_save (void *opaque) +static int cadence_timer_pre_save (void *opaque) {cadence_timer_sync ((CadenceTimerState *)opaque); + +return 0 ; }static int cadence_timer_post_load (void *opaque,int version_id)
hw/timer/hpet.c +3 -1
@@ -216,12 +216,14 @@ 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)
hw/timer/i8254_common.c +3 -1
@@ -237,7 +237,7 @@ static int pit_load_old(QEMUFile *f, void *opaque, int version_id)return 0 ; } -static void pit_dispatch_pre_save (void *opaque) +static int pit_dispatch_pre_save (void *opaque) {PITCommonState *s = opaque;PITCommonClass *c =PIT_COMMON_GET_CLASS (s);
@@ -245,6 +245,8 @@ static void pit_dispatch_pre_save(void *opaque)if (c->pre_save) { c->pre_save (s); } + +return 0 ; }static int pit_dispatch_post_load (void *opaque,int version_id)
hw/timer/mc146818rtc.c +3 -1
@@ -795,11 +795,13 @@ static void rtc_set_date_from_host(ISADevice *dev)rtc_set_cmos (s, &tm); } -static void rtc_pre_save (void *opaque) +static int rtc_pre_save (void *opaque) {RTCState *s = opaque;rtc_update_time (s); + +return 0 ; }static int rtc_post_load (void *opaque,int version_id)
hw/timer/pl031.c +3 -1
@@ -211,7 +211,7 @@ static void pl031_init(Object *obj) s->timer =timer_new_ns (rtc_clock, pl031_interrupt, s); } -static void pl031_pre_save (void *opaque) +static int pl031_pre_save (void *opaque) {PL031State *s = opaque;
@@ -219,6 +219,8 @@ static void pl031_pre_save(void *opaque)* store the base time relative to the QEMU_CLOCK_VIRTUAL for backwards-compatibility. */ int64_t delta =qemu_clock_get_ns (rtc_clock) -qemu_clock_get_ns (QEMU_CLOCK_VIRTUAL ); s->tick_offset_vmstate = s->tick_offset + delta /NANOSECONDS_PER_SECOND ; + +return 0 ; }static int pl031_post_load (void *opaque,int version_id)
hw/timer/twl92230.c +3 -1
@@ -791,11 +791,13 @@ 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_clock_get_ms (rtc_clock); + +return 0 ; }static int menelaus_post_load (void *opaque,int version_id)
hw/usb/dev-smartcard-reader.c +3 -1
@@ -1374,7 +1374,7 @@ static int ccid_post_load(void *opaque, int version_id)return 0 ; } -static void ccid_pre_save (void *opaque) +static int ccid_pre_save (void *opaque) {USBCCIDState *s = opaque;
@@ -1386,6 +1386,8 @@ static void ccid_pre_save(void *opaque)*/ s->migration_state =MIGRATION_MIGRATED ; } + +return 0 ; }static VMStateDescription bulk_in_vmstate = {
hw/usb/hcd-ehci.c +3 -1
@@ -2381,7 +2381,7 @@ static USBBusOps ehci_bus_ops_standalone = { .wakeup_endpoint = ehci_wakeup_endpoint, }; -static void usb_ehci_pre_save (void *opaque) +static int usb_ehci_pre_save (void *opaque) {EHCIState *ehci = opaque;uint32_t new_frindex;
@@ -2390,6 +2390,8 @@ static void usb_ehci_pre_save(void *opaque) new_frindex = ehci->frindex & ~7 ; ehci->last_run_ns -= (ehci->frindex - new_frindex) *UFRAME_TIMER_NS ; ehci->frindex = new_frindex; + +return 0 ; }static int usb_ehci_post_load (void *opaque,int version_id)
hw/usb/redirect.c +3 -1
@@ -2111,11 +2111,13 @@ static void usbredir_buffered_bulk_packet(void *priv, uint64_t id,* Migration code */ -static void usbredir_pre_save (void *priv) +static int usbredir_pre_save (void *priv) {USBRedirDevice *dev = priv;usbredir_fill_already_in_flight (dev); + +return 0 ; }static int usbredir_post_load (void *priv,int version_id)
hw/virtio/vhost-vsock.c +3 -1
@@ -254,13 +254,15 @@ static void vhost_vsock_post_load_timer_cb(void *opaque)vhost_vsock_send_transport_reset (vsock); } -static void vhost_vsock_pre_save (void *opaque) +static int vhost_vsock_pre_save (void *opaque) {VHostVSock *vsock = opaque;/* At this point, backend must be stopped, otherwise * it might keep writing to memory. */ assert (!vsock->vhost_dev.started); + +return 0 ; }static int vhost_vsock_post_load (void *opaque,int version_id)
include/migration/vmstate.h +1 -1
@@ -179,7 +179,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);bool (*needed)(void *opaque);VMStateField *fields;const VMStateDescription **subsections;
migration/colo-comm.c +3 -1
@@ -34,11 +34,13 @@ COLOMode get_colo_mode(void) } } -static void colo_info_pre_save (void *opaque) +static int colo_info_pre_save (void *opaque) {COLOInfo *s = opaque; s->colo_requested =migrate_colo_enabled (); + +return 0 ; }static bool colo_info_need (void *opaque)
migration/global_state.c +3 -1
@@ -101,12 +101,14 @@ static int global_state_post_load(void *opaque, int version_id)return 0 ; } -static void global_state_pre_save (void *opaque) +static int global_state_pre_save (void *opaque) {GlobalState *s = opaque;trace_migrate_global_state_pre_save ((char *)s->runstate); s->size =strlen ((char *)s->runstate) +1 ; + +return 0 ; }static const VMStateDescription vmstate_globalstate = {
migration/savevm.c +3 -1
@@ -312,7 +312,7 @@ static SaveState savevm_state = { .global_section_id = 0, }; -static void configuration_pre_save (void *opaque) +static int configuration_pre_save (void *opaque) {SaveState *state = opaque;const char *current_name =MACHINE_GET_CLASS (current_machine)->name;
@@ -320,6 +320,8 @@ static void configuration_pre_save(void *opaque) state->len =strlen (current_name); state->name = current_name; state->target_page_bits =qemu_target_page_bits (); + +return 0 ; }static int configuration_pre_load (void *opaque)
replay/replay-snapshot.c +3 -1
@@ -21,10 +21,12 @@#include "migration/vmstate.h" #include "migration/snapshot.h" -static void replay_pre_save (void *opaque) +static int replay_pre_save (void *opaque) {ReplayState *state = opaque; state->file_offset =ftell (replay_file); + +return 0 ; }static int replay_post_load (void *opaque,int version_id)
slirp/slirp.c +6 -2
@@ -1203,11 +1203,13 @@ struct sbuf_tmp {uint32_t roff, woff; }; -static void sbuf_tmp_pre_save (void *opaque) +static int sbuf_tmp_pre_save (void *opaque) {struct sbuf_tmp *tmp = opaque; tmp->woff = tmp->parent->sb_wptr - tmp->parent->sb_data; tmp->roff = tmp->parent->sb_rptr - tmp->parent->sb_data; + +return 0 ; }static int sbuf_tmp_post_load (void *opaque,int version)
@@ -1303,7 +1305,7 @@ typedef struct SS_FamilyTmpStruct {#define SS_FAMILY_MIG_IPV6 10/* Linux */ #define SS_FAMILY_MIG_OTHER 0xffff -static void ss_family_pre_save (void *opaque) +static int ss_family_pre_save (void *opaque) {SS_FamilyTmpStruct *tss = opaque;
@@ -1314,6 +1316,8 @@ static void ss_family_pre_save(void *opaque) }else if (tss->parent->ss.ss_family ==AF_INET6 ) { tss->portable_family =SS_FAMILY_MIG_IPV6 ; } + +return 0 ; }static int ss_family_post_load (void *opaque,int version_id)
target/arm/machine.c +3 -1
@@ -394,7 +394,7 @@ static const VMStateInfo vmstate_powered_off = { .put = put_power, }; -static void cpu_pre_save (void *opaque) +static int cpu_pre_save (void *opaque) {ARMCPU *cpu = opaque;
@@ -415,6 +415,8 @@ static void cpu_pre_save(void *opaque) cpu->cpreg_array_len * sizeof(uint64_t));memcpy (cpu->cpreg_vmstate_values, cpu->cpreg_values, cpu->cpreg_array_len *sizeof (uint64_t )); + +return 0 ; }static int cpu_post_load (void *opaque,int version_id)
target/i386/machine.c +5 -2
@@ -160,13 +160,15 @@ static floatx80 cpu_set_fp80(uint64_t mant, uint16_t upper)return temp.d; } -static void fpreg_pre_save (void *opaque) +static int fpreg_pre_save (void *opaque) {x86_FPReg_tmp *tmp = opaque;/* we save the real CPU data (in case of MMX usage only 'mant' contains the MMX register */ cpu_get_fp80 (&tmp->tmp_mant, &tmp->tmp_exp, tmp->parent->d); + +return 0 ; }static int fpreg_post_load (void *opaque,int version)
@@ -196,7 +198,7 @@ static const VMStateDescription vmstate_fpreg = { } }; -static void cpu_pre_save (void *opaque) +static int cpu_pre_save (void *opaque) {X86CPU *cpu = opaque;CPUX86State *env = &cpu->env;
@@ -228,6 +230,7 @@ static void cpu_pre_save(void *opaque) env->segs[R_SS ].flags &= ~(env->segs[R_SS ].flags &DESC_DPL_MASK ); } +return 0 ; }static int cpu_post_load (void *opaque,int version_id)
target/ppc/machine.c +3 -1
@@ -146,7 +146,7 @@ static bool cpu_pre_2_8_migration(void *opaque, int version_id)return cpu->pre_2_8_migration; } -static void cpu_pre_save (void *opaque) +static int cpu_pre_save (void *opaque) {PowerPCCPU *cpu = opaque;CPUPPCState *env = &cpu->env;
@@ -195,6 +195,8 @@ static void cpu_pre_save(void *opaque) cpu->mig_insns_flags2 = env->insns_flags2 & insns_compat_mask2; cpu->mig_nb_BATs = env->nb_BATs; } + +return 0 ; }/*
target/s390x/machine.c +3 -1
@@ -37,13 +37,15 @@ static int cpu_post_load(void *opaque, int version_id)return 0 ; } -static void cpu_pre_save (void *opaque) +static int cpu_pre_save (void *opaque) {S390CPU *cpu = opaque;if (kvm_enabled ()) {kvm_s390_vcpu_interrupt_pre_save (cpu); } + +return 0 ; }static inline bool fpu_needed (void *opaque)
target/sparc/machine.c +3 -1
@@ -88,7 +88,7 @@ static const VMStateInfo vmstate_psr = { .put = put_psr, }; -static void cpu_pre_save (void *opaque) +static int cpu_pre_save (void *opaque) {SPARCCPU *cpu = opaque;CPUSPARCState *env = &cpu->env;
@@ -97,6 +97,8 @@ static void cpu_pre_save(void *opaque)* window as the outs of the first window */ cpu_set_cwp (env, env->cwp); + +return 0 ; }/* 32-bit SPARC retains migration compatibility with older versions
tests/test-vmstate.c +3 -1
@@ -765,11 +765,13 @@ typedef struct TmpTestStruct {int64_t diff; } TmpTestStruct; -static void tmp_child_pre_save (void *opaque) +static int tmp_child_pre_save (void *opaque) {struct TmpTestStruct *tts = opaque; tts->diff = tts->parent->b - tts->parent->a; + +return 0 ; }static int tmp_child_post_load (void *opaque,int version_id)
Signature · 1 line
2.13.5
Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
Quoted 2 lines · Dr. David Alan Gilbert (git)
Modify the
pre_savemethod on VMStateDescription to return an int rather than void so that it potentially can fail.
What does the int value signify? Why not a bool?
r~
Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
Quoted 5 lines · * Richard Henderson ([email protected]) wrote:
Quoted 2 lines · Dr. David Alan Gilbert (git)
Modify the
pre_savemethod on VMStateDescription to return an int rather than void so that it potentially can fail.What does the int value signify?
0 is success, anything else is a failure; typically -errno.
Quoted 1 line
Why not a bool?
Consistency with pre_load, post_load, put and get methods that are already int.
Dave
Quoted 2 lines
r~
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
Quoted 4 lines · Dr. David Alan Gilbert
Quoted 1 line
Why not a bool?
Consistency with
pre_load,post_load, put and get methods that are already int.
Fair enough, thanks.
r~
Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
In reply to Dr. David Alan Gilbert (git)Quoted 5 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Modify the
pre_savemethod on VMStateDescription to return an int rather than void so that it potentially can fail.
Quoted 8 lines
Changed zillions of devices to make them return 0; the only case I've made it return non-0 is
hw/intc/s390_flic_kvm.cthat already had an error_report/return case.Note: If you add an error exit in your
pre_saveyou must emit anerror_reportto say why.Signed-off-by Dr. David Alan Gilbert <[email protected]>
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
In reply to Dr. David Alan Gilbert (git)Quoted 66 lines · On Mon, 25 Sep 2017 12:29:12 +0100 "Dr. David Alan Gilbert (git)"…
From: "Dr. David Alan Gilbert" <[email protected]>
Modify the
pre_savemethod on VMStateDescription to return an int rather than void so that it potentially can fail.Changed zillions of devices to make them return 0; the only case I've made it return non-0 is
hw/intc/s390_flic_kvm.cthat already had an error_report/return case.Note: If you add an error exit in your
pre_saveyou must emit anerror_reportto say why.Signed-off-by Dr. David Alan Gilbert <[email protected]>--- docs/devel/migration.txt | 2 +- hw/arm/pxa2xx.c | 4 +++- hw/arm/strongarm.c | 4 +++- hw/audio/wm8750.c | 4 +++- hw/block/fdc.c | 4 +++- hw/block/m25p80.c | 4 +++- hw/block/nand.c | 4 +++- hw/block/onenand.c | 4 +++- hw/char/serial.c | 4 +++- hw/display/qxl.c | 4 +++- hw/i2c/core.c | 4 +++- hw/i386/kvm/clock.c | 4 +++- hw/ide/core.c | 4 +++- hw/ide/pci.c | 4 +++- hw/input/ps2.c | 8 ++++++-- hw/input/tsc210x.c | 4 +++- hw/intc/apic_common.c | 4 +++- hw/intc/arm_gic_common.c | 4 +++- hw/intc/arm_gicv3_common.c | 4 +++- hw/intc/arm_gicv3_its_common.c | 4 +++- hw/intc/i8259_common.c | 4 +++- hw/intc/ioapic_common.c | 4 +++- hw/intc/s390_flic_kvm.c | 6 ++++-- hw/intc/xics.c | 8 ++++++-- hw/net/e1000.c | 4 +++- hw/net/e1000e.c | 4 +++- hw/net/rtl8139.c | 4 +++- hw/net/virtio-net.c | 16 ++++++++++++---- hw/net/vmxnet3.c | 4 +++- hw/pci-host/piix.c | 4 +++- hw/ppc/ppc.c | 4 +++- hw/ppc/spapr_iommu.c | 4 +++- hw/ppc/spapr_pci.c | 6 ++++-- hw/s390x/css.c | 10 +++++++--- hw/s390x/virtio-ccw.c | 4 +++- hw/scsi/lsi53c895a.c | 4 +++- hw/scsi/vmw_pvscsi.c | 4 +++- hw/timer/cadence_ttc.c | 4 +++- hw/timer/hpet.c | 4 +++- hw/timer/i8254_common.c | 4 +++- hw/timer/mc146818rtc.c | 4 +++- hw/timer/pl031.c | 4 +++- hw/timer/twl92230.c | 4 +++- hw/usb/dev-smartcard-reader.c | 4 +++- hw/usb/hcd-ehci.c | 4 +++- hw/usb/redirect.c | 4 +++- hw/virtio/vhost-vsock.c | 4 +++- include/migration/vmstate.h | 2 +- migration/colo-comm.c | 4 +++- migration/global_state.c | 4 +++- migration/savevm.c | 4 +++- replay/replay-snapshot.c | 4 +++-
Quoted 8 lines
slirp/slirp.c | 8 ++++++-- target/arm/machine.c | 4 +++- target/i386/machine.c | 7 +++++-- target/ppc/machine.c | 4 +++- target/s390x/machine.c | 4 +++- target/sparc/machine.c | 4 +++- tests/test-vmstate.c | 4 +++- 59 files changed, 199 insertions(+), 70 deletions(-)
Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
In reply to Dr. David Alan Gilbert (git)Quoted 5 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Modify the
pre_savemethod on VMStateDescription to return an int rather than void so that it potentially can fail.
Quoted 8 lines
Changed zillions of devices to make them return 0; the only case I've made it return non-0 is
hw/intc/s390_flic_kvm.cthat already had an error_report/return case.Note: If you add an error exit in your
pre_saveyou must emit anerror_reportto say why.Signed-off-by Dr. David Alan Gilbert <[email protected]>
PATCH v2 2/6 migration: check pre_save return in vmstate_save_state
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Check the return value of pre_save state and fail vmstate_save_state if the pre_save failed.
--- include/migration/vmstate.h | 4 ++-- migration/trace-events | 1 + migration/vmstate.c | 12 ++++++++++-- 3 files changed, 13 insertions(+), 4 deletions(-)
include/migration/vmstate.h +2 -2
@@ -994,8 +994,8 @@ extern const VMStateInfo vmstate_info_qtailq;int vmstate_load_state (QEMUFile *f,const VMStateDescription *vmsd,void *opaque,int version_id); -void vmstate_save_state (QEMUFile *f,const VMStateDescription *vmsd, -void *opaque,QJSON *vmdesc); +int vmstate_save_state (QEMUFile *f,const VMStateDescription *vmsd, +void *opaque,QJSON *vmdesc);bool vmstate_save_needed (const VMStateDescription *vmsd,void *opaque);
migration/trace-events +1 -0
@@ -40,6 +40,7 @@ savevm_state_iterate(void) "" savevm_state_cleanup(void) "" savevm_state_complete_precopy(void) "" vmstate_save(const char *idstr, const char *vmsd_name) "%s, %s" +vmstate_save_state_pre_save_res(const char *name, int res) "%s/%d" vmstate_save_state_loop(const char *name, const char *field, int n_elems) "%s/%s[%d]" vmstate_save_state_top(const char *idstr) "%s" vmstate_subsection_save_loop(const char *name, const char *sub) "%s/%s"
migration/vmstate.c +10 -2
@@ -308,15 +308,21 @@ bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque) } -void vmstate_save_state (QEMUFile *f,const VMStateDescription *vmsd, +int vmstate_save_state (QEMUFile *f,const VMStateDescription *vmsd,void *opaque,QJSON *vmdesc) { +int ret =0 ;VMStateField *field = vmsd->fields;trace_vmstate_save_state_top (vmsd->name);if (vmsd->pre_save) { - vmsd->pre_save (opaque); + ret = vmsd->pre_save (opaque); +trace_vmstate_save_state_pre_save_res (vmsd->name, ret); +if (ret) { +error_report ("pre-save failed: %s" , vmsd->name); +return ret; + } }if (vmdesc) {
@@ -381,6 +387,8 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, }vmstate_subsection_save (f, vmsd, opaque, vmdesc); + +return 0 ; } static const VMStateDescription *
Signature · 1 line
2.13.5
Re: [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state
Quoted 6 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Check the return value of
pre_savestate and failvmstate_save_stateif thepre_savefailed.Signed-off-by Dr. David Alan Gilbert <[email protected]>
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state
In reply to Dr. David Alan Gilbert (git)Quoted 11 lines · On Mon, 25 Sep 2017 12:29:13 +0100 "Dr. David Alan Gilbert (git)"…
From: "Dr. David Alan Gilbert" <[email protected]>
Check the return value of
pre_savestate and failvmstate_save_stateif thepre_savefailed.Signed-off-by Dr. David Alan Gilbert <[email protected]>--- include/migration/vmstate.h | 4 ++-- migration/trace-events | 1 + migration/vmstate.c | 12 ++++++++++-- 3 files changed, 13 insertions(+), 4 deletions(-)
Re: [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state
In reply to Dr. David Alan Gilbert (git)Quoted 6 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Check the return value of
pre_savestate and failvmstate_save_stateif thepre_savefailed.Signed-off-by Dr. David Alan Gilbert <[email protected]>
PATCH v2 3/6 migration: Check field save returns
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Check the return values from vmstate_save_state for fields and also the return values from 'put' for fields that use that.
--- migration/vmstate.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
migration/vmstate.c +12 -3
@@ -347,6 +347,7 @@ int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, }for (i =0 ; i < n_elems; i++) {void *curr_elem = first_elem + size * i; + ret =0 ;vmsd_desc_field_start (vmsd, vmdesc_loop, field, i, n_elems); old_offset =qemu_ftell_fast (f);
@@ -357,11 +358,19 @@ int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,if (!curr_elem && size) {/* if null pointer write placeholder and do not follow */ assert (field->flags &VMS_ARRAY_OF_POINTER ); - vmstate_info_nullptr.put (f, curr_elem, size,NULL ,NULL ); + ret = vmstate_info_nullptr.put (f, curr_elem, size,NULL , +NULL ); }else if (field->flags &VMS_STRUCT ) { -vmstate_save_state (f, field->vmsd, curr_elem, vmdesc_loop); + ret =vmstate_save_state (f, field->vmsd, curr_elem, + vmdesc_loop); }else { - field->info->put (f, curr_elem, size, field, vmdesc_loop); + ret = field->info->put (f, curr_elem, size, field, + vmdesc_loop); + } +if (ret) { +error_report ("Save of field %s/%s failed" , + vmsd->name, field->name); +return ret; } written_bytes =qemu_ftell_fast (f) - old_offset;
Signature · 1 line
2.13.5
Re: [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns
Quoted 31 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Check the return values from
vmstate_save_statefor fields and also the return values from 'put' for fields that use that.Signed-off-by Dr. David Alan Gilbert <[email protected]>--- migration/vmstate.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)migration/vmstate.c +3
-2@@ -347,6 +347,7 @@ int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, }for (i =0 ; i < n_elems; i++) {void *curr_elem = first_elem + size * i; + ret =0 ;vmsd_desc_field_start (vmsd, vmdesc_loop, field, i, n_elems); old_offset =qemu_ftell_fast (f);@@ -357,11 +358,19 @@ int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,if (!curr_elem && size) {/* if null pointer write placeholder and do not follow */ assert (field->flags &VMS_ARRAY_OF_POINTER ); - vmstate_info_nullptr.put (f, curr_elem, size,NULL ,NULL ); + ret = vmstate_info_nullptr.put (f, curr_elem, size,NULL , +NULL ); }else if (field->flags &VMS_STRUCT ) { -vmstate_save_state (f, field->vmsd, curr_elem, vmdesc_loop);
migration/vmstate.c int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd, · line 372
Quoted 10 lines
migration/vmstate.c +8
-1+ ret =vmstate_save_state (f, field->vmsd, curr_elem, + vmdesc_loop); }else { - field->info->put (f, curr_elem, size, field, vmdesc_loop); + ret = field->info->put (f, curr_elem, size, field, + vmdesc_loop); + } +if (ret) { +error_report ("Save of field %s/%s failed" , + vmsd->name, field->name);
I would prefer to prefix the function name (also there is one error_report in previous patch) but that's optional.
Quoted 7 lines
migration/vmstate.c +1
-0+return ret; } written_bytes =qemu_ftell_fast (f) - old_offset;Signature · 1 line
2.13.5
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns
In reply to Dr. David Alan Gilbert (git)Quoted 9 lines · On Mon, 25 Sep 2017 12:29:14 +0100 "Dr. David Alan Gilbert (git)"…
From: "Dr. David Alan Gilbert" <[email protected]>
Check the return values from
vmstate_save_statefor fields and also the return values from 'put' for fields that use that.Signed-off-by Dr. David Alan Gilbert <[email protected]>--- migration/vmstate.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
Re: [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns
In reply to Dr. David Alan Gilbert (git)Quoted 6 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Check the return values from
vmstate_save_statefor fields and also the return values from 'put' for fields that use that.Signed-off-by Dr. David Alan Gilbert <[email protected]>
PATCH v2 4/6 migration: wire vmstate_save_state errors up to vmstate_subsection_save
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Route the errors from vmstate_subsection_save up through vmstate_subsection_save (and back down, all rather recursive).
--- migration/vmstate.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
migration/vmstate.c +12 -8
@@ -21,8 +21,8 @@#include "trace.h" #include "qjson.h" -static void vmstate_subsection_save (QEMUFile *f,const VMStateDescription *vmsd, -void *opaque,QJSON *vmdesc); +static int vmstate_subsection_save (QEMUFile *f,const VMStateDescription *vmsd, +void *opaque,QJSON *vmdesc);static int vmstate_subsection_load (QEMUFile *f,const VMStateDescription *vmsd,void *opaque);
@@ -395,9 +395,7 @@ int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,json_end_array (vmdesc); } -vmstate_subsection_save (f, vmsd, opaque, vmdesc); - -return 0 ; +return vmstate_subsection_save (f, vmsd, opaque, vmdesc); } static const VMStateDescription *
@@ -463,11 +461,12 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,return 0 ; } -static void vmstate_subsection_save (QEMUFile *f,const VMStateDescription *vmsd, -void *opaque,QJSON *vmdesc) +static int vmstate_subsection_save (QEMUFile *f,const VMStateDescription *vmsd, +void *opaque,QJSON *vmdesc) {const VMStateDescription **sub = vmsd->subsections;bool subsection_found = false; +int ret =0 ;trace_vmstate_subsection_save_top (vmsd->name);while (sub && *sub && (*sub)->needed) {
@@ -491,7 +490,10 @@ static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,qemu_put_byte (f, len);qemu_put_buffer (f, (uint8_t *)vmsdsub->name, len);qemu_put_be32 (f, vmsdsub->version_id); -vmstate_save_state (f, vmsdsub, opaque, vmdesc); + ret =vmstate_save_state (f, vmsdsub, opaque, vmdesc); +if (ret) { +return ret; + }if (vmdesc) {json_end_object (vmdesc);
@@ -503,4 +505,6 @@ static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,if (vmdesc && subsection_found) {json_end_array (vmdesc); } + +return ret; }
Signature · 1 line
2.13.5
Re: [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save
Quoted 4 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Route the errors from
vmstate_subsection_saveup throughvmstate_subsection_save(and back down, all rather recursive).
I guess here one of the "vmstate_subsection_save" should be replaced by "vmstate_save_state"? :-)
Quoted 2 lines
Signed-off-by Dr. David Alan Gilbert <[email protected]>
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save
Quoted 8 lines · * Peter Xu ([email protected]) wrote:
Quoted 4 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Route the errors from
vmstate_subsection_saveup throughvmstate_subsection_save(and back down, all rather recursive).I guess here one of the "
vmstate_subsection_save" should be replaced by "vmstate_save_state"? :-)
Ah yes, fixed.
Quoted 4 lines
Quoted 2 lines
Signed-off-by Dr. David Alan Gilbert <[email protected]>Reviewed-by Peter Xu <[email protected]>
Thanks.
Dave
Quoted 3 lines
Signature · 2 lines
-- Peter Xu
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save
In reply to Dr. David Alan Gilbert (git)Quoted 9 lines · On Mon, 25 Sep 2017 12:29:15 +0100 "Dr. David Alan Gilbert (git)"…
From: "Dr. David Alan Gilbert" <[email protected]>
Route the errors from
vmstate_subsection_saveup throughvmstate_subsection_save(and back down, all rather recursive).Signed-off-by Dr. David Alan Gilbert <[email protected]>--- migration/vmstate.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
With the patch description fixed up:
Re: [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save
In reply to Dr. David Alan Gilbert (git)Quoted 6 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Route the errors from
vmstate_subsection_saveup throughvmstate_subsection_save(and back down, all rather recursive).Signed-off-by Dr. David Alan Gilbert <[email protected]>
PATCH v2 5/6 migration: Route errors up through vmstate_save
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
Route the errors from vsmtate_save_state back up through vmstate_save and out to the normal device state path. That's the normal error path done.
--- migration/savevm.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
migration/savevm.c +14 -5
@@ -768,14 +768,14 @@ static void vmstate_save_old_style(QEMUFile *f, SaveStateEntry *se, QJSON *vmdes } } -static void vmstate_save (QEMUFile *f,SaveStateEntry *se,QJSON *vmdesc) +static int vmstate_save (QEMUFile *f,SaveStateEntry *se,QJSON *vmdesc) {trace_vmstate_save (se->idstr, se->vmsd ? se->vmsd->name :"(old)" );if (!se->vmsd) {vmstate_save_old_style (f, se, vmdesc); -return ; +return 0 ; } -vmstate_save_state (f, se->vmsd, se->opaque, vmdesc); +return vmstate_save_state (f, se->vmsd, se->opaque, vmdesc); }/*
@@ -1171,7 +1171,11 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,json_prop_int (vmdesc,"instance_id" , se->instance_id);save_section_header (f, se,QEMU_VM_SECTION_FULL ); -vmstate_save (f, se, vmdesc); + ret =vmstate_save (f, se, vmdesc); +if (ret) { +qemu_file_set_error (f, ret); +return ret; + }trace_savevm_section_end (se->idstr, se->section_id,0 );save_section_footer (f, se);
@@ -1313,6 +1317,8 @@ static int qemu_save_device_state(QEMUFile *f)cpu_synchronize_all_states (); QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { +int ret; +if (se->is_ram) {continue ; }
@@ -1325,7 +1331,10 @@ static int qemu_save_device_state(QEMUFile *f)save_section_header (f, se,QEMU_VM_SECTION_FULL ); -vmstate_save (f, se,NULL ); + ret =vmstate_save (f, se,NULL ); +if (ret) { +return ret; + }save_section_footer (f, se); }
Signature · 1 line
2.13.5
Re: [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save
Quoted 7 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Route the errors from
vsmtate_save_stateback up throughvmstate_saveand out to the normal device state path. That's the normal error path done.Signed-off-by Dr. David Alan Gilbert <[email protected]>
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save
In reply to Dr. David Alan Gilbert (git)Quoted 10 lines · On Mon, 25 Sep 2017 12:29:16 +0100 "Dr. David Alan Gilbert (git)"…
From: "Dr. David Alan Gilbert" <[email protected]>
Route the errors from
vsmtate_save_stateback up throughvmstate_saveand out to the normal device state path. That's the normal error path done.Signed-off-by Dr. David Alan Gilbert <[email protected]>--- migration/savevm.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
Re: [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save
In reply to Dr. David Alan Gilbert (git)Quoted 7 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
Route the errors from
vsmtate_save_stateback up throughvmstate_saveand out to the normal device state path. That's the normal error path done.Signed-off-by Dr. David Alan Gilbert <[email protected]>
PATCH v2 6/6 migration: Route more error paths
In reply to Dr. David Alan Gilbert (git)From: "Dr. David Alan Gilbert" <[email protected]>
vmstate_save is called in a few places, and vmstate_save_state is called in lots of places.
Route error returns from the easier cases back up; there are lots of more complex cases where their own error paths need fixing.
--- hw/display/virtio-gpu.c | 4 +--- hw/virtio/virtio.c | 13 +++++++------ include/hw/virtio/virtio.h | 2 +- migration/vmstate-types.c | 11 ++++++++--- tests/test-vmstate.c | 9 ++++++--- 5 files changed, 23 insertions(+), 16 deletions(-)
hw/display/virtio-gpu.c +1 -3
@@ -1050,9 +1050,7 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size, }qemu_put_be32 (f,0 );/* end of list */ -vmstate_save_state (f, &vmstate_virtio_gpu_scanouts, g,NULL ); - -return 0 ; +return vmstate_save_state (f, &vmstate_virtio_gpu_scanouts, g,NULL ); } static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
hw/virtio/virtio.c +7 -6
@@ -1897,7 +1897,7 @@ static const VMStateDescription vmstate_virtio = { } }; -void virtio_save (VirtIODevice *vdev,QEMUFile *f) +int virtio_save (VirtIODevice *vdev,QEMUFile *f) {BusState *qbus =qdev_get_parent_bus (DEVICE (vdev));VirtioBusClass *k =VIRTIO_BUS_GET_CLASS (qbus);
@@ -1947,20 +1947,21 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) }if (vdc->vmsd) { -vmstate_save_state (f, vdc->vmsd, vdev,NULL ); +int ret =vmstate_save_state (f, vdc->vmsd, vdev,NULL ); +if (ret) { +return ret; + } }/* Subsections */ -vmstate_save_state (f, &vmstate_virtio, vdev,NULL ); +return vmstate_save_state (f, &vmstate_virtio, vdev,NULL ); }/* A wrapper for use as a VMState .put function */ static int virtio_device_put (QEMUFile *f,void *opaque,size_t size,VMStateField *field,QJSON *vmdesc) { -virtio_save (VIRTIO_DEVICE (opaque), f); - -return 0 ; +return virtio_save (VIRTIO_DEVICE (opaque), f); }/* A wrapper for use as a VMState .get function */
include/hw/virtio/virtio.h +1 -1
@@ -188,7 +188,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,void virtio_notify_irqfd (VirtIODevice *vdev,VirtQueue *vq);void virtio_notify (VirtIODevice *vdev,VirtQueue *vq); -void virtio_save (VirtIODevice *vdev,QEMUFile *f); +int virtio_save (VirtIODevice *vdev,QEMUFile *f);extern const VMStateInfo virtio_vmstate_info;
migration/vmstate-types.c +8 -3
@@ -550,13 +550,14 @@ static int put_tmp(QEMUFile *f, void *pv, size_t size, VMStateField *field, {const VMStateDescription *vmsd = field->vmsd;void *tmp =g_malloc (size); +int ret;/* Writes the parent field which is at the start of the tmp */ *(void **)tmp = pv; -vmstate_save_state (f, vmsd, tmp, vmdesc); + ret =vmstate_save_state (f, vmsd, tmp, vmdesc);g_free (tmp); -return 0 ; +return ret; }const VMStateInfo vmstate_info_tmp = {
@@ -657,12 +658,16 @@ static int put_qtailq(QEMUFile *f, void *pv, size_t unused_size,/* offset of the QTAILQ entry in a QTAILQ element*/ size_t entry_offset = field->start;void *elm; +int ret;trace_put_qtailq (vmsd->name, vmsd->version_id); QTAILQ_RAW_FOREACH(elm, pv, entry_offset) {qemu_put_byte (f, true); -vmstate_save_state (f, vmsd, elm, vmdesc); + ret =vmstate_save_state (f, vmsd, elm, vmdesc); +if (ret) { +return ret; + } }qemu_put_byte (f, false);
tests/test-vmstate.c +6 -3
@@ -70,7 +70,8 @@ static void save_vmstate(const VMStateDescription *desc, void *obj)QEMUFile *f =open_test_file (true);/* Save file with vmstate */ -vmstate_save_state (f, desc, obj,NULL ); +int ret =vmstate_save_state (f, desc, obj,NULL ); +g_assert (!ret);qemu_put_byte (f,QEMU_VM_EOF );g_assert (!qemu_file_get_error (f));qemu_fclose (f);
@@ -381,7 +382,8 @@ static void test_save_noskip(void)QEMUFile *fsave =open_test_file (true);TestStruct obj = { .a =1 , .b =2 , .c =3 , .d =4 , .e =5 , .f =6 , .skip_c_e = false }; -vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +int ret =vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +g_assert (!ret);g_assert (!qemu_file_get_error (fsave));uint8_t expected[] = {
@@ -402,7 +404,8 @@ static void test_save_skip(void)QEMUFile *fsave =open_test_file (true);TestStruct obj = { .a =1 , .b =2 , .c =3 , .d =4 , .e =5 , .f =6 , .skip_c_e = true }; -vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +int ret =vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +g_assert (!ret);g_assert (!qemu_file_get_error (fsave));uint8_t expected[] = {
Signature · 1 line
2.13.5
Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths
Quoted 4 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
vmstate_saveis called in a few places, andvmstate_save_stateis called in lots of places.
vmstate_save() should have been processed with previous patch?
Quoted 5 lines
Route error returns from the easier cases back up; there are lots of more complex cases where their own error paths need fixing.
Signed-off-by Dr. David Alan Gilbert <[email protected]>
The patch content wires quite a few vmstate_save_state() callers, so the patch itself makes sense to me.
Quoted 152 lines
--- hw/display/virtio-gpu.c | 4 +--- hw/virtio/virtio.c | 13 +++++++------ include/hw/virtio/virtio.h | 2 +- migration/vmstate-types.c | 11 ++++++++--- tests/test-vmstate.c | 9 ++++++--- 5 files changed, 23 insertions(+), 16 deletions(-)hw/display/virtio-gpu.c +1
-3@@ -1050,9 +1050,7 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size, }qemu_put_be32 (f,0 );/* end of list */ -vmstate_save_state (f, &vmstate_virtio_gpu_scanouts, g,NULL ); - -return 0 ; +return vmstate_save_state (f, &vmstate_virtio_gpu_scanouts, g,NULL ); } static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,hw/virtio/virtio.c +7
-6@@ -1897,7 +1897,7 @@ static const VMStateDescription vmstate_virtio = { } }; -void virtio_save (VirtIODevice *vdev,QEMUFile *f) +int virtio_save (VirtIODevice *vdev,QEMUFile *f) {BusState *qbus =qdev_get_parent_bus (DEVICE (vdev));VirtioBusClass *k =VIRTIO_BUS_GET_CLASS (qbus);@@ -1947,20 +1947,21 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) }if (vdc->vmsd) { -vmstate_save_state (f, vdc->vmsd, vdev,NULL ); +int ret =vmstate_save_state (f, vdc->vmsd, vdev,NULL ); +if (ret) { +return ret; + } }/* Subsections */ -vmstate_save_state (f, &vmstate_virtio, vdev,NULL ); +return vmstate_save_state (f, &vmstate_virtio, vdev,NULL ); }/* A wrapper for use as a VMState .put function */ static int virtio_device_put (QEMUFile *f,void *opaque,size_t size,VMStateField *field,QJSON *vmdesc) { -virtio_save (VIRTIO_DEVICE (opaque), f); - -return 0 ; +return virtio_save (VIRTIO_DEVICE (opaque), f); }/* A wrapper for use as a VMState .get function */ include/hw/virtio/virtio.h +1
-1@@ -188,7 +188,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,void virtio_notify_irqfd (VirtIODevice *vdev,VirtQueue *vq);void virtio_notify (VirtIODevice *vdev,VirtQueue *vq); -void virtio_save (VirtIODevice *vdev,QEMUFile *f); +int virtio_save (VirtIODevice *vdev,QEMUFile *f);extern const VMStateInfo virtio_vmstate_info;migration/vmstate-types.c +8
-3@@ -550,13 +550,14 @@ static int put_tmp(QEMUFile *f, void *pv, size_t size, VMStateField *field, {const VMStateDescription *vmsd = field->vmsd;void *tmp =g_malloc (size); +int ret;/* Writes the parent field which is at the start of the tmp */ *(void **)tmp = pv; -vmstate_save_state (f, vmsd, tmp, vmdesc); + ret =vmstate_save_state (f, vmsd, tmp, vmdesc);g_free (tmp); -return 0 ; +return ret; }const VMStateInfo vmstate_info_tmp = {@@ -657,12 +658,16 @@ static int put_qtailq(QEMUFile *f, void *pv, size_t unused_size,/* offset of the QTAILQ entry in a QTAILQ element*/ size_t entry_offset = field->start;void *elm; +int ret;trace_put_qtailq (vmsd->name, vmsd->version_id); QTAILQ_RAW_FOREACH(elm, pv, entry_offset) {qemu_put_byte (f, true); -vmstate_save_state (f, vmsd, elm, vmdesc); + ret =vmstate_save_state (f, vmsd, elm, vmdesc); +if (ret) { +return ret; + } }qemu_put_byte (f, false);tests/test-vmstate.c +6
-3@@ -70,7 +70,8 @@ static void save_vmstate(const VMStateDescription *desc, void *obj)QEMUFile *f =open_test_file (true);/* Save file with vmstate */ -vmstate_save_state (f, desc, obj,NULL ); +int ret =vmstate_save_state (f, desc, obj,NULL ); +g_assert (!ret);qemu_put_byte (f,QEMU_VM_EOF );g_assert (!qemu_file_get_error (f));qemu_fclose (f);@@ -381,7 +382,8 @@ static void test_save_noskip(void)QEMUFile *fsave =open_test_file (true);TestStruct obj = { .a =1 , .b =2 , .c =3 , .d =4 , .e =5 , .f =6 , .skip_c_e = false }; -vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +int ret =vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +g_assert (!ret);g_assert (!qemu_file_get_error (fsave));uint8_t expected[] = {@@ -402,7 +404,8 @@ static void test_save_skip(void)QEMUFile *fsave =open_test_file (true);TestStruct obj = { .a =1 , .b =2 , .c =3 , .d =4 , .e =5 , .f =6 , .skip_c_e = true }; -vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +int ret =vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +g_assert (!ret);g_assert (!qemu_file_get_error (fsave));uint8_t expected[] = {Signature · 1 line
2.13.5
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths
Quoted 7 lines · * Peter Xu ([email protected]) wrote:
Quoted 4 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
vmstate_saveis called in a few places, andvmstate_save_stateis called in lots of places.
vmstate_save()should have been processed with previous patch?
Ah yes, it was just vmstate_save_state in the end fixed in this one.
Quoted 10 lines
Quoted 5 lines
Route error returns from the easier cases back up; there are lots of more complex cases where their own error paths need fixing.
Signed-off-by Dr. David Alan Gilbert <[email protected]>The patch content wires quite a few
vmstate_save_state()callers, so the patch itself makes sense to me.Reviewed-by Peter Xu <[email protected]>
Thanks!
Dave
Quoted 148 lines
Quoted 148 lines
--- hw/display/virtio-gpu.c | 4 +--- hw/virtio/virtio.c | 13 +++++++------ include/hw/virtio/virtio.h | 2 +- migration/vmstate-types.c | 11 ++++++++--- tests/test-vmstate.c | 9 ++++++--- 5 files changed, 23 insertions(+), 16 deletions(-)hw/display/virtio-gpu.c +1
-3@@ -1050,9 +1050,7 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size, }qemu_put_be32 (f,0 );/* end of list */ -vmstate_save_state (f, &vmstate_virtio_gpu_scanouts, g,NULL ); - -return 0 ; +return vmstate_save_state (f, &vmstate_virtio_gpu_scanouts, g,NULL ); } static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,hw/virtio/virtio.c +7
-6@@ -1897,7 +1897,7 @@ static const VMStateDescription vmstate_virtio = { } }; -void virtio_save (VirtIODevice *vdev,QEMUFile *f) +int virtio_save (VirtIODevice *vdev,QEMUFile *f) {BusState *qbus =qdev_get_parent_bus (DEVICE (vdev));VirtioBusClass *k =VIRTIO_BUS_GET_CLASS (qbus);@@ -1947,20 +1947,21 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) }if (vdc->vmsd) { -vmstate_save_state (f, vdc->vmsd, vdev,NULL ); +int ret =vmstate_save_state (f, vdc->vmsd, vdev,NULL ); +if (ret) { +return ret; + } }/* Subsections */ -vmstate_save_state (f, &vmstate_virtio, vdev,NULL ); +return vmstate_save_state (f, &vmstate_virtio, vdev,NULL ); }/* A wrapper for use as a VMState .put function */ static int virtio_device_put (QEMUFile *f,void *opaque,size_t size,VMStateField *field,QJSON *vmdesc) { -virtio_save (VIRTIO_DEVICE (opaque), f); - -return 0 ; +return virtio_save (VIRTIO_DEVICE (opaque), f); }/* A wrapper for use as a VMState .get function */ include/hw/virtio/virtio.h +1
-1@@ -188,7 +188,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,void virtio_notify_irqfd (VirtIODevice *vdev,VirtQueue *vq);void virtio_notify (VirtIODevice *vdev,VirtQueue *vq); -void virtio_save (VirtIODevice *vdev,QEMUFile *f); +int virtio_save (VirtIODevice *vdev,QEMUFile *f);extern const VMStateInfo virtio_vmstate_info;migration/vmstate-types.c +8
-3@@ -550,13 +550,14 @@ static int put_tmp(QEMUFile *f, void *pv, size_t size, VMStateField *field, {const VMStateDescription *vmsd = field->vmsd;void *tmp =g_malloc (size); +int ret;/* Writes the parent field which is at the start of the tmp */ *(void **)tmp = pv; -vmstate_save_state (f, vmsd, tmp, vmdesc); + ret =vmstate_save_state (f, vmsd, tmp, vmdesc);g_free (tmp); -return 0 ; +return ret; }const VMStateInfo vmstate_info_tmp = {@@ -657,12 +658,16 @@ static int put_qtailq(QEMUFile *f, void *pv, size_t unused_size,/* offset of the QTAILQ entry in a QTAILQ element*/ size_t entry_offset = field->start;void *elm; +int ret;trace_put_qtailq (vmsd->name, vmsd->version_id); QTAILQ_RAW_FOREACH(elm, pv, entry_offset) {qemu_put_byte (f, true); -vmstate_save_state (f, vmsd, elm, vmdesc); + ret =vmstate_save_state (f, vmsd, elm, vmdesc); +if (ret) { +return ret; + } }qemu_put_byte (f, false);tests/test-vmstate.c +6
-3@@ -70,7 +70,8 @@ static void save_vmstate(const VMStateDescription *desc, void *obj)QEMUFile *f =open_test_file (true);/* Save file with vmstate */ -vmstate_save_state (f, desc, obj,NULL ); +int ret =vmstate_save_state (f, desc, obj,NULL ); +g_assert (!ret);qemu_put_byte (f,QEMU_VM_EOF );g_assert (!qemu_file_get_error (f));qemu_fclose (f);@@ -381,7 +382,8 @@ static void test_save_noskip(void)QEMUFile *fsave =open_test_file (true);TestStruct obj = { .a =1 , .b =2 , .c =3 , .d =4 , .e =5 , .f =6 , .skip_c_e = false }; -vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +int ret =vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +g_assert (!ret);g_assert (!qemu_file_get_error (fsave));uint8_t expected[] = {@@ -402,7 +404,8 @@ static void test_save_skip(void)QEMUFile *fsave =open_test_file (true);TestStruct obj = { .a =1 , .b =2 , .c =3 , .d =4 , .e =5 , .f =6 , .skip_c_e = true }; -vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +int ret =vmstate_save_state (fsave, &vmstate_skipping, &obj,NULL ); +g_assert (!ret);g_assert (!qemu_file_get_error (fsave));
Quoted 7 lines
Quoted 4 lines
tests/test-vmstate.c +0
-0uint8_t expected[] = {Signature · 1 line
2.13.5Signature · 1 line
Peter Xu
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths
In reply to Dr. David Alan Gilbert (git)Quoted 8 lines · On Mon, 25 Sep 2017 12:29:17 +0100 "Dr. David Alan Gilbert (git)"…
From: "Dr. David Alan Gilbert" <[email protected]>
vmstate_saveis called in a few places, andvmstate_save_stateis called in lots of places.Route error returns from the easier cases back up; there are lots of more complex cases where their own error paths need fixing.
Quoted 8 lines
Signed-off-by Dr. David Alan Gilbert <[email protected]>--- hw/display/virtio-gpu.c | 4 +--- hw/virtio/virtio.c | 13 +++++++------ include/hw/virtio/virtio.h | 2 +- migration/vmstate-types.c | 11 ++++++++--- tests/test-vmstate.c | 9 ++++++--- 5 files changed, 23 insertions(+), 16 deletions(-)
If you fix up the description to only refer to vmstate_save_state:
Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths
In reply to Dr. David Alan Gilbert (git)Quoted 9 lines · Dr. David Alan Gilbert (git)
From: "Dr. David Alan Gilbert" <[email protected]>
vmstate_saveis called in a few places, andvmstate_save_stateis called in lots of places.Route error returns from the easier cases back up; there are lots of more complex cases where their own error paths need fixing.
Signed-off-by Dr. David Alan Gilbert <[email protected]>
But .....
Quoted 10 lines
hw/virtio/virtio.c +1
-2-void virtio_save (VirtIODevice *vdev,QEMUFile *f) +int virtio_save (VirtIODevice *vdev,QEMUFile *f) {BusState *qbus =qdev_get_parent_bus (DEVICE (vdev));VirtioBusClass *k =VIRTIO_BUS_GET_CLASS (qbus);@@ -1947,20 +1947,21 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) }if (vdc->vmsd) { -vmstate_save_state (f, vdc->vmsd, vdev,NULL );
hw/virtio/virtio.c void virtio_save(VirtIODevice *vdev, QEMUFile *f) · line 1958
Quoted 10 lines
hw/virtio/virtio.c +5
-1+int ret =vmstate_save_state (f, vdc->vmsd, vdev,NULL ); +if (ret) { +return ret; + } }/* Subsections */ -vmstate_save_state (f, &vmstate_virtio, vdev,NULL ); +return vmstate_save_state (f, &vmstate_virtio, vdev,NULL ); }
We add error code, good.
hw/virtio/virtio.c void virtio_save(VirtIODevice *vdev, QEMUFile *f) · line 1965
Quoted 10 lines
hw/virtio/virtio.c +1
-3/* A wrapper for use as a VMState .put function */ static int virtio_device_put (QEMUFile *f,void *opaque,size_t size,VMStateField *field,QJSON *vmdesc) { -virtio_save (VIRTIO_DEVICE (opaque), f); - -return 0 ; +return virtio_save (VIRTIO_DEVICE (opaque), f); }
And we add the error code. But are the callers ready to use it?
...
} else {
field->info->put(f, curr_elem, size, field, vmdesc_loop);
}No, we need to fix it on the callers also. Yeap, that is can be fixed in a posterios series.
Later, Juan.
Quoted 11 lines · * Dr. David Alan Gilbert (git) ([email protected]) wrote:
From: "Dr. David Alan Gilbert" <[email protected]>
At the moment the pre-save method on VMStateDescription returns void which means there's no sane way of a
pre_saveroutine failing. There are quite a few pre_save's that abort or do other nasty things, and it would be better if they could fail cleanly.This patch series doesn't fix any of those pre_save's - it just changes
pre_saveto return an error value and routes that back up so the migration fails. I'll leave that up to the individual device owners.
Quoted 6 lines
There are a bunch of corner cases that I've not fixed yet; these are mostly places that call
vmstate_save_statewhich have been partially converted to modern migration code; often the functions they're called from return void as well - they can get fixed later.Signed-off-by Dr. David Alan Gilbert <[email protected]>
Queued.
Quoted 73 lines
v2
test: Split assert/side effects [Fam]
new patch 3: Changed to also check the return of the ->put methods
that should have been checked previously. [Peter]Dr. David Alan Gilbert (6):
migration:pre_savereturn int
migration: checkpre_savereturn invmstate_save_state
migration: Check field save returns
migration: wirevmstate_save_stateerrors up to
vmstate_subsection_save
migration: Route errors up throughvmstate_save
migration: Route more error pathsdocs/devel/migration.txt | 2 +- hw/arm/pxa2xx.c | 4 +++- hw/arm/strongarm.c | 4 +++- hw/audio/wm8750.c | 4 +++- hw/block/fdc.c | 4 +++- hw/block/m25p80.c | 4 +++- hw/block/nand.c | 4 +++- hw/block/onenand.c | 4 +++- hw/char/serial.c | 4 +++- hw/display/qxl.c | 4 +++- hw/display/virtio-gpu.c | 4 +--- hw/i2c/core.c | 4 +++- hw/i386/kvm/clock.c | 4 +++- hw/ide/core.c | 4 +++- hw/ide/pci.c | 4 +++- hw/input/ps2.c | 8 ++++++-- hw/input/tsc210x.c | 4 +++- hw/intc/apic_common.c | 4 +++- hw/intc/arm_gic_common.c | 4 +++- hw/intc/arm_gicv3_common.c | 4 +++- hw/intc/arm_gicv3_its_common.c | 4 +++- hw/intc/i8259_common.c | 4 +++- hw/intc/ioapic_common.c | 4 +++- hw/intc/s390_flic_kvm.c | 6 ++++-- hw/intc/xics.c | 8 ++++++-- hw/net/e1000.c | 4 +++- hw/net/e1000e.c | 4 +++- hw/net/rtl8139.c | 4 +++- hw/net/virtio-net.c | 16 ++++++++++++---- hw/net/vmxnet3.c | 4 +++- hw/pci-host/piix.c | 4 +++- hw/ppc/ppc.c | 4 +++- hw/ppc/spapr_iommu.c | 4 +++- hw/ppc/spapr_pci.c | 6 ++++-- hw/s390x/css.c | 10 +++++++--- hw/s390x/virtio-ccw.c | 4 +++- hw/scsi/lsi53c895a.c | 4 +++- hw/scsi/vmw_pvscsi.c | 4 +++- hw/timer/cadence_ttc.c | 4 +++- hw/timer/hpet.c | 4 +++- hw/timer/i8254_common.c | 4 +++- hw/timer/mc146818rtc.c | 4 +++- hw/timer/pl031.c | 4 +++- hw/timer/twl92230.c | 4 +++- hw/usb/dev-smartcard-reader.c | 4 +++- hw/usb/hcd-ehci.c | 4 +++- hw/usb/redirect.c | 4 +++- hw/virtio/vhost-vsock.c | 4 +++- hw/virtio/virtio.c | 13 +++++++------ include/hw/virtio/virtio.h | 2 +- include/migration/vmstate.h | 6 +++--- migration/colo-comm.c | 4 +++- migration/global_state.c | 4 +++- migration/savevm.c | 23 ++++++++++++++++------ migration/trace-events | 1 + migration/vmstate-types.c | 11 ++++++++--- migration/vmstate.c | 43 +++++++++++++++++++++++++++++++----------- replay/replay-snapshot.c | 4 +++-
Quoted 13 lines
slirp/slirp.c | 8 ++++++-- target/arm/machine.c | 4 +++- target/i386/machine.c | 7 +++++-- target/ppc/machine.c | 4 +++- target/s390x/machine.c | 4 +++- target/sparc/machine.c | 4 +++- tests/test-vmstate.c | 13 +++++++++---- 65 files changed, 271 insertions(+), 104 deletions(-)Signature · 1 line
2.13.5
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK