* [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail
@ 2017-09-25 11:29 Dr. David Alan Gilbert (git)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int Dr. David Alan Gilbert (git)
` (6 more replies)
0 siblings, 7 replies; 31+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-09-25 11:29 UTC (permalink / raw)
To: qemu-devel, quintela, peterx, famz; +Cc: peter.maydell, cohuck, rth
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
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.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
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(-)
--
2.13.5
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
2017-09-25 11:29 [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert (git)
@ 2017-09-25 11:29 ` Dr. David Alan Gilbert (git)
2017-09-25 13:19 ` Richard Henderson
` (3 more replies)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state Dr. David Alan Gilbert (git)
` (5 subsequent siblings)
6 siblings, 4 replies; 31+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-09-25 11:29 UTC (permalink / raw)
To: qemu-devel, quintela, peterx, famz; +Cc: peter.maydell, cohuck, rth
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
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.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
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(-)
diff --git a/docs/devel/migration.txt b/docs/devel/migration.txt
index 1b940a829b..4030703726 100644
--- a/docs/devel/migration.txt
+++ b/docs/devel/migration.txt
@@ -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.
diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index cf07234578..ab691a7985 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -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)
diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index 3d1a231d9e..4cdb3a670b 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -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)
diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c
index d2bf2e1da1..8bb44a7cc1 100644
--- a/hw/audio/wm8750.c
+++ b/hw/audio/wm8750.c
@@ -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)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 2853cdc205..67f78ac702 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -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)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 190573cefa..a2438b9ed2 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -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[] = {
diff --git a/hw/block/nand.c b/hw/block/nand.c
index 0d33ac281f..76dcd3f76e 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -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)
diff --git a/hw/block/onenand.c b/hw/block/onenand.c
index b7423607d9..30e40f3914 100644
--- a/hw/block/onenand.c
+++ b/hw/block/onenand.c
@@ -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)
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 9aec6c60d8..376bd2f240 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -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)
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index ae3677fd1e..d92fe05f1a 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -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)
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index 2c1234cdff..59068f157e 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -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 = {
diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 363d1b5743..75ad1ba6f0 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -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 = {
diff --git a/hw/ide/core.c b/hw/ide/core.c
index d63eb4a72e..5f1cd3b91f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -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)
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index f2dcc0ed77..d53ff5341c 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -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
diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 77906d5f46..dff3f1e024 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -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 = {
diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index b068343771..75ac8c2ab5 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -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)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index e1ac33042f..78903ea909 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -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)
diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c
index 70f1134823..aee50a20e0 100644
--- a/hw/intc/arm_gic_common.c
+++ b/hw/intc/arm_gic_common.c
@@ -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)
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 410915a2ac..7b54d52376 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -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)
diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c
index 68b20fccd1..f2cce597a9 100644
--- a/hw/intc/arm_gicv3_its_common.c
+++ b/hw/intc/arm_gicv3_its_common.c
@@ -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)
diff --git a/hw/intc/i8259_common.c b/hw/intc/i8259_common.c
index c2fd563b5b..18427b459a 100644
--- a/hw/intc/i8259_common.c
+++ b/hw/intc/i8259_common.c
@@ -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)
diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
index 97c4f9c2df..3b3d0a7680 100644
--- a/hw/intc/ioapic_common.c
+++ b/hw/intc/ioapic_common.c
@@ -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)
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index 7ead17ac3e..d208cb81c4 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -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)
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 80c33be02e..cc9816e7f2 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -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)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index eebe3a9c13..cae95185a5 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -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)
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 6c42b4478c..744f0f3b91 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -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)
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 3be24bbee7..d6c8188ae1 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -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 = {
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 148071a396..150fd0748e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -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 = {
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index a19a7a31dd..b43b58be2b 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -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 = {
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 894e131c00..dec345fd24 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -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)
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index f76886f4d3..05da316e0b 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -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 = {
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index fa8b969840..5ccd785d5a 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -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)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index cf54160526..8c2b6dc1dc 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -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)
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 75d4f301fb..c59be1aad1 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -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)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index ff1bb1534c..62d69aa30b 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -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)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 3e56ab267c..a67ee074d9 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -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 = {
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 6d3f0bf11d..d8079158ac 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -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
diff --git a/hw/timer/cadence_ttc.c b/hw/timer/cadence_ttc.c
index 03f5b9c206..5e65fdb5a0 100644
--- a/hw/timer/cadence_ttc.c
+++ b/hw/timer/cadence_ttc.c
@@ -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)
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index a2c18b30c3..577371bc6d 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -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)
diff --git a/hw/timer/i8254_common.c b/hw/timer/i8254_common.c
index ee064aa819..b623c96198 100644
--- a/hw/timer/i8254_common.c
+++ b/hw/timer/i8254_common.c
@@ -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)
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 82843ed03f..7764be25ec 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -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)
diff --git a/hw/timer/pl031.c b/hw/timer/pl031.c
index dbbeb9b16b..d3aacce80d 100644
--- a/hw/timer/pl031.c
+++ b/hw/timer/pl031.c
@@ -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)
diff --git a/hw/timer/twl92230.c b/hw/timer/twl92230.c
index c0aa8ae3de..eb58c378e0 100644
--- a/hw/timer/twl92230.c
+++ b/hw/timer/twl92230.c
@@ -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)
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index bef1f03c42..0c77d2a41d 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -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 = {
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 46fd30b075..0134232627 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -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)
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 5e42730449..ec174309db 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -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)
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 49e0022533..5ec1c6a2a2 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -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)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 85e43da568..da46b1a36d 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -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;
diff --git a/migration/colo-comm.c b/migration/colo-comm.c
index b61aa19a38..df26e4dfe7 100644
--- a/migration/colo-comm.c
+++ b/migration/colo-comm.c
@@ -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)
diff --git a/migration/global_state.c b/migration/global_state.c
index dfdaf63910..8e8ab5c51e 100644
--- a/migration/global_state.c
+++ b/migration/global_state.c
@@ -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 = {
diff --git a/migration/savevm.c b/migration/savevm.c
index 2478352baf..e9feade46a 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -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)
diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index a4ded2956d..b2e10769a6 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -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)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 1d6756821c..1cb6b07004 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -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)
diff --git a/target/arm/machine.c b/target/arm/machine.c
index e5fe083da4..29df7ac87d 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -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)
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 29cc58eda9..df5ec359eb 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -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)
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index e36b7100cb..384caee800 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -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;
}
/*
diff --git a/target/s390x/machine.c b/target/s390x/machine.c
index 097a147dbb..e1b7015d20 100644
--- a/target/s390x/machine.c
+++ b/target/s390x/machine.c
@@ -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)
diff --git a/target/sparc/machine.c b/target/sparc/machine.c
index 6bd6b8ee3e..8ff9dea297 100644
--- a/target/sparc/machine.c
+++ b/target/sparc/machine.c
@@ -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
diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index ee292c7bee..e643ac662b 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -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)
--
2.13.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state
2017-09-25 11:29 [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert (git)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int Dr. David Alan Gilbert (git)
@ 2017-09-25 11:29 ` Dr. David Alan Gilbert (git)
2017-09-26 2:36 ` Peter Xu
` (2 more replies)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns Dr. David Alan Gilbert (git)
` (4 subsequent siblings)
6 siblings, 3 replies; 31+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-09-25 11:29 UTC (permalink / raw)
To: qemu-devel, quintela, peterx, famz; +Cc: peter.maydell, cohuck, rth
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Check the return value of pre_save state and fail vmstate_save_state
if the pre_save failed.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
include/migration/vmstate.h | 4 ++--
migration/trace-events | 1 +
migration/vmstate.c | 12 ++++++++++--
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index da46b1a36d..88b55df5ae 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -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);
diff --git a/migration/trace-events b/migration/trace-events
index d2910a6e7b..6f29fcc686 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -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"
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 3226e8eb45..ae8abd3c32 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -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 *
--
2.13.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns
2017-09-25 11:29 [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert (git)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int Dr. David Alan Gilbert (git)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state Dr. David Alan Gilbert (git)
@ 2017-09-25 11:29 ` Dr. David Alan Gilbert (git)
2017-09-26 2:42 ` Peter Xu
` (2 more replies)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save Dr. David Alan Gilbert (git)
` (3 subsequent siblings)
6 siblings, 3 replies; 31+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-09-25 11:29 UTC (permalink / raw)
To: qemu-devel, quintela, peterx, famz; +Cc: peter.maydell, cohuck, rth
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Check the return values from vmstate_save_state for fields and also the
return values from 'put' for fields that use that.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
migration/vmstate.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index ae8abd3c32..848e8448c6 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -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;
--
2.13.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save
2017-09-25 11:29 [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert (git)
` (2 preceding siblings ...)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns Dr. David Alan Gilbert (git)
@ 2017-09-25 11:29 ` Dr. David Alan Gilbert (git)
2017-09-26 2:45 ` Peter Xu
` (2 more replies)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save Dr. David Alan Gilbert (git)
` (2 subsequent siblings)
6 siblings, 3 replies; 31+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-09-25 11:29 UTC (permalink / raw)
To: qemu-devel, quintela, peterx, famz; +Cc: peter.maydell, cohuck, rth
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Route the errors from vmstate_subsection_save up through
vmstate_subsection_save (and back down, all rather recursive).
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
migration/vmstate.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 848e8448c6..0b3282c9df 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -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;
}
--
2.13.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save
2017-09-25 11:29 [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert (git)
` (3 preceding siblings ...)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save Dr. David Alan Gilbert (git)
@ 2017-09-25 11:29 ` Dr. David Alan Gilbert (git)
2017-09-26 2:56 ` Peter Xu
` (2 more replies)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths Dr. David Alan Gilbert (git)
2017-09-27 10:51 ` [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert
6 siblings, 3 replies; 31+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-09-25 11:29 UTC (permalink / raw)
To: qemu-devel, quintela, peterx, famz; +Cc: peter.maydell, cohuck, rth
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
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.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
migration/savevm.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index e9feade46a..4a88228614 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -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);
}
--
2.13.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths
2017-09-25 11:29 [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert (git)
` (4 preceding siblings ...)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save Dr. David Alan Gilbert (git)
@ 2017-09-25 11:29 ` Dr. David Alan Gilbert (git)
2017-09-26 3:06 ` Peter Xu
` (2 more replies)
2017-09-27 10:51 ` [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert
6 siblings, 3 replies; 31+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-09-25 11:29 UTC (permalink / raw)
To: qemu-devel, quintela, peterx, famz; +Cc: peter.maydell, cohuck, rth
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
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.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
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(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 3a8f1e1a2d..2becbfda59 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -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,
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 3129d25c00..311929e9df 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -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 */
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 80c45c321e..5abada6966 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -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;
diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
index c056c98bdb..48184c380d 100644
--- a/migration/vmstate-types.c
+++ b/migration/vmstate-types.c
@@ -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);
diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index e643ac662b..087844b6c8 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -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[] = {
--
2.13.5
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int Dr. David Alan Gilbert (git)
@ 2017-09-25 13:19 ` Richard Henderson
2017-09-25 13:25 ` Dr. David Alan Gilbert
2017-09-26 2:31 ` Peter Xu
` (2 subsequent siblings)
3 siblings, 1 reply; 31+ messages in thread
From: Richard Henderson @ 2017-09-25 13:19 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git), qemu-devel, quintela, peterx, famz
Cc: peter.maydell, cohuck
On 09/25/2017 06:29 AM, Dr. David Alan Gilbert (git) wrote:
> Modify the pre_save method 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~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
2017-09-25 13:19 ` Richard Henderson
@ 2017-09-25 13:25 ` Dr. David Alan Gilbert
2017-09-25 13:29 ` Richard Henderson
0 siblings, 1 reply; 31+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-25 13:25 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel, quintela, peterx, famz, peter.maydell, cohuck
* Richard Henderson (rth@twiddle.net) wrote:
> On 09/25/2017 06:29 AM, Dr. David Alan Gilbert (git) wrote:
> > Modify the pre_save method 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.
> Why not a bool?
Consistency with pre_load, post_load, put and get methods
that are already int.
Dave
>
> r~
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
2017-09-25 13:25 ` Dr. David Alan Gilbert
@ 2017-09-25 13:29 ` Richard Henderson
0 siblings, 0 replies; 31+ messages in thread
From: Richard Henderson @ 2017-09-25 13:29 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, quintela, peterx, famz, peter.maydell, cohuck
On 09/25/2017 06:25 AM, Dr. David Alan Gilbert wrote:
>> Why not a bool?
>
> Consistency with pre_load, post_load, put and get methods
> that are already int.
Fair enough, thanks.
r~
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int Dr. David Alan Gilbert (git)
2017-09-25 13:19 ` Richard Henderson
@ 2017-09-26 2:31 ` Peter Xu
2017-09-26 13:27 ` Cornelia Huck
2017-09-27 8:57 ` Juan Quintela
3 siblings, 0 replies; 31+ messages in thread
From: Peter Xu @ 2017-09-26 2:31 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, famz, peter.maydell, cohuck, rth
On Mon, Sep 25, 2017 at 12:29:12PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> 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.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state Dr. David Alan Gilbert (git)
@ 2017-09-26 2:36 ` Peter Xu
2017-09-26 13:29 ` Cornelia Huck
2017-09-27 8:58 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Peter Xu @ 2017-09-26 2:36 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, famz, peter.maydell, cohuck, rth
On Mon, Sep 25, 2017 at 12:29:13PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Check the return value of pre_save state and fail vmstate_save_state
> if the pre_save failed.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns Dr. David Alan Gilbert (git)
@ 2017-09-26 2:42 ` Peter Xu
2017-09-26 13:32 ` Cornelia Huck
2017-09-27 8:59 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Peter Xu @ 2017-09-26 2:42 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, famz, peter.maydell, cohuck, rth
On Mon, Sep 25, 2017 at 12:29:14PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Check the return values from vmstate_save_state for fields and also the
> return values from 'put' for fields that use that.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/vmstate.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index ae8abd3c32..848e8448c6 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -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);
I would prefer to prefix the function name (also there is one
error_report in previous patch) but that's optional.
Reviewed-by: Peter Xu <peterx@redhat.com>
> + return ret;
> }
>
> written_bytes = qemu_ftell_fast(f) - old_offset;
> --
> 2.13.5
>
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save Dr. David Alan Gilbert (git)
@ 2017-09-26 2:45 ` Peter Xu
2017-09-26 9:55 ` Dr. David Alan Gilbert
2017-09-26 13:35 ` Cornelia Huck
2017-09-27 9:00 ` Juan Quintela
2 siblings, 1 reply; 31+ messages in thread
From: Peter Xu @ 2017-09-26 2:45 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, famz, peter.maydell, cohuck, rth
On Mon, Sep 25, 2017 at 12:29:15PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Route the errors from vmstate_subsection_save up through
> vmstate_subsection_save (and back down, all rather recursive).
I guess here one of the "vmstate_subsection_save" should be replaced
by "vmstate_save_state"? :-)
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save Dr. David Alan Gilbert (git)
@ 2017-09-26 2:56 ` Peter Xu
2017-09-26 13:36 ` Cornelia Huck
2017-09-27 9:00 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Peter Xu @ 2017-09-26 2:56 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, famz, peter.maydell, cohuck, rth
On Mon, Sep 25, 2017 at 12:29:16PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> 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.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths Dr. David Alan Gilbert (git)
@ 2017-09-26 3:06 ` Peter Xu
2017-09-26 10:03 ` Dr. David Alan Gilbert
2017-09-26 13:39 ` Cornelia Huck
2017-09-27 9:07 ` Juan Quintela
2 siblings, 1 reply; 31+ messages in thread
From: Peter Xu @ 2017-09-26 3:06 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, famz, peter.maydell, cohuck, rth
On Mon, Sep 25, 2017 at 12:29:17PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> vmstate_save is called in a few places, and vmstate_save_state is
> called in lots of places.
vmstate_save() should have been processed with previous patch?
>
> 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 <dgilbert@redhat.com>
The patch content wires quite a few vmstate_save_state() callers, so
the patch itself makes sense to me.
Reviewed-by: Peter Xu <peterx@redhat.com>
> ---
> 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(-)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 3a8f1e1a2d..2becbfda59 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -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,
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 3129d25c00..311929e9df 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -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 */
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 80c45c321e..5abada6966 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -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;
>
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index c056c98bdb..48184c380d 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -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);
>
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index e643ac662b..087844b6c8 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -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[] = {
> --
> 2.13.5
>
--
Peter Xu
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save
2017-09-26 2:45 ` Peter Xu
@ 2017-09-26 9:55 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 31+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-26 9:55 UTC (permalink / raw)
To: Peter Xu; +Cc: qemu-devel, quintela, famz, peter.maydell, cohuck, rth
* Peter Xu (peterx@redhat.com) wrote:
> On Mon, Sep 25, 2017 at 12:29:15PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Route the errors from vmstate_subsection_save up through
> > vmstate_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.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
Thanks.
Dave
>
> --
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths
2017-09-26 3:06 ` Peter Xu
@ 2017-09-26 10:03 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 31+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-26 10:03 UTC (permalink / raw)
To: Peter Xu; +Cc: qemu-devel, quintela, famz, peter.maydell, cohuck, rth
* Peter Xu (peterx@redhat.com) wrote:
> On Mon, Sep 25, 2017 at 12:29:17PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > vmstate_save is called in a few places, and vmstate_save_state is
> > 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.
> >
> > 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 <dgilbert@redhat.com>
>
> The patch content wires quite a few vmstate_save_state() callers, so
> the patch itself makes sense to me.
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
Thanks!
Dave
> > ---
> > 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(-)
> >
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index 3a8f1e1a2d..2becbfda59 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -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,
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 3129d25c00..311929e9df 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -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 */
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index 80c45c321e..5abada6966 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -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;
> >
> > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> > index c056c98bdb..48184c380d 100644
> > --- a/migration/vmstate-types.c
> > +++ b/migration/vmstate-types.c
> > @@ -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);
> >
> > diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> > index e643ac662b..087844b6c8 100644
> > --- a/tests/test-vmstate.c
> > +++ b/tests/test-vmstate.c
> > @@ -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[] = {
> > --
> > 2.13.5
> >
>
> --
> Peter Xu
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int Dr. David Alan Gilbert (git)
2017-09-25 13:19 ` Richard Henderson
2017-09-26 2:31 ` Peter Xu
@ 2017-09-26 13:27 ` Cornelia Huck
2017-09-27 8:57 ` Juan Quintela
3 siblings, 0 replies; 31+ messages in thread
From: Cornelia Huck @ 2017-09-26 13:27 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, peterx, famz, peter.maydell, rth
On Mon, 25 Sep 2017 12:29:12 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> 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.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> 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(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state Dr. David Alan Gilbert (git)
2017-09-26 2:36 ` Peter Xu
@ 2017-09-26 13:29 ` Cornelia Huck
2017-09-27 8:58 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Cornelia Huck @ 2017-09-26 13:29 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, peterx, famz, peter.maydell, rth
On Mon, 25 Sep 2017 12:29:13 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Check the return value of pre_save state and fail vmstate_save_state
> if the pre_save failed.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> include/migration/vmstate.h | 4 ++--
> migration/trace-events | 1 +
> migration/vmstate.c | 12 ++++++++++--
> 3 files changed, 13 insertions(+), 4 deletions(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns Dr. David Alan Gilbert (git)
2017-09-26 2:42 ` Peter Xu
@ 2017-09-26 13:32 ` Cornelia Huck
2017-09-27 8:59 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Cornelia Huck @ 2017-09-26 13:32 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, peterx, famz, peter.maydell, rth
On Mon, 25 Sep 2017 12:29:14 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Check the return values from vmstate_save_state for fields and also the
> return values from 'put' for fields that use that.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/vmstate.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save Dr. David Alan Gilbert (git)
2017-09-26 2:45 ` Peter Xu
@ 2017-09-26 13:35 ` Cornelia Huck
2017-09-27 9:00 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Cornelia Huck @ 2017-09-26 13:35 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, peterx, famz, peter.maydell, rth
On Mon, 25 Sep 2017 12:29:15 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Route the errors from vmstate_subsection_save up through
> vmstate_subsection_save (and back down, all rather recursive).
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/vmstate.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
With the patch description fixed up:
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save Dr. David Alan Gilbert (git)
2017-09-26 2:56 ` Peter Xu
@ 2017-09-26 13:36 ` Cornelia Huck
2017-09-27 9:00 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Cornelia Huck @ 2017-09-26 13:36 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, peterx, famz, peter.maydell, rth
On Mon, 25 Sep 2017 12:29:16 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> 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.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> migration/savevm.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths Dr. David Alan Gilbert (git)
2017-09-26 3:06 ` Peter Xu
@ 2017-09-26 13:39 ` Cornelia Huck
2017-09-27 9:07 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Cornelia Huck @ 2017-09-26 13:39 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, quintela, peterx, famz, peter.maydell, rth
On Mon, 25 Sep 2017 12:29:17 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> 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.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> 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:
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int Dr. David Alan Gilbert (git)
` (2 preceding siblings ...)
2017-09-26 13:27 ` Cornelia Huck
@ 2017-09-27 8:57 ` Juan Quintela
3 siblings, 0 replies; 31+ messages in thread
From: Juan Quintela @ 2017-09-27 8:57 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, peterx, famz, peter.maydell, cohuck, rth
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> 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.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state Dr. David Alan Gilbert (git)
2017-09-26 2:36 ` Peter Xu
2017-09-26 13:29 ` Cornelia Huck
@ 2017-09-27 8:58 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Juan Quintela @ 2017-09-27 8:58 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, peterx, famz, peter.maydell, cohuck, rth
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Check the return value of pre_save state and fail vmstate_save_state
> if the pre_save failed.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns Dr. David Alan Gilbert (git)
2017-09-26 2:42 ` Peter Xu
2017-09-26 13:32 ` Cornelia Huck
@ 2017-09-27 8:59 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Juan Quintela @ 2017-09-27 8:59 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, peterx, famz, peter.maydell, cohuck, rth
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Check the return values from vmstate_save_state for fields and also the
> return values from 'put' for fields that use that.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save Dr. David Alan Gilbert (git)
2017-09-26 2:45 ` Peter Xu
2017-09-26 13:35 ` Cornelia Huck
@ 2017-09-27 9:00 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Juan Quintela @ 2017-09-27 9:00 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, peterx, famz, peter.maydell, cohuck, rth
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Route the errors from vmstate_subsection_save up through
> vmstate_subsection_save (and back down, all rather recursive).
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save Dr. David Alan Gilbert (git)
2017-09-26 2:56 ` Peter Xu
2017-09-26 13:36 ` Cornelia Huck
@ 2017-09-27 9:00 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Juan Quintela @ 2017-09-27 9:00 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, peterx, famz, peter.maydell, cohuck, rth
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> 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.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths Dr. David Alan Gilbert (git)
2017-09-26 3:06 ` Peter Xu
2017-09-26 13:39 ` Cornelia Huck
@ 2017-09-27 9:07 ` Juan Quintela
2 siblings, 0 replies; 31+ messages in thread
From: Juan Quintela @ 2017-09-27 9:07 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git)
Cc: qemu-devel, peterx, famz, peter.maydell, cohuck, rth
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> 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.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
But .....
> -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);
> }
We add error code, good.
>
> /* 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.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail
2017-09-25 11:29 [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert (git)
` (5 preceding siblings ...)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths Dr. David Alan Gilbert (git)
@ 2017-09-27 10:51 ` Dr. David Alan Gilbert
6 siblings, 0 replies; 31+ messages in thread
From: Dr. David Alan Gilbert @ 2017-09-27 10:51 UTC (permalink / raw)
To: qemu-devel, quintela, peterx, famz; +Cc: peter.maydell, cohuck, rth
* Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> 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.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Queued.
>
> 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(-)
>
> --
> 2.13.5
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2017-09-27 10:51 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-25 11:29 [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert (git)
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int Dr. David Alan Gilbert (git)
2017-09-25 13:19 ` Richard Henderson
2017-09-25 13:25 ` Dr. David Alan Gilbert
2017-09-25 13:29 ` Richard Henderson
2017-09-26 2:31 ` Peter Xu
2017-09-26 13:27 ` Cornelia Huck
2017-09-27 8:57 ` Juan Quintela
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 2/6] migration: check pre_save return in vmstate_save_state Dr. David Alan Gilbert (git)
2017-09-26 2:36 ` Peter Xu
2017-09-26 13:29 ` Cornelia Huck
2017-09-27 8:58 ` Juan Quintela
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 3/6] migration: Check field save returns Dr. David Alan Gilbert (git)
2017-09-26 2:42 ` Peter Xu
2017-09-26 13:32 ` Cornelia Huck
2017-09-27 8:59 ` Juan Quintela
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 4/6] migration: wire vmstate_save_state errors up to vmstate_subsection_save Dr. David Alan Gilbert (git)
2017-09-26 2:45 ` Peter Xu
2017-09-26 9:55 ` Dr. David Alan Gilbert
2017-09-26 13:35 ` Cornelia Huck
2017-09-27 9:00 ` Juan Quintela
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 5/6] migration: Route errors up through vmstate_save Dr. David Alan Gilbert (git)
2017-09-26 2:56 ` Peter Xu
2017-09-26 13:36 ` Cornelia Huck
2017-09-27 9:00 ` Juan Quintela
2017-09-25 11:29 ` [Qemu-devel] [PATCH v2 6/6] migration: Route more error paths Dr. David Alan Gilbert (git)
2017-09-26 3:06 ` Peter Xu
2017-09-26 10:03 ` Dr. David Alan Gilbert
2017-09-26 13:39 ` Cornelia Huck
2017-09-27 9:07 ` Juan Quintela
2017-09-27 10:51 ` [Qemu-devel] [PATCH v2 0/6] migration: let pre_save fail Dr. David Alan Gilbert
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).