Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

2017-09-27 Thread Juan Quintela 
"Dr. David Alan Gilbert (git)"  wrote:
> From: "Dr. David Alan Gilbert" 
>
> 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 

Reviewed-by: Juan Quintela

Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

2017-09-26 Thread Cornelia Huck 
On Mon, 25 Sep 2017 12:29:12 +0100
"Dr. David Alan Gilbert (git)"  wrote:

> From: "Dr. David Alan Gilbert" 
> 
> 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 
> ---
>  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

Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

2017-09-25 Thread Peter Xu 
On Mon, Sep 25, 2017 at 12:29:12PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> 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 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

2017-09-25 Thread Richard Henderson 
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~

Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

2017-09-25 Thread Dr. David Alan Gilbert 
* Richard Henderson (r...@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 / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

2017-09-25 Thread Richard Henderson 
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~

[Qemu-devel] [PATCH v2 1/6] migration: pre_save return int

2017-09-25 Thread Dr. David Alan Gilbert (git) 
From: "Dr. David Alan Gilbert" 

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 
---
 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, >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, >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;
 }