All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 00/15] migration queue
@ 2017-01-24 18:47 Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 01/15] MAINTAINERS: Add myself as a migration submaintainer Dr. David Alan Gilbert (git)
                   ` (15 more replies)
  0 siblings, 16 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The following changes since commit a678502e4f7580a6f143f680404aaee57ac3f4b5:

  Merge remote-tracking branch 'remotes/ehabkost/tags/x86-and-machine-pull-request' into staging (2017-01-24 15:39:09 +0000)

are available in the git repository at:

  git://github.com/dagrh/qemu.git tags/pull-migration-20170124b

for you to fetch changes up to 46a02a745176e1ef30de41706b5da4f1bc7fc495:

  migration/tracing: Add tracing on save (2017-01-24 18:00:32 +0000)

----------------------------------------------------------------
Migration

1 My maintainer change
2 Jianjun's qtailq
3 Ashijeet's only-migratable
4 Zhanghailiang's re-active images
5 Pankaj's change name of migration thread
6 My PCI migration merge
7 Juan's debug to tracing
8 My tracing on save

----------------------------------------------------------------
Ashijeet Acharya (5):
      block/vvfat: Remove the undesirable comment
      migration: Add a new option to enable only-migratable
      migration: Allow "device add" options to only add migratable devices
      migration: disallow migrate_add_blocker during migration
      migration: Fail migration blocker for --only-migratable

Dr. David Alan Gilbert (3):
      MAINTAINERS: Add myself as a migration submaintainer
      PCI/migration merge vmstate_pci_device and vmstate_pcie_device
      migration/tracing: Add tracing on save

Jianjun Duan (4):
      migration: extend VMStateInfo
      migration: migrate QTAILQ
      tests/migration: Add test for QTAILQ migration
      migration: add error_report

Juan Quintela (1):
      migration: transform remaining DPRINTF into trace_

Pankaj Gupta (1):
      migration: Change name of live migration thread

zhanghailiang (1):
      migration: re-active images while migration been canceled after inactive them

 MAINTAINERS                        |   1 +
 block/qcow.c                       |   8 +-
 block/vdi.c                        |   8 +-
 block/vhdx.c                       |  17 ++--
 block/vmdk.c                       |   9 +-
 block/vpc.c                        |  11 +-
 block/vvfat.c                      |  20 ++--
 hw/9pfs/9p.c                       |  33 ++++--
 hw/display/virtio-gpu.c            |  40 +++++---
 hw/intc/arm_gic_kvm.c              |  17 ++--
 hw/intc/arm_gicv3_its_kvm.c        |  20 ++--
 hw/intc/arm_gicv3_kvm.c            |  19 ++--
 hw/intc/s390_flic_kvm.c            |   8 +-
 hw/misc/ivshmem.c                  |  14 ++-
 hw/net/e1000e.c                    |   2 +-
 hw/net/vmxnet3.c                   |  26 +++--
 hw/nvram/eeprom93xx.c              |   8 +-
 hw/nvram/fw_cfg.c                  |   8 +-
 hw/pci-bridge/ioh3420.c            |   2 +-
 hw/pci-bridge/xio3130_downstream.c |   2 +-
 hw/pci-bridge/xio3130_upstream.c   |   2 +-
 hw/pci/msix.c                      |   8 +-
 hw/pci/pci.c                       |  57 ++++++-----
 hw/pci/shpc.c                      |   7 +-
 hw/scsi/megasas.c                  |   2 +-
 hw/scsi/scsi-bus.c                 |   8 +-
 hw/scsi/vhost-scsi.c               |  25 +++--
 hw/scsi/vmw_pvscsi.c               |   2 +-
 hw/timer/twl92230.c                |   8 +-
 hw/usb/bus.c                       |  19 ++++
 hw/usb/hcd-xhci.c                  |   2 +-
 hw/usb/redirect.c                  |  26 +++--
 hw/virtio/vhost.c                  |   8 +-
 hw/virtio/virtio-pci.c             |   8 +-
 hw/virtio/virtio.c                 |  15 ++-
 include/hw/pci/pcie.h              |  10 --
 include/migration/migration.h      |  13 ++-
 include/migration/vmstate.h        |  39 ++++++-
 include/qemu/queue.h               |  60 +++++++++++
 migration/migration.c              |  61 ++++++++++-
 migration/ram.c                    |  18 +---
 migration/savevm.c                 |   7 +-
 migration/trace-events             |  12 +++
 migration/vmstate.c                | 203 +++++++++++++++++++++++++++++--------
 qdev-monitor.c                     |   9 ++
 qemu-options.hx                    |   9 ++
 stubs/migr-blocker.c               |   3 +-
 target/alpha/machine.c             |   6 +-
 target/arm/machine.c               |  14 ++-
 target/i386/kvm.c                  |  16 ++-
 target/i386/machine.c              |  26 +++--
 target/mips/machine.c              |  14 ++-
 target/ppc/machine.c               |  12 ++-
 target/sparc/machine.c             |   6 +-
 tests/test-vmstate.c               | 147 +++++++++++++++++++++++++++
 vl.c                               |   4 +
 56 files changed, 910 insertions(+), 249 deletions(-)

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 01/15] MAINTAINERS: Add myself as a migration submaintainer
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo Dr. David Alan Gilbert (git)
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <20170124100437.18200-1-dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6a3df66..e41271c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1420,6 +1420,7 @@ F: scripts/checkpatch.pl
 Migration
 M: Juan Quintela <quintela@redhat.com>
 M: Amit Shah <amit.shah@redhat.com>
+M: Dr. David Alan Gilbert <dgilbert@redhat.com>
 S: Maintained
 F: include/migration/
 F: migration/
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 01/15] MAINTAINERS: Add myself as a migration submaintainer Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-25 11:46   ` Fam Zheng
  2017-01-24 18:47 ` [Qemu-devel] [PULL 03/15] migration: migrate QTAILQ Dr. David Alan Gilbert (git)
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Jianjun Duan <duanj@linux.vnet.ibm.com>

Current migration code cannot handle some data structures such as
QTAILQ in qemu/queue.h. Here we extend the signatures of put/get
in VMStateInfo so that customized handling is supported. put now
will return int type.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
Message-Id: <1484852453-12728-2-git-send-email-duanj@linux.vnet.ibm.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/display/virtio-gpu.c     |   8 +++-
 hw/intc/s390_flic_kvm.c     |   8 +++-
 hw/net/vmxnet3.c            |  24 +++++++---
 hw/nvram/eeprom93xx.c       |   8 +++-
 hw/nvram/fw_cfg.c           |   8 +++-
 hw/pci/msix.c               |   8 +++-
 hw/pci/pci.c                |  16 +++++--
 hw/pci/shpc.c               |   7 ++-
 hw/scsi/scsi-bus.c          |   8 +++-
 hw/timer/twl92230.c         |   8 +++-
 hw/usb/redirect.c           |  26 +++++++---
 hw/virtio/virtio-pci.c      |   8 +++-
 hw/virtio/virtio.c          |  15 ++++--
 include/migration/vmstate.h |  19 ++++++--
 migration/savevm.c          |   7 ++-
 migration/vmstate.c         | 113 +++++++++++++++++++++++++++++---------------
 target/alpha/machine.c      |   6 ++-
 target/arm/machine.c        |  14 ++++--
 target/i386/machine.c       |  26 +++++++---
 target/mips/machine.c       |  14 ++++--
 target/ppc/machine.c        |  12 +++--
 target/sparc/machine.c      |   6 ++-
 22 files changed, 263 insertions(+), 106 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 7a15c61..6e09474 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1003,7 +1003,8 @@ static const VMStateDescription vmstate_virtio_gpu_scanouts = {
     },
 };
 
-static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size)
+static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,
+                           VMStateField *field, QJSON *vmdesc)
 {
     VirtIOGPU *g = opaque;
     struct virtio_gpu_simple_resource *res;
@@ -1028,9 +1029,12 @@ static void 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;
 }
 
-static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size)
+static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
+                           VMStateField *field)
 {
     VirtIOGPU *g = opaque;
     struct virtio_gpu_simple_resource *res;
diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index c313166..da8e4df 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -286,7 +286,8 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
  * increase until buffer is sufficient or maxium size is
  * reached
  */
-static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
+static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
+                         VMStateField *field, QJSON *vmdesc)
 {
     KVMS390FLICState *flic = opaque;
     int len = FLIC_SAVE_INITIAL_SIZE;
@@ -319,6 +320,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
                         count * sizeof(struct kvm_s390_irq));
     }
     g_free(buf);
+
+    return 0;
 }
 
 /**
@@ -331,7 +334,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
  * Note: Do nothing when no interrupts where stored
  * in QEMUFile
  */
-static int kvm_flic_load(QEMUFile *f, void *opaque, size_t size)
+static int kvm_flic_load(QEMUFile *f, void *opaque, size_t size,
+                         VMStateField *field)
 {
     uint64_t len = 0;
     uint64_t count = 0;
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 92f6af9..4163ca8 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2451,7 +2451,8 @@ static void vmxnet3_put_tx_stats_to_file(QEMUFile *f,
     qemu_put_be64(f, tx_stat->pktsTxDiscard);
 }
 
-static int vmxnet3_get_txq_descr(QEMUFile *f, void *pv, size_t size)
+static int vmxnet3_get_txq_descr(QEMUFile *f, void *pv, size_t size,
+    VMStateField *field)
 {
     Vmxnet3TxqDescr *r = pv;
 
@@ -2465,7 +2466,8 @@ static int vmxnet3_get_txq_descr(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void vmxnet3_put_txq_descr(QEMUFile *f, void *pv, size_t size)
+static int vmxnet3_put_txq_descr(QEMUFile *f, void *pv, size_t size,
+                                 VMStateField *field, QJSON *vmdesc)
 {
     Vmxnet3TxqDescr *r = pv;
 
@@ -2474,6 +2476,8 @@ static void vmxnet3_put_txq_descr(QEMUFile *f, void *pv, size_t size)
     qemu_put_byte(f, r->intr_idx);
     qemu_put_be64(f, r->tx_stats_pa);
     vmxnet3_put_tx_stats_to_file(f, &r->txq_stats);
+
+    return 0;
 }
 
 static const VMStateInfo txq_descr_info = {
@@ -2512,7 +2516,8 @@ static void vmxnet3_put_rx_stats_to_file(QEMUFile *f,
     qemu_put_be64(f, rx_stat->pktsRxError);
 }
 
-static int vmxnet3_get_rxq_descr(QEMUFile *f, void *pv, size_t size)
+static int vmxnet3_get_rxq_descr(QEMUFile *f, void *pv, size_t size,
+    VMStateField *field)
 {
     Vmxnet3RxqDescr *r = pv;
     int i;
@@ -2530,7 +2535,8 @@ static int vmxnet3_get_rxq_descr(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void vmxnet3_put_rxq_descr(QEMUFile *f, void *pv, size_t size)
+static int vmxnet3_put_rxq_descr(QEMUFile *f, void *pv, size_t size,
+                                 VMStateField *field, QJSON *vmdesc)
 {
     Vmxnet3RxqDescr *r = pv;
     int i;
@@ -2543,6 +2549,8 @@ static void vmxnet3_put_rxq_descr(QEMUFile *f, void *pv, size_t size)
     qemu_put_byte(f, r->intr_idx);
     qemu_put_be64(f, r->rx_stats_pa);
     vmxnet3_put_rx_stats_to_file(f, &r->rxq_stats);
+
+    return 0;
 }
 
 static int vmxnet3_post_load(void *opaque, int version_id)
@@ -2575,7 +2583,8 @@ static const VMStateInfo rxq_descr_info = {
     .put = vmxnet3_put_rxq_descr
 };
 
-static int vmxnet3_get_int_state(QEMUFile *f, void *pv, size_t size)
+static int vmxnet3_get_int_state(QEMUFile *f, void *pv, size_t size,
+    VMStateField *field)
 {
     Vmxnet3IntState *r = pv;
 
@@ -2586,13 +2595,16 @@ static int vmxnet3_get_int_state(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void vmxnet3_put_int_state(QEMUFile *f, void *pv, size_t size)
+static int vmxnet3_put_int_state(QEMUFile *f, void *pv, size_t size,
+                                 VMStateField *field, QJSON *vmdesc)
 {
     Vmxnet3IntState *r = pv;
 
     qemu_put_byte(f, r->is_masked);
     qemu_put_byte(f, r->is_pending);
     qemu_put_byte(f, r->is_asserted);
+
+    return 0;
 }
 
 static const VMStateInfo int_state_info = {
diff --git a/hw/nvram/eeprom93xx.c b/hw/nvram/eeprom93xx.c
index 2c16fc2..848692a 100644
--- a/hw/nvram/eeprom93xx.c
+++ b/hw/nvram/eeprom93xx.c
@@ -94,18 +94,22 @@ struct _eeprom_t {
    This is a Big hack, but it is how the old state did it.
  */
 
-static int get_uint16_from_uint8(QEMUFile *f, void *pv, size_t size)
+static int get_uint16_from_uint8(QEMUFile *f, void *pv, size_t size,
+                                 VMStateField *field)
 {
     uint16_t *v = pv;
     *v = qemu_get_ubyte(f);
     return 0;
 }
 
-static void put_unused(QEMUFile *f, void *pv, size_t size)
+static int put_unused(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                      QJSON *vmdesc)
 {
     fprintf(stderr, "uint16_from_uint8 is used only for backwards compatibility.\n");
     fprintf(stderr, "Never should be used to write a new state.\n");
     exit(0);
+
+    return 0;
 }
 
 static const VMStateInfo vmstate_hack_uint16_from_uint8 = {
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 523d585..316fca9 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -555,17 +555,21 @@ static void fw_cfg_reset(DeviceState *d)
    Or we broke compatibility in the state, or we can't use struct tm
  */
 
-static int get_uint32_as_uint16(QEMUFile *f, void *pv, size_t size)
+static int get_uint32_as_uint16(QEMUFile *f, void *pv, size_t size,
+                                VMStateField *field)
 {
     uint32_t *v = pv;
     *v = qemu_get_be16(f);
     return 0;
 }
 
-static void put_unused(QEMUFile *f, void *pv, size_t size)
+static int put_unused(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                      QJSON *vmdesc)
 {
     fprintf(stderr, "uint32_as_uint16 is only used for backward compatibility.\n");
     fprintf(stderr, "This functions shouldn't be called.\n");
+
+    return 0;
 }
 
 static const VMStateInfo vmstate_hack_uint32_as_uint16 = {
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 0ec1cb1..ee1714d 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -587,12 +587,16 @@ void msix_unset_vector_notifiers(PCIDevice *dev)
     dev->msix_vector_poll_notifier = NULL;
 }
 
-static void put_msix_state(QEMUFile *f, void *pv, size_t size)
+static int put_msix_state(QEMUFile *f, void *pv, size_t size,
+                          VMStateField *field, QJSON *vmdesc)
 {
     msix_save(pv, f);
+
+    return 0;
 }
 
-static int get_msix_state(QEMUFile *f, void *pv, size_t size)
+static int get_msix_state(QEMUFile *f, void *pv, size_t size,
+                          VMStateField *field)
 {
     msix_load(pv, f);
     return 0;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index fe9acec..c35d011 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -445,7 +445,8 @@ int pci_bus_numa_node(PCIBus *bus)
     return PCI_BUS_GET_CLASS(bus)->numa_node(bus);
 }
 
-static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
+static int get_pci_config_device(QEMUFile *f, void *pv, size_t size,
+                                 VMStateField *field)
 {
     PCIDevice *s = container_of(pv, PCIDevice, config);
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(s);
@@ -484,11 +485,14 @@ static int get_pci_config_device(QEMUFile *f, void *pv, size_t size)
 }
 
 /* just put buffer */
-static void put_pci_config_device(QEMUFile *f, void *pv, size_t size)
+static int put_pci_config_device(QEMUFile *f, void *pv, size_t size,
+                                 VMStateField *field, QJSON *vmdesc)
 {
     const uint8_t **v = pv;
     assert(size == pci_config_size(container_of(pv, PCIDevice, config)));
     qemu_put_buffer(f, *v, size);
+
+    return 0;
 }
 
 static VMStateInfo vmstate_info_pci_config = {
@@ -497,7 +501,8 @@ static VMStateInfo vmstate_info_pci_config = {
     .put  = put_pci_config_device,
 };
 
-static int get_pci_irq_state(QEMUFile *f, void *pv, size_t size)
+static int get_pci_irq_state(QEMUFile *f, void *pv, size_t size,
+                             VMStateField *field)
 {
     PCIDevice *s = container_of(pv, PCIDevice, irq_state);
     uint32_t irq_state[PCI_NUM_PINS];
@@ -518,7 +523,8 @@ static int get_pci_irq_state(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void put_pci_irq_state(QEMUFile *f, void *pv, size_t size)
+static int put_pci_irq_state(QEMUFile *f, void *pv, size_t size,
+                             VMStateField *field, QJSON *vmdesc)
 {
     int i;
     PCIDevice *s = container_of(pv, PCIDevice, irq_state);
@@ -526,6 +532,8 @@ static void put_pci_irq_state(QEMUFile *f, void *pv, size_t size)
     for (i = 0; i < PCI_NUM_PINS; ++i) {
         qemu_put_be32(f, pci_irq_state(s, i));
     }
+
+    return 0;
 }
 
 static VMStateInfo vmstate_info_pci_irq_state = {
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 3dcd472..42fafac 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -695,13 +695,16 @@ void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int l)
     shpc_cap_update_dword(d);
 }
 
-static void shpc_save(QEMUFile *f, void *pv, size_t size)
+static int shpc_save(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                     QJSON *vmdesc)
 {
     PCIDevice *d = container_of(pv, PCIDevice, shpc);
     qemu_put_buffer(f, d->shpc->config, SHPC_SIZEOF(d));
+
+    return 0;
 }
 
-static int shpc_load(QEMUFile *f, void *pv, size_t size)
+static int shpc_load(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     PCIDevice *d = container_of(pv, PCIDevice, shpc);
     int ret = qemu_get_buffer(f, d->shpc->config, SHPC_SIZEOF(d));
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 297216d..5940cb1 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1945,7 +1945,8 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
 
 /* SCSI request list.  For simplicity, pv points to the whole device */
 
-static void put_scsi_requests(QEMUFile *f, void *pv, size_t size)
+static int put_scsi_requests(QEMUFile *f, void *pv, size_t size,
+                             VMStateField *field, QJSON *vmdesc)
 {
     SCSIDevice *s = pv;
     SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, s->qdev.parent_bus);
@@ -1968,9 +1969,12 @@ static void put_scsi_requests(QEMUFile *f, void *pv, size_t size)
         }
     }
     qemu_put_sbyte(f, 0);
+
+    return 0;
 }
 
-static int get_scsi_requests(QEMUFile *f, void *pv, size_t size)
+static int get_scsi_requests(QEMUFile *f, void *pv, size_t size,
+                             VMStateField *field)
 {
     SCSIDevice *s = pv;
     SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, s->qdev.parent_bus);
diff --git a/hw/timer/twl92230.c b/hw/timer/twl92230.c
index b8d914e..c0aa8ae 100644
--- a/hw/timer/twl92230.c
+++ b/hw/timer/twl92230.c
@@ -749,17 +749,21 @@ static int menelaus_rx(I2CSlave *i2c)
    Or we broke compatibility in the state, or we can't use struct tm
  */
 
-static int get_int32_as_uint16(QEMUFile *f, void *pv, size_t size)
+static int get_int32_as_uint16(QEMUFile *f, void *pv, size_t size,
+                               VMStateField *field)
 {
     int *v = pv;
     *v = qemu_get_be16(f);
     return 0;
 }
 
-static void put_int32_as_uint16(QEMUFile *f, void *pv, size_t size)
+static int put_int32_as_uint16(QEMUFile *f, void *pv, size_t size,
+                               VMStateField *field, QJSON *vmdesc)
 {
     int *v = pv;
     qemu_put_be16(f, *v);
+
+    return 0;
 }
 
 static const VMStateInfo vmstate_hack_int32_as_uint16 = {
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index a657237..4a0ebbf 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -2165,7 +2165,8 @@ static int usbredir_post_load(void *priv, int version_id)
 }
 
 /* For usbredirparser migration */
-static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused)
+static int usbredir_put_parser(QEMUFile *f, void *priv, size_t unused,
+                               VMStateField *field, QJSON *vmdesc)
 {
     USBRedirDevice *dev = priv;
     uint8_t *data;
@@ -2173,7 +2174,7 @@ static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused)
 
     if (dev->parser == NULL) {
         qemu_put_be32(f, 0);
-        return;
+        return 0;
     }
 
     usbredirparser_serialize(dev->parser, &data, &len);
@@ -2183,9 +2184,12 @@ static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused)
     qemu_put_buffer(f, data, len);
 
     free(data);
+
+    return 0;
 }
 
-static int usbredir_get_parser(QEMUFile *f, void *priv, size_t unused)
+static int usbredir_get_parser(QEMUFile *f, void *priv, size_t unused,
+                               VMStateField *field)
 {
     USBRedirDevice *dev = priv;
     uint8_t *data;
@@ -2228,7 +2232,8 @@ static const VMStateInfo usbredir_parser_vmstate_info = {
 
 
 /* For buffered packets (iso/irq) queue migration */
-static void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused)
+static int usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused,
+                              VMStateField *field, QJSON *vmdesc)
 {
     struct endp_data *endp = priv;
     USBRedirDevice *dev = endp->dev;
@@ -2246,9 +2251,12 @@ static void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused)
         i++;
     }
     assert(i == endp->bufpq_size);
+
+    return 0;
 }
 
-static int usbredir_get_bufpq(QEMUFile *f, void *priv, size_t unused)
+static int usbredir_get_bufpq(QEMUFile *f, void *priv, size_t unused,
+                              VMStateField *field)
 {
     struct endp_data *endp = priv;
     USBRedirDevice *dev = endp->dev;
@@ -2351,7 +2359,8 @@ static const VMStateDescription usbredir_ep_vmstate = {
 
 
 /* For PacketIdQueue migration */
-static void usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused)
+static int usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused,
+                                    VMStateField *field, QJSON *vmdesc)
 {
     struct PacketIdQueue *q = priv;
     USBRedirDevice *dev = q->dev;
@@ -2365,9 +2374,12 @@ static void usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused)
         remain--;
     }
     assert(remain == 0);
+
+    return 0;
 }
 
-static int usbredir_get_packet_id_q(QEMUFile *f, void *priv, size_t unused)
+static int usbredir_get_packet_id_q(QEMUFile *f, void *priv, size_t unused,
+                                    VMStateField *field)
 {
     struct PacketIdQueue *q = priv;
     USBRedirDevice *dev = q->dev;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 09230c0..b5af2a0 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -108,7 +108,8 @@ static bool virtio_pci_has_extra_state(DeviceState *d)
     return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA;
 }
 
-static int get_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size)
+static int get_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size,
+                                       VMStateField *field)
 {
     VirtIOPCIProxy *proxy = pv;
     int i;
@@ -137,7 +138,8 @@ static void virtio_pci_save_modern_queue_state(VirtIOPCIQueue *vq,
     qemu_put_be32(f, vq->used[1]);
 }
 
-static void put_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size)
+static int put_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size,
+                                       VMStateField *field, QJSON *vmdesc)
 {
     VirtIOPCIProxy *proxy = pv;
     int i;
@@ -149,6 +151,8 @@ static void put_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size)
     for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
         virtio_pci_save_modern_queue_state(&proxy->vqs[i], f);
     }
+
+    return 0;
 }
 
 static const VMStateInfo vmstate_info_virtio_pci_modern_state = {
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index cc17b97..f292a53 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1555,7 +1555,8 @@ static const VMStateDescription vmstate_virtio_ringsize = {
     }
 };
 
-static int get_extra_state(QEMUFile *f, void *pv, size_t size)
+static int get_extra_state(QEMUFile *f, void *pv, size_t size,
+                           VMStateField *field)
 {
     VirtIODevice *vdev = pv;
     BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
@@ -1568,13 +1569,15 @@ static int get_extra_state(QEMUFile *f, void *pv, size_t size)
     }
 }
 
-static void put_extra_state(QEMUFile *f, void *pv, size_t size)
+static int put_extra_state(QEMUFile *f, void *pv, size_t size,
+                           VMStateField *field, QJSON *vmdesc)
 {
     VirtIODevice *vdev = pv;
     BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 
     k->save_extra_state(qbus->parent, f);
+    return 0;
 }
 
 static const VMStateInfo vmstate_info_extra_state = {
@@ -1709,13 +1712,17 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
 }
 
 /* A wrapper for use as a VMState .put function */
-static void virtio_device_put(QEMUFile *f, void *opaque, size_t size)
+static int virtio_device_put(QEMUFile *f, void *opaque, size_t size,
+                              VMStateField *field, QJSON *vmdesc)
 {
     virtio_save(VIRTIO_DEVICE(opaque), f);
+
+    return 0;
 }
 
 /* A wrapper for use as a VMState .get function */
-static int virtio_device_get(QEMUFile *f, void *opaque, size_t size)
+static int virtio_device_get(QEMUFile *f, void *opaque, size_t size,
+                             VMStateField *field)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
     DeviceClass *dc = DEVICE_CLASS(VIRTIO_DEVICE_GET_CLASS(vdev));
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 2125829..7e61b1e 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -81,11 +81,20 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque);
 
 typedef struct VMStateInfo VMStateInfo;
 typedef struct VMStateDescription VMStateDescription;
-
+typedef struct VMStateField VMStateField;
+
+/* VMStateInfo allows customized migration of objects that don't fit in
+ * any category in VMStateFlags. Additional information is always passed
+ * into get and put in terms of field and vmdesc parameters. However
+ * these two parameters should only be used in cases when customized
+ * handling is needed, such as QTAILQ. For primitive data types such as
+ * integer, field and vmdesc parameters should be ignored inside get/put.
+ */
 struct VMStateInfo {
     const char *name;
-    int (*get)(QEMUFile *f, void *pv, size_t size);
-    void (*put)(QEMUFile *f, void *pv, size_t size);
+    int (*get)(QEMUFile *f, void *pv, size_t size, VMStateField *field);
+    int (*put)(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+               QJSON *vmdesc);
 };
 
 enum VMStateFlags {
@@ -192,7 +201,7 @@ typedef enum {
     MIG_PRI_MAX,
 } MigrationPriority;
 
-typedef struct {
+struct VMStateField {
     const char *name;
     size_t offset;
     size_t size;
@@ -205,7 +214,7 @@ typedef struct {
     const VMStateDescription *vmsd;
     int version_id;
     bool (*field_exists)(void *opaque, int version_id);
-} VMStateField;
+};
 
 struct VMStateDescription {
     const char *name;
diff --git a/migration/savevm.c b/migration/savevm.c
index f9c06e9..455d5ba 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -220,17 +220,20 @@ void timer_get(QEMUFile *f, QEMUTimer *ts)
  * Not in vmstate.c to not add qemu-timer.c as dependency to vmstate.c
  */
 
-static int get_timer(QEMUFile *f, void *pv, size_t size)
+static int get_timer(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     QEMUTimer *v = pv;
     timer_get(f, v);
     return 0;
 }
 
-static void put_timer(QEMUFile *f, void *pv, size_t size)
+static int put_timer(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                     QJSON *vmdesc)
 {
     QEMUTimer *v = pv;
     timer_put(f, v);
+
+    return 0;
 }
 
 const VMStateInfo vmstate_info_timer = {
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 0bc9f35..7b4bd6e 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -6,6 +6,7 @@
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
 #include "trace.h"
+#include "migration/qjson.h"
 
 static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
                                     void *opaque, QJSON *vmdesc);
@@ -122,8 +123,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
                     ret = vmstate_load_state(f, field->vmsd, addr,
                                              field->vmsd->version_id);
                 } else {
-                    ret = field->info->get(f, addr, size);
-
+                   ret = field->info->get(f, addr, size, field);
                 }
                 if (ret >= 0) {
                     ret = qemu_file_get_error(f);
@@ -330,7 +330,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
                 if (field->flags & VMS_STRUCT) {
                     vmstate_save_state(f, field->vmsd, addr, vmdesc_loop);
                 } else {
-                    field->info->put(f, addr, size);
+                    field->info->put(f, addr, size, field, vmdesc_loop);
                 }
 
                 written_bytes = qemu_ftell_fast(f) - old_offset;
@@ -463,17 +463,19 @@ static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
 
 /* bool */
 
-static int get_bool(QEMUFile *f, void *pv, size_t size)
+static int get_bool(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     bool *v = pv;
     *v = qemu_get_byte(f);
     return 0;
 }
 
-static void put_bool(QEMUFile *f, void *pv, size_t size)
+static int put_bool(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                    QJSON *vmdesc)
 {
     bool *v = pv;
     qemu_put_byte(f, *v);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_bool = {
@@ -484,17 +486,19 @@ const VMStateInfo vmstate_info_bool = {
 
 /* 8 bit int */
 
-static int get_int8(QEMUFile *f, void *pv, size_t size)
+static int get_int8(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     int8_t *v = pv;
     qemu_get_s8s(f, v);
     return 0;
 }
 
-static void put_int8(QEMUFile *f, void *pv, size_t size)
+static int put_int8(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                     QJSON *vmdesc)
 {
     int8_t *v = pv;
     qemu_put_s8s(f, v);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_int8 = {
@@ -505,17 +509,19 @@ const VMStateInfo vmstate_info_int8 = {
 
 /* 16 bit int */
 
-static int get_int16(QEMUFile *f, void *pv, size_t size)
+static int get_int16(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     int16_t *v = pv;
     qemu_get_sbe16s(f, v);
     return 0;
 }
 
-static void put_int16(QEMUFile *f, void *pv, size_t size)
+static int put_int16(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                     QJSON *vmdesc)
 {
     int16_t *v = pv;
     qemu_put_sbe16s(f, v);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_int16 = {
@@ -526,17 +532,19 @@ const VMStateInfo vmstate_info_int16 = {
 
 /* 32 bit int */
 
-static int get_int32(QEMUFile *f, void *pv, size_t size)
+static int get_int32(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     int32_t *v = pv;
     qemu_get_sbe32s(f, v);
     return 0;
 }
 
-static void put_int32(QEMUFile *f, void *pv, size_t size)
+static int put_int32(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                     QJSON *vmdesc)
 {
     int32_t *v = pv;
     qemu_put_sbe32s(f, v);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_int32 = {
@@ -548,7 +556,8 @@ const VMStateInfo vmstate_info_int32 = {
 /* 32 bit int. See that the received value is the same than the one
    in the field */
 
-static int get_int32_equal(QEMUFile *f, void *pv, size_t size)
+static int get_int32_equal(QEMUFile *f, void *pv, size_t size,
+                           VMStateField *field)
 {
     int32_t *v = pv;
     int32_t v2;
@@ -571,7 +580,7 @@ const VMStateInfo vmstate_info_int32_equal = {
  * and less than or equal to the one in the field.
  */
 
-static int get_int32_le(QEMUFile *f, void *pv, size_t size)
+static int get_int32_le(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     int32_t *cur = pv;
     int32_t loaded;
@@ -595,17 +604,19 @@ const VMStateInfo vmstate_info_int32_le = {
 
 /* 64 bit int */
 
-static int get_int64(QEMUFile *f, void *pv, size_t size)
+static int get_int64(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     int64_t *v = pv;
     qemu_get_sbe64s(f, v);
     return 0;
 }
 
-static void put_int64(QEMUFile *f, void *pv, size_t size)
+static int put_int64(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                      QJSON *vmdesc)
 {
     int64_t *v = pv;
     qemu_put_sbe64s(f, v);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_int64 = {
@@ -616,17 +627,19 @@ const VMStateInfo vmstate_info_int64 = {
 
 /* 8 bit unsigned int */
 
-static int get_uint8(QEMUFile *f, void *pv, size_t size)
+static int get_uint8(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     uint8_t *v = pv;
     qemu_get_8s(f, v);
     return 0;
 }
 
-static void put_uint8(QEMUFile *f, void *pv, size_t size)
+static int put_uint8(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                     QJSON *vmdesc)
 {
     uint8_t *v = pv;
     qemu_put_8s(f, v);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_uint8 = {
@@ -637,17 +650,19 @@ const VMStateInfo vmstate_info_uint8 = {
 
 /* 16 bit unsigned int */
 
-static int get_uint16(QEMUFile *f, void *pv, size_t size)
+static int get_uint16(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     uint16_t *v = pv;
     qemu_get_be16s(f, v);
     return 0;
 }
 
-static void put_uint16(QEMUFile *f, void *pv, size_t size)
+static int put_uint16(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                      QJSON *vmdesc)
 {
     uint16_t *v = pv;
     qemu_put_be16s(f, v);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_uint16 = {
@@ -658,17 +673,19 @@ const VMStateInfo vmstate_info_uint16 = {
 
 /* 32 bit unsigned int */
 
-static int get_uint32(QEMUFile *f, void *pv, size_t size)
+static int get_uint32(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     uint32_t *v = pv;
     qemu_get_be32s(f, v);
     return 0;
 }
 
-static void put_uint32(QEMUFile *f, void *pv, size_t size)
+static int put_uint32(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                      QJSON *vmdesc)
 {
     uint32_t *v = pv;
     qemu_put_be32s(f, v);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_uint32 = {
@@ -680,7 +697,8 @@ const VMStateInfo vmstate_info_uint32 = {
 /* 32 bit uint. See that the received value is the same than the one
    in the field */
 
-static int get_uint32_equal(QEMUFile *f, void *pv, size_t size)
+static int get_uint32_equal(QEMUFile *f, void *pv, size_t size,
+                            VMStateField *field)
 {
     uint32_t *v = pv;
     uint32_t v2;
@@ -701,17 +719,19 @@ const VMStateInfo vmstate_info_uint32_equal = {
 
 /* 64 bit unsigned int */
 
-static int get_uint64(QEMUFile *f, void *pv, size_t size)
+static int get_uint64(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     uint64_t *v = pv;
     qemu_get_be64s(f, v);
     return 0;
 }
 
-static void put_uint64(QEMUFile *f, void *pv, size_t size)
+static int put_uint64(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                      QJSON *vmdesc)
 {
     uint64_t *v = pv;
     qemu_put_be64s(f, v);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_uint64 = {
@@ -723,7 +743,8 @@ const VMStateInfo vmstate_info_uint64 = {
 /* 64 bit unsigned int. See that the received value is the same than the one
    in the field */
 
-static int get_uint64_equal(QEMUFile *f, void *pv, size_t size)
+static int get_uint64_equal(QEMUFile *f, void *pv, size_t size,
+                            VMStateField *field)
 {
     uint64_t *v = pv;
     uint64_t v2;
@@ -745,7 +766,8 @@ const VMStateInfo vmstate_info_uint64_equal = {
 /* 8 bit int. See that the received value is the same than the one
    in the field */
 
-static int get_uint8_equal(QEMUFile *f, void *pv, size_t size)
+static int get_uint8_equal(QEMUFile *f, void *pv, size_t size,
+                           VMStateField *field)
 {
     uint8_t *v = pv;
     uint8_t v2;
@@ -767,7 +789,8 @@ const VMStateInfo vmstate_info_uint8_equal = {
 /* 16 bit unsigned int int. See that the received value is the same than the one
    in the field */
 
-static int get_uint16_equal(QEMUFile *f, void *pv, size_t size)
+static int get_uint16_equal(QEMUFile *f, void *pv, size_t size,
+                            VMStateField *field)
 {
     uint16_t *v = pv;
     uint16_t v2;
@@ -788,7 +811,8 @@ const VMStateInfo vmstate_info_uint16_equal = {
 
 /* floating point */
 
-static int get_float64(QEMUFile *f, void *pv, size_t size)
+static int get_float64(QEMUFile *f, void *pv, size_t size,
+                       VMStateField *field)
 {
     float64 *v = pv;
 
@@ -796,11 +820,13 @@ static int get_float64(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void put_float64(QEMUFile *f, void *pv, size_t size)
+static int put_float64(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                       QJSON *vmdesc)
 {
     uint64_t *v = pv;
 
     qemu_put_be64(f, float64_val(*v));
+    return 0;
 }
 
 const VMStateInfo vmstate_info_float64 = {
@@ -811,7 +837,8 @@ const VMStateInfo vmstate_info_float64 = {
 
 /* CPU_DoubleU type */
 
-static int get_cpudouble(QEMUFile *f, void *pv, size_t size)
+static int get_cpudouble(QEMUFile *f, void *pv, size_t size,
+                         VMStateField *field)
 {
     CPU_DoubleU *v = pv;
     qemu_get_be32s(f, &v->l.upper);
@@ -819,11 +846,13 @@ static int get_cpudouble(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void put_cpudouble(QEMUFile *f, void *pv, size_t size)
+static int put_cpudouble(QEMUFile *f, void *pv, size_t size,
+                         VMStateField *field, QJSON *vmdesc)
 {
     CPU_DoubleU *v = pv;
     qemu_put_be32s(f, &v->l.upper);
     qemu_put_be32s(f, &v->l.lower);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_cpudouble = {
@@ -834,17 +863,20 @@ const VMStateInfo vmstate_info_cpudouble = {
 
 /* uint8_t buffers */
 
-static int get_buffer(QEMUFile *f, void *pv, size_t size)
+static int get_buffer(QEMUFile *f, void *pv, size_t size,
+                      VMStateField *field)
 {
     uint8_t *v = pv;
     qemu_get_buffer(f, v, size);
     return 0;
 }
 
-static void put_buffer(QEMUFile *f, void *pv, size_t size)
+static int put_buffer(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                      QJSON *vmdesc)
 {
     uint8_t *v = pv;
     qemu_put_buffer(f, v, size);
+    return 0;
 }
 
 const VMStateInfo vmstate_info_buffer = {
@@ -856,7 +888,8 @@ const VMStateInfo vmstate_info_buffer = {
 /* unused buffers: space that was used for some fields that are
    not useful anymore */
 
-static int get_unused_buffer(QEMUFile *f, void *pv, size_t size)
+static int get_unused_buffer(QEMUFile *f, void *pv, size_t size,
+                             VMStateField *field)
 {
     uint8_t buf[1024];
     int block_len;
@@ -869,7 +902,8 @@ static int get_unused_buffer(QEMUFile *f, void *pv, size_t size)
    return 0;
 }
 
-static void put_unused_buffer(QEMUFile *f, void *pv, size_t size)
+static int put_unused_buffer(QEMUFile *f, void *pv, size_t size,
+                             VMStateField *field, QJSON *vmdesc)
 {
     static const uint8_t buf[1024];
     int block_len;
@@ -879,6 +913,8 @@ static void put_unused_buffer(QEMUFile *f, void *pv, size_t size)
         size -= block_len;
         qemu_put_buffer(f, buf, block_len);
     }
+
+    return 0;
 }
 
 const VMStateInfo vmstate_info_unused_buffer = {
@@ -894,7 +930,7 @@ const VMStateInfo vmstate_info_unused_buffer = {
  */
 /* This is the number of 64 bit words sent over the wire */
 #define BITS_TO_U64S(nr) DIV_ROUND_UP(nr, 64)
-static int get_bitmap(QEMUFile *f, void *pv, size_t size)
+static int get_bitmap(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     unsigned long *bmp = pv;
     int i, idx = 0;
@@ -908,7 +944,8 @@ static int get_bitmap(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void put_bitmap(QEMUFile *f, void *pv, size_t size)
+static int put_bitmap(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                      QJSON *vmdesc)
 {
     unsigned long *bmp = pv;
     int i, idx = 0;
@@ -919,6 +956,8 @@ static void put_bitmap(QEMUFile *f, void *pv, size_t size)
         }
         qemu_put_be64(f, w);
     }
+
+    return 0;
 }
 
 const VMStateInfo vmstate_info_bitmap = {
diff --git a/target/alpha/machine.c b/target/alpha/machine.c
index b99a123..a102645 100644
--- a/target/alpha/machine.c
+++ b/target/alpha/machine.c
@@ -5,17 +5,19 @@
 #include "hw/boards.h"
 #include "migration/cpu.h"
 
-static int get_fpcr(QEMUFile *f, void *opaque, size_t size)
+static int get_fpcr(QEMUFile *f, void *opaque, size_t size, VMStateField *field)
 {
     CPUAlphaState *env = opaque;
     cpu_alpha_store_fpcr(env, qemu_get_be64(f));
     return 0;
 }
 
-static void put_fpcr(QEMUFile *f, void *opaque, size_t size)
+static int put_fpcr(QEMUFile *f, void *opaque, size_t size,
+                    VMStateField *field, QJSON *vmdesc)
 {
     CPUAlphaState *env = opaque;
     qemu_put_be64(f, cpu_alpha_load_fpcr(env));
+    return 0;
 }
 
 static const VMStateInfo vmstate_fpcr = {
diff --git a/target/arm/machine.c b/target/arm/machine.c
index d90943b..487320d 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -17,7 +17,8 @@ static bool vfp_needed(void *opaque)
     return arm_feature(env, ARM_FEATURE_VFP);
 }
 
-static int get_fpscr(QEMUFile *f, void *opaque, size_t size)
+static int get_fpscr(QEMUFile *f, void *opaque, size_t size,
+                     VMStateField *field)
 {
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
@@ -27,12 +28,14 @@ static int get_fpscr(QEMUFile *f, void *opaque, size_t size)
     return 0;
 }
 
-static void put_fpscr(QEMUFile *f, void *opaque, size_t size)
+static int put_fpscr(QEMUFile *f, void *opaque, size_t size,
+                     VMStateField *field, QJSON *vmdesc)
 {
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
 
     qemu_put_be32(f, vfp_get_fpscr(env));
+    return 0;
 }
 
 static const VMStateInfo vmstate_fpscr = {
@@ -163,7 +166,8 @@ static const VMStateDescription vmstate_pmsav7 = {
     }
 };
 
-static int get_cpsr(QEMUFile *f, void *opaque, size_t size)
+static int get_cpsr(QEMUFile *f, void *opaque, size_t size,
+                    VMStateField *field)
 {
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
@@ -180,7 +184,8 @@ static int get_cpsr(QEMUFile *f, void *opaque, size_t size)
     return 0;
 }
 
-static void put_cpsr(QEMUFile *f, void *opaque, size_t size)
+static int put_cpsr(QEMUFile *f, void *opaque, size_t size,
+                    VMStateField *field, QJSON *vmdesc)
 {
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
@@ -193,6 +198,7 @@ static void put_cpsr(QEMUFile *f, void *opaque, size_t size)
     }
 
     qemu_put_be32(f, val);
+    return 0;
 }
 
 static const VMStateInfo vmstate_cpsr = {
diff --git a/target/i386/machine.c b/target/i386/machine.c
index e002b4f..78ae2f9 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -136,10 +136,12 @@ static const VMStateDescription vmstate_mtrr_var = {
 #define VMSTATE_MTRR_VARS(_field, _state, _n, _v)                    \
     VMSTATE_STRUCT_ARRAY(_field, _state, _n, _v, vmstate_mtrr_var, MTRRVar)
 
-static void put_fpreg_error(QEMUFile *f, void *opaque, size_t size)
+static int put_fpreg_error(QEMUFile *f, void *opaque, size_t size,
+                           VMStateField *field, QJSON *vmdesc)
 {
     fprintf(stderr, "call put_fpreg() with invalid arguments\n");
     exit(0);
+    return 0;
 }
 
 /* XXX: add that in a FPU generic layer */
@@ -164,7 +166,8 @@ static void fp64_to_fp80(union x86_longdouble *p, uint64_t temp)
     p->exp = e;
 }
 
-static int get_fpreg(QEMUFile *f, void *opaque, size_t size)
+static int get_fpreg(QEMUFile *f, void *opaque, size_t size,
+                     VMStateField *field)
 {
     FPReg *fp_reg = opaque;
     uint64_t mant;
@@ -176,7 +179,8 @@ static int get_fpreg(QEMUFile *f, void *opaque, size_t size)
     return 0;
 }
 
-static void put_fpreg(QEMUFile *f, void *opaque, size_t size)
+static int put_fpreg(QEMUFile *f, void *opaque, size_t size,
+                     VMStateField *field, QJSON *vmdesc)
 {
     FPReg *fp_reg = opaque;
     uint64_t mant;
@@ -186,6 +190,8 @@ static void put_fpreg(QEMUFile *f, void *opaque, size_t size)
     cpu_get_fp80(&mant, &exp, fp_reg->d);
     qemu_put_be64s(f, &mant);
     qemu_put_be16s(f, &exp);
+
+    return 0;
 }
 
 static const VMStateInfo vmstate_fpreg = {
@@ -194,7 +200,8 @@ static const VMStateInfo vmstate_fpreg = {
     .put  = put_fpreg,
 };
 
-static int get_fpreg_1_mmx(QEMUFile *f, void *opaque, size_t size)
+static int get_fpreg_1_mmx(QEMUFile *f, void *opaque, size_t size,
+                           VMStateField *field)
 {
     union x86_longdouble *p = opaque;
     uint64_t mant;
@@ -211,7 +218,8 @@ static const VMStateInfo vmstate_fpreg_1_mmx = {
     .put  = put_fpreg_error,
 };
 
-static int get_fpreg_1_no_mmx(QEMUFile *f, void *opaque, size_t size)
+static int get_fpreg_1_no_mmx(QEMUFile *f, void *opaque, size_t size,
+                              VMStateField *field)
 {
     union x86_longdouble *p = opaque;
     uint64_t mant;
@@ -273,17 +281,21 @@ static bool less_than_7(void *opaque, int version_id)
     return version_id < 7;
 }
 
-static int get_uint64_as_uint32(QEMUFile *f, void *pv, size_t size)
+static int get_uint64_as_uint32(QEMUFile *f, void *pv, size_t size,
+                                VMStateField *field)
 {
     uint64_t *v = pv;
     *v = qemu_get_be32(f);
     return 0;
 }
 
-static void put_uint64_as_uint32(QEMUFile *f, void *pv, size_t size)
+static int put_uint64_as_uint32(QEMUFile *f, void *pv, size_t size,
+                                VMStateField *field, QJSON *vmdesc)
 {
     uint64_t *v = pv;
     qemu_put_be32(f, *v);
+
+    return 0;
 }
 
 static const VMStateInfo vmstate_hack_uint64_as_uint32 = {
diff --git a/target/mips/machine.c b/target/mips/machine.c
index d20d948..38c8fe9 100644
--- a/target/mips/machine.c
+++ b/target/mips/machine.c
@@ -19,7 +19,7 @@ static int cpu_post_load(void *opaque, int version_id)
 
 /* FPU state */
 
-static int get_fpr(QEMUFile *f, void *pv, size_t size)
+static int get_fpr(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     int i;
     fpr_t *v = pv;
@@ -30,7 +30,8 @@ static int get_fpr(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void put_fpr(QEMUFile *f, void *pv, size_t size)
+static int put_fpr(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                   QJSON *vmdesc)
 {
     int i;
     fpr_t *v = pv;
@@ -38,6 +39,8 @@ static void put_fpr(QEMUFile *f, void *pv, size_t size)
     for (i = 0; i < MSA_WRLEN/64; i++) {
         qemu_put_sbe64s(f, &v->wr.d[i]);
     }
+
+    return 0;
 }
 
 const VMStateInfo vmstate_info_fpr = {
@@ -124,7 +127,7 @@ const VMStateDescription vmstate_mvp = {
 
 /* TLB state */
 
-static int get_tlb(QEMUFile *f, void *pv, size_t size)
+static int get_tlb(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     r4k_tlb_t *v = pv;
     uint16_t flags;
@@ -151,7 +154,8 @@ static int get_tlb(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void put_tlb(QEMUFile *f, void *pv, size_t size)
+static int put_tlb(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                   QJSON *vmdesc)
 {
     r4k_tlb_t *v = pv;
 
@@ -175,6 +179,8 @@ static void put_tlb(QEMUFile *f, void *pv, size_t size)
     qemu_put_be16s(f, &flags);
     qemu_put_be64s(f, &v->PFN[0]);
     qemu_put_be64s(f, &v->PFN[1]);
+
+    return 0;
 }
 
 const VMStateInfo vmstate_info_tlb = {
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 18c16d2..df9f7a4 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -105,7 +105,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-static int get_avr(QEMUFile *f, void *pv, size_t size)
+static int get_avr(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     ppc_avr_t *v = pv;
 
@@ -115,12 +115,14 @@ static int get_avr(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void put_avr(QEMUFile *f, void *pv, size_t size)
+static int put_avr(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                   QJSON *vmdesc)
 {
     ppc_avr_t *v = pv;
 
     qemu_put_be64(f, v->u64[0]);
     qemu_put_be64(f, v->u64[1]);
+    return 0;
 }
 
 static const VMStateInfo vmstate_info_avr = {
@@ -353,7 +355,7 @@ static const VMStateDescription vmstate_sr = {
 };
 
 #ifdef TARGET_PPC64
-static int get_slbe(QEMUFile *f, void *pv, size_t size)
+static int get_slbe(QEMUFile *f, void *pv, size_t size, VMStateField *field)
 {
     ppc_slb_t *v = pv;
 
@@ -363,12 +365,14 @@ static int get_slbe(QEMUFile *f, void *pv, size_t size)
     return 0;
 }
 
-static void put_slbe(QEMUFile *f, void *pv, size_t size)
+static int put_slbe(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+                    QJSON *vmdesc)
 {
     ppc_slb_t *v = pv;
 
     qemu_put_be64(f, v->esid);
     qemu_put_be64(f, v->vsid);
+    return 0;
 }
 
 static const VMStateInfo vmstate_info_slbe = {
diff --git a/target/sparc/machine.c b/target/sparc/machine.c
index 39e262c..6bd6b8e 100644
--- a/target/sparc/machine.c
+++ b/target/sparc/machine.c
@@ -56,7 +56,7 @@ static const VMStateDescription vmstate_tlb_entry = {
 };
 #endif
 
-static int get_psr(QEMUFile *f, void *opaque, size_t size)
+static int get_psr(QEMUFile *f, void *opaque, size_t size, VMStateField *field)
 {
     SPARCCPU *cpu = opaque;
     CPUSPARCState *env = &cpu->env;
@@ -69,7 +69,8 @@ static int get_psr(QEMUFile *f, void *opaque, size_t size)
     return 0;
 }
 
-static void put_psr(QEMUFile *f, void *opaque, size_t size)
+static int put_psr(QEMUFile *f, void *opaque, size_t size, VMStateField *field,
+                   QJSON *vmdesc)
 {
     SPARCCPU *cpu = opaque;
     CPUSPARCState *env = &cpu->env;
@@ -78,6 +79,7 @@ static void put_psr(QEMUFile *f, void *opaque, size_t size)
     val = cpu_get_psr(env);
 
     qemu_put_be32(f, val);
+    return 0;
 }
 
 static const VMStateInfo vmstate_psr = {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 03/15] migration: migrate QTAILQ
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 01/15] MAINTAINERS: Add myself as a migration submaintainer Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 04/15] tests/migration: Add test for QTAILQ migration Dr. David Alan Gilbert (git)
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Jianjun Duan <duanj@linux.vnet.ibm.com>

Currently we cannot directly transfer a QTAILQ instance because of the
limitation in the migration code. Here we introduce an approach to
transfer such structures. We created VMStateInfo vmstate_info_qtailq
for QTAILQ. Similar VMStateInfo can be created for other data structures
such as list.

When a QTAILQ is migrated from source to target, it is appended to the
corresponding QTAILQ structure, which is assumed to have been properly
initialized.

This approach will be used to transfer pending_events and ccs_list in spapr
state.

We also create some macros in qemu/queue.h to access a QTAILQ using pointer
arithmetic. This ensures that we do not depend on the implementation
details about QTAILQ in the migration code.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
Message-Id: <1484852453-12728-3-git-send-email-duanj@linux.vnet.ibm.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/vmstate.h | 20 +++++++++++++
 include/qemu/queue.h        | 60 +++++++++++++++++++++++++++++++++++++++
 migration/trace-events      |  4 +++
 migration/vmstate.c         | 69 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 153 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 7e61b1e..3bbe3ed 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -260,6 +260,7 @@ extern const VMStateInfo vmstate_info_timer;
 extern const VMStateInfo vmstate_info_buffer;
 extern const VMStateInfo vmstate_info_unused_buffer;
 extern const VMStateInfo vmstate_info_bitmap;
+extern const VMStateInfo vmstate_info_qtailq;
 
 #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
 #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
@@ -671,6 +672,25 @@ extern const VMStateInfo vmstate_info_bitmap;
     .offset       = offsetof(_state, _field),                        \
 }
 
+/* For migrating a QTAILQ.
+ * Target QTAILQ needs be properly initialized.
+ * _type: type of QTAILQ element
+ * _next: name of QTAILQ entry field in QTAILQ element
+ * _vmsd: VMSD for QTAILQ element
+ * size: size of QTAILQ element
+ * start: offset of QTAILQ entry in QTAILQ element
+ */
+#define VMSTATE_QTAILQ_V(_field, _state, _version, _vmsd, _type, _next)  \
+{                                                                        \
+    .name         = (stringify(_field)),                                 \
+    .version_id   = (_version),                                          \
+    .vmsd         = &(_vmsd),                                            \
+    .size         = sizeof(_type),                                       \
+    .info         = &vmstate_info_qtailq,                                \
+    .offset       = offsetof(_state, _field),                            \
+    .start        = offsetof(_type, _next),                              \
+}
+
 /* _f : field name
    _f_n : num of elements field_name
    _n : num of elements
diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index 342073f..35292c3 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -438,4 +438,64 @@ struct {                                                                \
 #define QTAILQ_PREV(elm, headname, field) \
         (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
 
+#define field_at_offset(base, offset, type)                                    \
+        ((type) (((char *) (base)) + (offset)))
+
+typedef struct DUMMY_Q_ENTRY DUMMY_Q_ENTRY;
+typedef struct DUMMY_Q DUMMY_Q;
+
+struct DUMMY_Q_ENTRY {
+        QTAILQ_ENTRY(DUMMY_Q_ENTRY) next;
+};
+
+struct DUMMY_Q {
+        QTAILQ_HEAD(DUMMY_Q_HEAD, DUMMY_Q_ENTRY) head;
+};
+
+#define dummy_q ((DUMMY_Q *) 0)
+#define dummy_qe ((DUMMY_Q_ENTRY *) 0)
+
+/*
+ * Offsets of layout of a tail queue head.
+ */
+#define QTAILQ_FIRST_OFFSET (offsetof(typeof(dummy_q->head), tqh_first))
+#define QTAILQ_LAST_OFFSET  (offsetof(typeof(dummy_q->head), tqh_last))
+/*
+ * Raw access of elements of a tail queue
+ */
+#define QTAILQ_RAW_FIRST(head)                                                 \
+        (*field_at_offset(head, QTAILQ_FIRST_OFFSET, void **))
+#define QTAILQ_RAW_TQH_LAST(head)                                              \
+        (*field_at_offset(head, QTAILQ_LAST_OFFSET, void ***))
+
+/*
+ * Offsets of layout of a tail queue element.
+ */
+#define QTAILQ_NEXT_OFFSET (offsetof(typeof(dummy_qe->next), tqe_next))
+#define QTAILQ_PREV_OFFSET (offsetof(typeof(dummy_qe->next), tqe_prev))
+
+/*
+ * Raw access of elements of a tail entry
+ */
+#define QTAILQ_RAW_NEXT(elm, entry)                                            \
+        (*field_at_offset(elm, entry + QTAILQ_NEXT_OFFSET, void **))
+#define QTAILQ_RAW_TQE_PREV(elm, entry)                                        \
+        (*field_at_offset(elm, entry + QTAILQ_PREV_OFFSET, void ***))
+/*
+ * Tail queue tranversal using pointer arithmetic.
+ */
+#define QTAILQ_RAW_FOREACH(elm, head, entry)                                   \
+        for ((elm) = QTAILQ_RAW_FIRST(head);                                   \
+             (elm);                                                            \
+             (elm) = QTAILQ_RAW_NEXT(elm, entry))
+/*
+ * Tail queue insertion using pointer arithmetic.
+ */
+#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do {                          \
+        QTAILQ_RAW_NEXT(elm, entry) = NULL;                                    \
+        QTAILQ_RAW_TQE_PREV(elm, entry) = QTAILQ_RAW_TQH_LAST(head);           \
+        *QTAILQ_RAW_TQH_LAST(head) = (elm);                                    \
+        QTAILQ_RAW_TQH_LAST(head) = &QTAILQ_RAW_NEXT(elm, entry);              \
+} while (/*CONSTCOND*/0)
+
 #endif /* QEMU_SYS_QUEUE_H */
diff --git a/migration/trace-events b/migration/trace-events
index 94134f7..c46f9e9 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -52,6 +52,10 @@ vmstate_n_elems(const char *name, int n_elems) "%s: %d"
 vmstate_subsection_load(const char *parent) "%s"
 vmstate_subsection_load_bad(const char *parent,  const char *sub, const char *sub2) "%s: %s/%s"
 vmstate_subsection_load_good(const char *parent) "%s"
+get_qtailq(const char *name, int version_id) "%s v%d"
+get_qtailq_end(const char *name, const char *reason, int val) "%s %s/%d"
+put_qtailq(const char *name, int version_id) "%s v%d"
+put_qtailq_end(const char *name, const char *reason) "%s %s"
 
 # migration/qemu-file.c
 qemu_file_fclose(void) ""
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 7b4bd6e..2f9d4ba 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -5,6 +5,7 @@
 #include "migration/vmstate.h"
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
+#include "qemu/queue.h"
 #include "trace.h"
 #include "migration/qjson.h"
 
@@ -965,3 +966,71 @@ const VMStateInfo vmstate_info_bitmap = {
     .get = get_bitmap,
     .put = put_bitmap,
 };
+
+/* get for QTAILQ
+ * meta data about the QTAILQ is encoded in a VMStateField structure
+ */
+static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size,
+                      VMStateField *field)
+{
+    int ret = 0;
+    const VMStateDescription *vmsd = field->vmsd;
+    /* size of a QTAILQ element */
+    size_t size = field->size;
+    /* offset of the QTAILQ entry in a QTAILQ element */
+    size_t entry_offset = field->start;
+    int version_id = field->version_id;
+    void *elm;
+
+    trace_get_qtailq(vmsd->name, version_id);
+    if (version_id > vmsd->version_id) {
+        error_report("%s %s",  vmsd->name, "too new");
+        trace_get_qtailq_end(vmsd->name, "too new", -EINVAL);
+
+        return -EINVAL;
+    }
+    if (version_id < vmsd->minimum_version_id) {
+        error_report("%s %s",  vmsd->name, "too old");
+        trace_get_qtailq_end(vmsd->name, "too old", -EINVAL);
+        return -EINVAL;
+    }
+
+    while (qemu_get_byte(f)) {
+        elm = g_malloc(size);
+        ret = vmstate_load_state(f, vmsd, elm, version_id);
+        if (ret) {
+            return ret;
+        }
+        QTAILQ_RAW_INSERT_TAIL(pv, elm, entry_offset);
+    }
+
+    trace_get_qtailq_end(vmsd->name, "end", ret);
+    return ret;
+}
+
+/* put for QTAILQ */
+static int put_qtailq(QEMUFile *f, void *pv, size_t unused_size,
+                      VMStateField *field, QJSON *vmdesc)
+{
+    const VMStateDescription *vmsd = field->vmsd;
+    /* offset of the QTAILQ entry in a QTAILQ element*/
+    size_t entry_offset = field->start;
+    void *elm;
+
+    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);
+    }
+    qemu_put_byte(f, false);
+
+    trace_put_qtailq_end(vmsd->name, "end");
+
+    return 0;
+}
+const VMStateInfo vmstate_info_qtailq = {
+    .name = "qtailq",
+    .get  = get_qtailq,
+    .put  = put_qtailq,
+};
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 04/15] tests/migration: Add test for QTAILQ migration
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (2 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 03/15] migration: migrate QTAILQ Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 05/15] migration: add error_report Dr. David Alan Gilbert (git)
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Jianjun Duan <duanj@linux.vnet.ibm.com>

Add a test for QTAILQ migration to tests/test-vmstate.c.

Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
Message-Id: <1484852453-12728-4-git-send-email-duanj@linux.vnet.ibm.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/test-vmstate.c | 147 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)

diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index d2f529b..9d87faf 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -544,6 +544,150 @@ static void test_arr_ptr_str_no0_load(void)
     }
 }
 
+/* test QTAILQ migration */
+typedef struct TestQtailqElement TestQtailqElement;
+
+struct TestQtailqElement {
+    bool     b;
+    uint8_t  u8;
+    QTAILQ_ENTRY(TestQtailqElement) next;
+};
+
+typedef struct TestQtailq {
+    int16_t  i16;
+    QTAILQ_HEAD(TestQtailqHead, TestQtailqElement) q;
+    int32_t  i32;
+} TestQtailq;
+
+static const VMStateDescription vmstate_q_element = {
+    .name = "test/queue-element",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_BOOL(b, TestQtailqElement),
+        VMSTATE_UINT8(u8, TestQtailqElement),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+static const VMStateDescription vmstate_q = {
+    .name = "test/queue",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_INT16(i16, TestQtailq),
+        VMSTATE_QTAILQ_V(q, TestQtailq, 1, vmstate_q_element, TestQtailqElement,
+                         next),
+        VMSTATE_INT32(i32, TestQtailq),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+uint8_t wire_q[] = {
+    /* i16 */                     0xfe, 0x0,
+    /* start of element 0 of q */ 0x01,
+    /* .b  */                     0x01,
+    /* .u8 */                     0x82,
+    /* start of element 1 of q */ 0x01,
+    /* b */                       0x00,
+    /* u8 */                      0x41,
+    /* end of q */                0x00,
+    /* i32 */                     0x00, 0x01, 0x11, 0x70,
+    QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */
+};
+
+static void test_save_q(void)
+{
+    TestQtailq obj_q = {
+        .i16 = -512,
+        .i32 = 70000,
+    };
+
+    TestQtailqElement obj_qe1 = {
+        .b = true,
+        .u8 = 130,
+    };
+
+    TestQtailqElement obj_qe2 = {
+        .b = false,
+        .u8 = 65,
+    };
+
+    QTAILQ_INIT(&obj_q.q);
+    QTAILQ_INSERT_TAIL(&obj_q.q, &obj_qe1, next);
+    QTAILQ_INSERT_TAIL(&obj_q.q, &obj_qe2, next);
+
+    save_vmstate(&vmstate_q, &obj_q);
+    compare_vmstate(wire_q, sizeof(wire_q));
+}
+
+static void test_load_q(void)
+{
+    TestQtailq obj_q = {
+        .i16 = -512,
+        .i32 = 70000,
+    };
+
+    TestQtailqElement obj_qe1 = {
+        .b = true,
+        .u8 = 130,
+    };
+
+    TestQtailqElement obj_qe2 = {
+        .b = false,
+        .u8 = 65,
+    };
+
+    QTAILQ_INIT(&obj_q.q);
+    QTAILQ_INSERT_TAIL(&obj_q.q, &obj_qe1, next);
+    QTAILQ_INSERT_TAIL(&obj_q.q, &obj_qe2, next);
+
+    QEMUFile *fsave = open_test_file(true);
+
+    qemu_put_buffer(fsave, wire_q, sizeof(wire_q));
+    g_assert(!qemu_file_get_error(fsave));
+    qemu_fclose(fsave);
+
+    QEMUFile *fload = open_test_file(false);
+    TestQtailq tgt;
+
+    QTAILQ_INIT(&tgt.q);
+    vmstate_load_state(fload, &vmstate_q, &tgt, 1);
+    char eof = qemu_get_byte(fload);
+    g_assert(!qemu_file_get_error(fload));
+    g_assert_cmpint(tgt.i16, ==, obj_q.i16);
+    g_assert_cmpint(tgt.i32, ==, obj_q.i32);
+    g_assert_cmpint(eof, ==, QEMU_VM_EOF);
+
+    TestQtailqElement *qele_from = QTAILQ_FIRST(&obj_q.q);
+    TestQtailqElement *qlast_from = QTAILQ_LAST(&obj_q.q, TestQtailqHead);
+    TestQtailqElement *qele_to = QTAILQ_FIRST(&tgt.q);
+    TestQtailqElement *qlast_to = QTAILQ_LAST(&tgt.q, TestQtailqHead);
+
+    while (1) {
+        g_assert_cmpint(qele_to->b, ==, qele_from->b);
+        g_assert_cmpint(qele_to->u8, ==, qele_from->u8);
+        if ((qele_from == qlast_from) || (qele_to == qlast_to)) {
+            break;
+        }
+        qele_from = QTAILQ_NEXT(qele_from, next);
+        qele_to = QTAILQ_NEXT(qele_to, next);
+    }
+
+    g_assert_cmpint((uintptr_t) qele_from, ==, (uintptr_t) qlast_from);
+    g_assert_cmpint((uintptr_t) qele_to, ==, (uintptr_t) qlast_to);
+
+    /* clean up */
+    TestQtailqElement *qele;
+    while (!QTAILQ_EMPTY(&tgt.q)) {
+        qele = QTAILQ_LAST(&tgt.q, TestQtailqHead);
+        QTAILQ_REMOVE(&tgt.q, qele, next);
+        free(qele);
+        qele = NULL;
+    }
+    qemu_fclose(fload);
+}
+
 int main(int argc, char **argv)
 {
     temp_fd = mkstemp(temp_file);
@@ -562,6 +706,9 @@ int main(int argc, char **argv)
                     test_arr_ptr_str_no0_save);
     g_test_add_func("/vmstate/array/ptr/str/no0/load",
                     test_arr_ptr_str_no0_load);
+    g_test_add_func("/vmstate/qtailq/save/saveq", test_save_q);
+    g_test_add_func("/vmstate/qtailq/load/loadq", test_load_q);
+
     g_test_run();
 
     close(temp_fd);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 05/15] migration: add error_report
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (3 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 04/15] tests/migration: Add test for QTAILQ migration Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 06/15] block/vvfat: Remove the undesirable comment Dr. David Alan Gilbert (git)
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Jianjun Duan <duanj@linux.vnet.ibm.com>

Added error_report where version_ids do not match in vmstate_load_state.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
Message-Id: <1484852453-12728-5-git-send-email-duanj@linux.vnet.ibm.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/vmstate.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/migration/vmstate.c b/migration/vmstate.c
index 2f9d4ba..8ddd230 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -85,6 +85,9 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
 
     trace_vmstate_load_state(vmsd->name, version_id);
     if (version_id > vmsd->version_id) {
+        error_report("%s: incoming version_id %d is too new "
+                     "for local version_id %d",
+                     vmsd->name, version_id, vmsd->version_id);
         trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL);
         return -EINVAL;
     }
@@ -95,6 +98,9 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
             trace_vmstate_load_state_end(vmsd->name, "old path", ret);
             return ret;
         }
+        error_report("%s: incoming version_id %d is too old "
+                     "for local minimum version_id  %d",
+                     vmsd->name, version_id, vmsd->minimum_version_id);
         trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL);
         return -EINVAL;
     }
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 06/15] block/vvfat: Remove the undesirable comment
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (4 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 05/15] migration: add error_report Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 07/15] migration: Add a new option to enable only-migratable Dr. David Alan Gilbert (git)
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Ashijeet Acharya <ashijeetacharya@gmail.com>

Remove the "// assert(is_consistent(s))" comment in block/vvfat.c

Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
Message-Id: <1484566314-3987-2-git-send-email-ashijeetacharya@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 block/vvfat.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index ded2109..7b706dc 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1189,7 +1189,6 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
         init_mbr(s, cyls, heads, secs);
     }
 
-    //    assert(is_consistent(s));
     qemu_co_mutex_init(&s->lock);
 
     /* Disable migration when vvfat is used rw */
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 07/15] migration: Add a new option to enable only-migratable
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (5 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 06/15] block/vvfat: Remove the undesirable comment Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 08/15] migration: Allow "device add" options to only add migratable devices Dr. David Alan Gilbert (git)
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Ashijeet Acharya <ashijeetacharya@gmail.com>

Add a new option "--only-migratable" in qemu which will allow to add
only those devices which will not fail qemu after migration. Devices
set with the flag 'unmigratable' cannot be added when this option will
be used.

Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
Message-Id: <1484566314-3987-3-git-send-email-ashijeetacharya@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/migration.h | 3 +++
 qemu-options.hx               | 9 +++++++++
 vl.c                          | 4 ++++
 3 files changed, 16 insertions(+)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index c309d23..40b3697 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -38,6 +38,9 @@
 #define QEMU_VM_COMMAND              0x08
 #define QEMU_VM_SECTION_FOOTER       0x7e
 
+/* for vl.c */
+extern int only_migratable;
+
 struct MigrationParams {
     bool blk;
     bool shared;
diff --git a/qemu-options.hx b/qemu-options.hx
index 780528d..48c7f22 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3585,6 +3585,15 @@ be used to change settings (such as migration parameters) prior to issuing
 the migrate_incoming to allow the migration to begin.
 ETEXI
 
+DEF("only-migratable", 0, QEMU_OPTION_only_migratable, \
+    "-only-migratable     allow only migratable devices\n", QEMU_ARCH_ALL)
+STEXI
+@item -only-migratable
+@findex -only-migratable
+Only allow migratable devices. Devices will not be allowed to enter an
+unmigratable state.
+ETEXI
+
 DEF("nodefaults", 0, QEMU_OPTION_nodefaults, \
     "-nodefaults     don't create default devices\n", QEMU_ARCH_ALL)
 STEXI
diff --git a/vl.c b/vl.c
index abb0900..68e8c00 100644
--- a/vl.c
+++ b/vl.c
@@ -182,6 +182,7 @@ bool boot_strict;
 uint8_t *boot_splash_filedata;
 size_t boot_splash_filedata_size;
 uint8_t qemu_extra_params_fw[2];
+int only_migratable; /* turn it off unless user states otherwise */
 
 int icount_align_option;
 
@@ -3884,6 +3885,9 @@ int main(int argc, char **argv, char **envp)
                 }
                 incoming = optarg;
                 break;
+            case QEMU_OPTION_only_migratable:
+                only_migratable = 1;
+                break;
             case QEMU_OPTION_nodefaults:
                 has_defaults = 0;
                 break;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 08/15] migration: Allow "device add" options to only add migratable devices
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (6 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 07/15] migration: Add a new option to enable only-migratable Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 09/15] migration: disallow migrate_add_blocker during migration Dr. David Alan Gilbert (git)
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Ashijeet Acharya <ashijeetacharya@gmail.com>

Introduce checks for the unmigratable flag in the VMStateDescription
structs of respective devices when user attempts to add them. If the
"--only-migratable" was specified, all unmigratable devices will
rightly fail to add. This feature is made compatible for both "-device"
and "-usbdevice" command line options and covers their hmp and qmp
counterparts as well.

Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
Message-Id: <1484566314-3987-4-git-send-email-ashijeetacharya@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/usb/bus.c   | 19 +++++++++++++++++++
 qdev-monitor.c |  9 +++++++++
 2 files changed, 28 insertions(+)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 25913ad..1dcc35c 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -8,6 +8,7 @@
 #include "monitor/monitor.h"
 #include "trace.h"
 #include "qemu/cutils.h"
+#include "migration/migration.h"
 
 static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
 
@@ -686,6 +687,8 @@ USBDevice *usbdevice_create(const char *cmdline)
     const char *params;
     int len;
     USBDevice *dev;
+    ObjectClass *klass;
+    DeviceClass *dc;
 
     params = strchr(cmdline,':');
     if (params) {
@@ -720,6 +723,22 @@ USBDevice *usbdevice_create(const char *cmdline)
         return NULL;
     }
 
+    klass = object_class_by_name(f->name);
+    if (klass == NULL) {
+        error_report("Device '%s' not found", f->name);
+        return NULL;
+    }
+
+    dc = DEVICE_CLASS(klass);
+
+    if (only_migratable) {
+        if (dc->vmsd->unmigratable) {
+            error_report("Device %s is not migratable, but --only-migratable "
+                         "was specified", f->name);
+            return NULL;
+        }
+    }
+
     if (f->usbdevice_init) {
         dev = f->usbdevice_init(bus, params);
     } else {
diff --git a/qdev-monitor.c b/qdev-monitor.c
index c73410c..81d01df 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -29,6 +29,7 @@
 #include "qemu/error-report.h"
 #include "qemu/help_option.h"
 #include "sysemu/block-backend.h"
+#include "migration/migration.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -577,6 +578,14 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
         return NULL;
     }
 
+    if (only_migratable) {
+        if (dc->vmsd->unmigratable) {
+            error_setg(errp, "Device %s is not migratable, but "
+                       "--only-migratable was specified", driver);
+            return NULL;
+        }
+    }
+
     /* find bus */
     path = qemu_opt_get(opts, "bus");
     if (path != NULL) {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 09/15] migration: disallow migrate_add_blocker during migration
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (7 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 08/15] migration: Allow "device add" options to only add migratable devices Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 10/15] migration: Fail migration blocker for --only-migratable Dr. David Alan Gilbert (git)
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Ashijeet Acharya <ashijeetacharya@gmail.com>

If a migration is already in progress and somebody attempts
to add a migration blocker, this should rightly fail.

Add an errp parameter and a retcode return value to migrate_add_blocker.

Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
Message-Id: <1484566314-3987-5-git-send-email-ashijeetacharya@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
  Merged with recent 'Allow invtsc migration' change
---
 block/qcow.c                  |  8 +++++++-
 block/vdi.c                   |  8 +++++++-
 block/vhdx.c                  | 17 +++++++++++------
 block/vmdk.c                  |  9 ++++++++-
 block/vpc.c                   | 11 ++++++++---
 block/vvfat.c                 | 19 ++++++++++++-------
 hw/9pfs/9p.c                  | 33 ++++++++++++++++++++++-----------
 hw/display/virtio-gpu.c       | 32 +++++++++++++++++++-------------
 hw/intc/arm_gic_kvm.c         | 17 +++++++++++------
 hw/intc/arm_gicv3_its_kvm.c   | 20 +++++++++++++-------
 hw/intc/arm_gicv3_kvm.c       | 19 ++++++++++++-------
 hw/misc/ivshmem.c             | 14 ++++++++++----
 hw/scsi/vhost-scsi.c          | 25 +++++++++++++++++++------
 hw/virtio/vhost.c             |  8 +++++++-
 include/migration/migration.h |  7 ++++++-
 migration/migration.c         | 37 +++++++++++++++++++++++++++++++++++--
 stubs/migr-blocker.c          |  3 ++-
 target/i386/kvm.c             | 16 +++++++++++++---
 18 files changed, 222 insertions(+), 81 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 7540f43..fb738fc 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -104,6 +104,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
     unsigned int len, i, shift;
     int ret;
     QCowHeader header;
+    Error *local_err = NULL;
 
     ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
     if (ret < 0) {
@@ -252,7 +253,12 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
     error_setg(&s->migration_blocker, "The qcow format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    migrate_add_blocker(s->migration_blocker);
+    ret = migrate_add_blocker(s->migration_blocker, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        error_free(s->migration_blocker);
+        goto fail;
+    }
 
     qemu_co_mutex_init(&s->lock);
     return 0;
diff --git a/block/vdi.c b/block/vdi.c
index 96b78d5..0aeb940 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -361,6 +361,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
     VdiHeader header;
     size_t bmap_size;
     int ret;
+    Error *local_err = NULL;
 
     logout("\n");
 
@@ -471,7 +472,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
     error_setg(&s->migration_blocker, "The vdi format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    migrate_add_blocker(s->migration_blocker);
+    ret = migrate_add_blocker(s->migration_blocker, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        error_free(s->migration_blocker);
+        goto fail_free_bmap;
+    }
 
     qemu_co_mutex_init(&s->write_lock);
 
diff --git a/block/vhdx.c b/block/vhdx.c
index 0ba2f0a..68db9e0 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -991,6 +991,17 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
         }
     }
 
+    /* Disable migration when VHDX images are used */
+    error_setg(&s->migration_blocker, "The vhdx format used by node '%s' "
+               "does not support live migration",
+               bdrv_get_device_or_node_name(bs));
+    ret = migrate_add_blocker(s->migration_blocker, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        error_free(s->migration_blocker);
+        goto fail;
+    }
+
     if (flags & BDRV_O_RDWR) {
         ret = vhdx_update_headers(bs, s, false, NULL);
         if (ret < 0) {
@@ -1000,12 +1011,6 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
 
     /* TODO: differencing files */
 
-    /* Disable migration when VHDX images are used */
-    error_setg(&s->migration_blocker, "The vhdx format used by node '%s' "
-               "does not support live migration",
-               bdrv_get_device_or_node_name(bs));
-    migrate_add_blocker(s->migration_blocker);
-
     return 0;
 fail:
     vhdx_close(bs);
diff --git a/block/vmdk.c b/block/vmdk.c
index a11c27a..7750212 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -941,6 +941,7 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
     int ret;
     BDRVVmdkState *s = bs->opaque;
     uint32_t magic;
+    Error *local_err = NULL;
 
     buf = vmdk_read_desc(bs->file, 0, errp);
     if (!buf) {
@@ -976,7 +977,13 @@ static int vmdk_open(BlockDriverState *bs, QDict *options, int flags,
     error_setg(&s->migration_blocker, "The vmdk format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    migrate_add_blocker(s->migration_blocker);
+    ret = migrate_add_blocker(s->migration_blocker, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        error_free(s->migration_blocker);
+        goto fail;
+    }
+
     g_free(buf);
     return 0;
 
diff --git a/block/vpc.c b/block/vpc.c
index 8d5886f..ed6353d 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -422,13 +422,18 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
 #endif
     }
 
-    qemu_co_mutex_init(&s->lock);
-
     /* Disable migration when VHD images are used */
     error_setg(&s->migration_blocker, "The vpc format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    migrate_add_blocker(s->migration_blocker);
+    ret = migrate_add_blocker(s->migration_blocker, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        error_free(s->migration_blocker);
+        goto fail;
+    }
+
+    qemu_co_mutex_init(&s->lock);
 
     return 0;
 
diff --git a/block/vvfat.c b/block/vvfat.c
index 7b706dc..c6bf67e 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1185,21 +1185,26 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
 
     s->sector_count = s->faked_sectors + s->sectors_per_cluster*s->cluster_count;
 
-    if (s->first_sectors_number == 0x40) {
-        init_mbr(s, cyls, heads, secs);
-    }
-
-    qemu_co_mutex_init(&s->lock);
-
     /* Disable migration when vvfat is used rw */
     if (s->qcow) {
         error_setg(&s->migration_blocker,
                    "The vvfat (rw) format used by node '%s' "
                    "does not support live migration",
                    bdrv_get_device_or_node_name(bs));
-        migrate_add_blocker(s->migration_blocker);
+        ret = migrate_add_blocker(s->migration_blocker, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            error_free(s->migration_blocker);
+            goto fail;
+        }
     }
 
+    if (s->first_sectors_number == 0x40) {
+        init_mbr(s, cyls, heads, secs);
+    }
+
+    qemu_co_mutex_init(&s->lock);
+
     ret = 0;
 fail:
     qemu_opts_del(opts);
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index fa58877..06b6e7e 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -979,6 +979,7 @@ static void coroutine_fn v9fs_attach(void *opaque)
     size_t offset = 7;
     V9fsQID qid;
     ssize_t err;
+    Error *local_err = NULL;
 
     v9fs_string_init(&uname);
     v9fs_string_init(&aname);
@@ -1007,26 +1008,36 @@ static void coroutine_fn v9fs_attach(void *opaque)
         clunk_fid(s, fid);
         goto out;
     }
-    err = pdu_marshal(pdu, offset, "Q", &qid);
-    if (err < 0) {
-        clunk_fid(s, fid);
-        goto out;
-    }
-    err += offset;
-    memcpy(&s->root_qid, &qid, sizeof(qid));
-    trace_v9fs_attach_return(pdu->tag, pdu->id,
-                             qid.type, qid.version, qid.path);
+
     /*
      * disable migration if we haven't done already.
      * attach could get called multiple times for the same export.
      */
     if (!s->migration_blocker) {
-        s->root_fid = fid;
         error_setg(&s->migration_blocker,
                    "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'",
                    s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
-        migrate_add_blocker(s->migration_blocker);
+        err = migrate_add_blocker(s->migration_blocker, &local_err);
+        if (local_err) {
+            error_free(local_err);
+            error_free(s->migration_blocker);
+            s->migration_blocker = NULL;
+            clunk_fid(s, fid);
+            goto out;
+        }
+        s->root_fid = fid;
+    }
+
+    err = pdu_marshal(pdu, offset, "Q", &qid);
+    if (err < 0) {
+        clunk_fid(s, fid);
+        goto out;
     }
+    err += offset;
+
+    memcpy(&s->root_qid, &qid, sizeof(qid));
+    trace_v9fs_attach_return(pdu->tag, pdu->id,
+                             qid.type, qid.version, qid.path);
 out:
     put_fid(pdu, fidp);
 out_nofid:
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 6e09474..444ca06 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1136,6 +1136,7 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
     VirtIOGPU *g = VIRTIO_GPU(qdev);
     bool have_virgl;
+    Error *local_err = NULL;
     int i;
 
     if (g->conf.max_outputs > VIRTIO_GPU_MAX_SCANOUTS) {
@@ -1143,14 +1144,6 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
         return;
     }
 
-    g->config_size = sizeof(struct virtio_gpu_config);
-    g->virtio_config.num_scanouts = g->conf.max_outputs;
-    virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
-                g->config_size);
-
-    g->req_state[0].width = 1024;
-    g->req_state[0].height = 768;
-
     g->use_virgl_renderer = false;
 #if !defined(CONFIG_VIRGL) || defined(HOST_WORDS_BIGENDIAN)
     have_virgl = false;
@@ -1162,6 +1155,24 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
     }
 
     if (virtio_gpu_virgl_enabled(g->conf)) {
+        error_setg(&g->migration_blocker, "virgl is not yet migratable");
+        migrate_add_blocker(g->migration_blocker, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            error_free(g->migration_blocker);
+            return;
+        }
+    }
+
+    g->config_size = sizeof(struct virtio_gpu_config);
+    g->virtio_config.num_scanouts = g->conf.max_outputs;
+    virtio_init(VIRTIO_DEVICE(g), "virtio-gpu", VIRTIO_ID_GPU,
+                g->config_size);
+
+    g->req_state[0].width = 1024;
+    g->req_state[0].height = 768;
+
+    if (virtio_gpu_virgl_enabled(g->conf)) {
         /* use larger control queue in 3d mode */
         g->ctrl_vq   = virtio_add_queue(vdev, 256, virtio_gpu_handle_ctrl_cb);
         g->cursor_vq = virtio_add_queue(vdev, 16, virtio_gpu_handle_cursor_cb);
@@ -1187,11 +1198,6 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
             dpy_gfx_replace_surface(g->scanout[i].con, NULL);
         }
     }
-
-    if (virtio_gpu_virgl_enabled(g->conf)) {
-        error_setg(&g->migration_blocker, "virgl is not yet migratable");
-        migrate_add_blocker(g->migration_blocker);
-    }
 }
 
 static void virtio_gpu_device_unrealize(DeviceState *qdev, Error **errp)
diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
index 11729ee..ec952ec 100644
--- a/hw/intc/arm_gic_kvm.c
+++ b/hw/intc/arm_gic_kvm.c
@@ -510,6 +510,17 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (!kvm_arm_gic_can_save_restore(s)) {
+        error_setg(&s->migration_blocker, "This operating system kernel does "
+                                          "not support vGICv2 migration");
+        migrate_add_blocker(s->migration_blocker, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            error_free(s->migration_blocker);
+            return;
+        }
+    }
+
     gic_init_irqs_and_mmio(s, kvm_arm_gicv2_set_irq, NULL);
 
     for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) {
@@ -558,12 +569,6 @@ static void kvm_arm_gic_realize(DeviceState *dev, Error **errp)
                             KVM_VGIC_V2_ADDR_TYPE_CPU,
                             s->dev_fd);
 
-    if (!kvm_arm_gic_can_save_restore(s)) {
-        error_setg(&s->migration_blocker, "This operating system kernel does "
-                                          "not support vGICv2 migration");
-        migrate_add_blocker(s->migration_blocker);
-    }
-
     if (kvm_has_gsi_routing()) {
         /* set up irq routing */
         kvm_init_irq_routing(kvm_state);
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index fc246e0..bd4f3aa 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -56,6 +56,19 @@ static int kvm_its_send_msi(GICv3ITSState *s, uint32_t value, uint16_t devid)
 static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
 {
     GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);
+    Error *local_err = NULL;
+
+    /*
+     * Block migration of a KVM GICv3 ITS device: the API for saving and
+     * restoring the state in the kernel is not yet available
+     */
+    error_setg(&s->migration_blocker, "vITS migration is not implemented");
+    migrate_add_blocker(s->migration_blocker, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        error_free(s->migration_blocker);
+        return;
+    }
 
     s->dev_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_ITS, false);
     if (s->dev_fd < 0) {
@@ -73,13 +86,6 @@ static void kvm_arm_its_realize(DeviceState *dev, Error **errp)
 
     gicv3_its_init_mmio(s, NULL);
 
-    /*
-     * Block migration of a KVM GICv3 ITS device: the API for saving and
-     * restoring the state in the kernel is not yet available
-     */
-    error_setg(&s->migration_blocker, "vITS migration is not implemented");
-    migrate_add_blocker(s->migration_blocker);
-
     kvm_msi_use_devid = true;
     kvm_gsi_direct_mapping = false;
     kvm_msi_via_irqfd_allowed = kvm_irqfds_enabled();
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 199a439..d69dc47 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -103,6 +103,18 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
 
     gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
 
+    /* Block migration of a KVM GICv3 device: the API for saving and restoring
+     * the state in the kernel is not yet finalised in the kernel or
+     * implemented in QEMU.
+     */
+    error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
+    migrate_add_blocker(s->migration_blocker, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        error_free(s->migration_blocker);
+        return;
+    }
+
     /* Try to create the device via the device control API */
     s->dev_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false);
     if (s->dev_fd < 0) {
@@ -122,13 +134,6 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp)
     kvm_arm_register_device(&s->iomem_redist, -1, KVM_DEV_ARM_VGIC_GRP_ADDR,
                             KVM_VGIC_V3_ADDR_TYPE_REDIST, s->dev_fd);
 
-    /* Block migration of a KVM GICv3 device: the API for saving and restoring
-     * the state in the kernel is not yet finalised in the kernel or
-     * implemented in QEMU.
-     */
-    error_setg(&s->migration_blocker, "vGICv3 migration is not implemented");
-    migrate_add_blocker(s->migration_blocker);
-
     if (kvm_has_gsi_routing()) {
         /* set up irq routing */
         kvm_init_irq_routing(kvm_state);
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index abeaf3d..fd14d7a 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -840,6 +840,7 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
     uint8_t *pci_conf;
     uint8_t attr = PCI_BASE_ADDRESS_SPACE_MEMORY |
         PCI_BASE_ADDRESS_MEM_PREFETCH;
+    Error *local_err = NULL;
 
     /* IRQFD requires MSI */
     if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) &&
@@ -903,9 +904,6 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
         }
     }
 
-    vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
-    pci_register_bar(PCI_DEVICE(s), 2, attr, s->ivshmem_bar2);
-
     if (s->master == ON_OFF_AUTO_AUTO) {
         s->master = s->vm_id == 0 ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
     }
@@ -913,8 +911,16 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp)
     if (!ivshmem_is_master(s)) {
         error_setg(&s->migration_blocker,
                    "Migration is disabled when using feature 'peer mode' in device 'ivshmem'");
-        migrate_add_blocker(s->migration_blocker);
+        migrate_add_blocker(s->migration_blocker, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            error_free(s->migration_blocker);
+            return;
+        }
     }
+
+    vmstate_register_ram(s->ivshmem_bar2, DEVICE(s));
+    pci_register_bar(PCI_DEVICE(s), 2, attr, s->ivshmem_bar2);
 }
 
 static void ivshmem_exit(PCIDevice *dev)
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 5b26946..c491ece 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -238,8 +238,16 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
                                vhost_dummy_handle_output);
     if (err != NULL) {
         error_propagate(errp, err);
-        close(vhostfd);
-        return;
+        goto close_fd;
+    }
+
+    error_setg(&s->migration_blocker,
+               "vhost-scsi does not support migration");
+    migrate_add_blocker(s->migration_blocker, &err);
+    if (err) {
+        error_propagate(errp, err);
+        error_free(s->migration_blocker);
+        goto close_fd;
     }
 
     s->dev.nvqs = VHOST_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
@@ -252,7 +260,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
     if (ret < 0) {
         error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
                    strerror(-ret));
-        return;
+        goto free_vqs;
     }
 
     /* At present, channel and lun both are 0 for bootable vhost-scsi disk */
@@ -261,9 +269,14 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
     /* Note: we can also get the minimum tpgt from kernel */
     s->target = vs->conf.boot_tpgt;
 
-    error_setg(&s->migration_blocker,
-            "vhost-scsi does not support migration");
-    migrate_add_blocker(s->migration_blocker);
+    return;
+
+ free_vqs:
+    migrate_del_blocker(s->migration_blocker);
+    g_free(s->dev.vqs);
+ close_fd:
+    close(vhostfd);
+    return;
 }
 
 static void vhost_scsi_unrealize(DeviceState *dev, Error **errp)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 9cacf55..b124d97 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1176,6 +1176,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
 {
     uint64_t features;
     int i, r, n_initialized_vqs = 0;
+    Error *local_err = NULL;
 
     hdev->vdev = NULL;
     hdev->migration_blocker = NULL;
@@ -1256,7 +1257,12 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
     }
 
     if (hdev->migration_blocker != NULL) {
-        migrate_add_blocker(hdev->migration_blocker);
+        r = migrate_add_blocker(hdev->migration_blocker, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
+            error_free(hdev->migration_blocker);
+            goto fail_busyloop;
+        }
     }
 
     hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 40b3697..bcbdb03 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -243,6 +243,7 @@ void remove_migration_state_change_notifier(Notifier *notify);
 MigrationState *migrate_init(const MigrationParams *params);
 bool migration_is_blocked(Error **errp);
 bool migration_in_setup(MigrationState *);
+bool migration_is_idle(MigrationState *s);
 bool migration_has_finished(MigrationState *);
 bool migration_has_failed(MigrationState *);
 /* True if outgoing migration has entered postcopy phase */
@@ -287,8 +288,12 @@ int ram_postcopy_incoming_init(MigrationIncomingState *mis);
  * @migrate_add_blocker - prevent migration from proceeding
  *
  * @reason - an error to be returned whenever migration is attempted
+ *
+ * @errp - [out] The reason (if any) we cannot block migration right now.
+ *
+ * @returns - 0 on success, -EBUSY on failure, with errp set.
  */
-void migrate_add_blocker(Error *reason);
+int migrate_add_blocker(Error *reason, Error **errp);
 
 /**
  * @migrate_del_blocker - remove a blocking error from migration
diff --git a/migration/migration.c b/migration/migration.c
index f498ab8..0d88286 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1044,6 +1044,31 @@ bool migration_in_postcopy_after_devices(MigrationState *s)
     return migration_in_postcopy(s) && s->postcopy_after_devices;
 }
 
+bool migration_is_idle(MigrationState *s)
+{
+    if (!s) {
+        s = migrate_get_current();
+    }
+
+    switch (s->state) {
+    case MIGRATION_STATUS_NONE:
+    case MIGRATION_STATUS_CANCELLED:
+    case MIGRATION_STATUS_COMPLETED:
+    case MIGRATION_STATUS_FAILED:
+        return true;
+    case MIGRATION_STATUS_SETUP:
+    case MIGRATION_STATUS_CANCELLING:
+    case MIGRATION_STATUS_ACTIVE:
+    case MIGRATION_STATUS_POSTCOPY_ACTIVE:
+    case MIGRATION_STATUS_COLO:
+        return false;
+    case MIGRATION_STATUS__MAX:
+        g_assert_not_reached();
+    }
+
+    return false;
+}
+
 MigrationState *migrate_init(const MigrationParams *params)
 {
     MigrationState *s = migrate_get_current();
@@ -1086,9 +1111,17 @@ MigrationState *migrate_init(const MigrationParams *params)
 
 static GSList *migration_blockers;
 
-void migrate_add_blocker(Error *reason)
+int migrate_add_blocker(Error *reason, Error **errp)
 {
-    migration_blockers = g_slist_prepend(migration_blockers, reason);
+    if (migration_is_idle(NULL)) {
+        migration_blockers = g_slist_prepend(migration_blockers, reason);
+        return 0;
+    }
+
+    error_propagate(errp, error_copy(reason));
+    error_prepend(errp, "disallowing migration blocker (migration in "
+                      "progress) for: ");
+    return -EBUSY;
 }
 
 void migrate_del_blocker(Error *reason)
diff --git a/stubs/migr-blocker.c b/stubs/migr-blocker.c
index 8ab3604..a5ba18f 100644
--- a/stubs/migr-blocker.c
+++ b/stubs/migr-blocker.c
@@ -2,8 +2,9 @@
 #include "qemu-common.h"
 #include "migration/migration.h"
 
-void migrate_add_blocker(Error *reason)
+int migrate_add_blocker(Error *reason, Error **errp)
 {
+    return 0;
 }
 
 void migrate_del_blocker(Error *reason)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 3b52821..8e130cc 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -710,6 +710,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     uint32_t signature[3];
     int kvm_base = KVM_CPUID_SIGNATURE;
     int r;
+    Error *local_err = NULL;
 
     memset(&cpuid_data, 0, sizeof(cpuid_data));
 
@@ -970,7 +971,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
             error_setg(&invtsc_mig_blocker,
                        "State blocked by non-migratable CPU device"
                        " (invtsc flag)");
-            migrate_add_blocker(invtsc_mig_blocker);
+            r = migrate_add_blocker(invtsc_mig_blocker, &local_err);
+            if (local_err) {
+                error_report_err(local_err);
+                error_free(invtsc_mig_blocker);
+                goto fail;
+            }
             /* for savevm */
             vmstate_x86_cpu.unmigratable = 1;
         }
@@ -979,12 +985,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
     cpuid_data.cpuid.padding = 0;
     r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
     if (r) {
-        return r;
+        goto fail;
     }
 
     r = kvm_arch_set_tsc_khz(cs);
     if (r < 0) {
-        return r;
+        goto fail;
     }
 
     /* vcpu's TSC frequency is either specified by user, or following
@@ -1011,6 +1017,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
     }
 
     return 0;
+
+ fail:
+    migrate_del_blocker(invtsc_mig_blocker);
+    return r;
 }
 
 void kvm_arch_reset_vcpu(X86CPU *cpu)
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 10/15] migration: Fail migration blocker for --only-migratable
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (8 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 09/15] migration: disallow migrate_add_blocker during migration Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 11/15] migration: re-active images while migration been canceled after inactive them Dr. David Alan Gilbert (git)
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Ashijeet Acharya <ashijeetacharya@gmail.com>

migrate_add_blocker should rightly fail if the '--only-migratable'
option was specified and the device in use should not be able to
perform the action which results in an unmigratable VM.

Make migrate_add_blocker return -EACCES in this case.

Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
Message-Id: <1484566314-3987-6-git-send-email-ashijeetacharya@gmail.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/migration.h | 2 +-
 migration/migration.c         | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index bcbdb03..7881e89 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -291,7 +291,7 @@ int ram_postcopy_incoming_init(MigrationIncomingState *mis);
  *
  * @errp - [out] The reason (if any) we cannot block migration right now.
  *
- * @returns - 0 on success, -EBUSY on failure, with errp set.
+ * @returns - 0 on success, -EBUSY/-EACCES on failure, with errp set.
  */
 int migrate_add_blocker(Error *reason, Error **errp);
 
diff --git a/migration/migration.c b/migration/migration.c
index 0d88286..7dcb7d7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1113,6 +1113,13 @@ static GSList *migration_blockers;
 
 int migrate_add_blocker(Error *reason, Error **errp)
 {
+    if (only_migratable) {
+        error_propagate(errp, error_copy(reason));
+        error_prepend(errp, "disallowing migration blocker "
+                          "(--only_migratable) for: ");
+        return -EACCES;
+    }
+
     if (migration_is_idle(NULL)) {
         migration_blockers = g_slist_prepend(migration_blockers, reason);
         return 0;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 11/15] migration: re-active images while migration been canceled after inactive them
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (9 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 10/15] migration: Fail migration blocker for --only-migratable Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 12/15] migration: Change name of live migration thread Dr. David Alan Gilbert (git)
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: zhanghailiang <zhang.zhanghailiang@huawei.com>

commit fe904ea8242cbae2d7e69c052c754b8f5f1ba1d6 fixed a case
which migration aborted QEMU because it didn't regain the control
of images while some errors happened.

Actually, there are another two cases can trigger the same error reports:
" bdrv_co_do_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed",

Case 1, codes path:
migration_thread()
    migration_completion()
        bdrv_inactivate_all() ----------------> inactivate images
        qemu_savevm_state_complete_precopy()
            socket_writev_buffer() --------> error because destination fails
                qemu_fflush() ----------------> set error on migration stream
-> qmp_migrate_cancel() ----------------> user cancelled migration concurrently
    -> migrate_set_state() ------------------> set migrate CANCELLIN
    migration_completion() -----------------> go on to fail_invalidate
	if (s->state == MIGRATION_STATUS_ACTIVE) -> Jump this branch

Case 2, codes path:
migration_thread()
    migration_completion()
        bdrv_inactivate_all() ----------------> inactivate images
    migreation_completion() finished
-> qmp_migrate_cancel() ---------------> user cancelled migration concurrently
    qemu_mutex_lock_iothread();
    qemu_bh_schedule (s->cleanup_bh);

As we can see from above, qmp_migrate_cancel can slip in whenever
migration_thread does not hold the global lock. If this happens after
bdrv_inactive_all() been called, the above error reports will appear.

To prevent this, we can call bdrv_invalidate_cache_all() in qmp_migrate_cancel()
directly if we find images become inactive.

Besides, bdrv_invalidate_cache_all() in migration_completion() doesn't have the
protection of big lock, fix it by add the missing qemu_mutex_lock_iothread();

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Message-Id: <1485244792-11248-1-git-send-email-zhang.zhanghailiang@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/migration.h |  3 +++
 migration/migration.c         | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index 7881e89..af9135f 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -180,6 +180,9 @@ struct MigrationState
     /* Flag set once the migration thread is running (and needs joining) */
     bool migration_thread_running;
 
+    /* Flag set once the migration thread called bdrv_inactivate_all */
+    bool block_inactive;
+
     /* Queue of outstanding page requests from the destination */
     QemuMutex src_page_req_mutex;
     QSIMPLEQ_HEAD(src_page_requests, MigrationSrcPageRequest) src_page_requests;
diff --git a/migration/migration.c b/migration/migration.c
index 7dcb7d7..f8a4500 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1006,6 +1006,16 @@ static void migrate_fd_cancel(MigrationState *s)
     if (s->state == MIGRATION_STATUS_CANCELLING && f) {
         qemu_file_shutdown(f);
     }
+    if (s->state == MIGRATION_STATUS_CANCELLING && s->block_inactive) {
+        Error *local_err = NULL;
+
+        bdrv_invalidate_cache_all(&local_err);
+        if (local_err) {
+            error_report_err(local_err);
+        } else {
+            s->block_inactive = false;
+        }
+    }
 }
 
 void add_migration_state_change_notifier(Notifier *notify)
@@ -1745,6 +1755,7 @@ static void migration_completion(MigrationState *s, int current_active_state,
             if (ret >= 0) {
                 qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
                 qemu_savevm_state_complete_precopy(s->to_dst_file, false);
+                s->block_inactive = true;
             }
         }
         qemu_mutex_unlock_iothread();
@@ -1795,10 +1806,14 @@ fail_invalidate:
     if (s->state == MIGRATION_STATUS_ACTIVE) {
         Error *local_err = NULL;
 
+        qemu_mutex_lock_iothread();
         bdrv_invalidate_cache_all(&local_err);
         if (local_err) {
             error_report_err(local_err);
+        } else {
+            s->block_inactive = false;
         }
+        qemu_mutex_unlock_iothread();
     }
 
 fail:
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 12/15] migration: Change name of live migration thread
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (10 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 11/15] migration: re-active images while migration been canceled after inactive them Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 13/15] PCI/migration merge vmstate_pci_device and vmstate_pcie_device Dr. David Alan Gilbert (git)
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Pankaj Gupta <pagupta@redhat.com>

Change the name of live migration thread from 'migration'
to 'live_migration' to identify it clearly. 'migration'
is a generic word and kernel also has  tasks for process
migration with the name 'migration/cpu#'.

Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
Message-Id: <1485178976-15225-1-git-send-email-pagupta@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index f8a4500..2766d2f 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2024,7 +2024,7 @@ void migrate_fd_connect(MigrationState *s)
     }
 
     migrate_compress_threads_create();
-    qemu_thread_create(&s->thread, "migration", migration_thread, s,
+    qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
                        QEMU_THREAD_JOINABLE);
     s->migration_thread_running = true;
 }
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 13/15] PCI/migration merge vmstate_pci_device and vmstate_pcie_device
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (11 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 12/15] migration: Change name of live migration thread Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 14/15] migration: transform remaining DPRINTF into trace_ Dr. David Alan Gilbert (git)
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The vmstate_pci_device and vmstate_pcie_devices differ
just in the size of one buffer; combine the two using a _TEST
macro.

I think this is safe as long as everywhere which currently
uses either of these two uses the right type.

One thing that concerns me is that some places use pci_device_load/save
which does some irq mangling, but others just use the VMSTATE_PCI_DEVICE
macro - how are they getting the same irq mangling?

This passes a smoke test migrate of:
./x86_64-softmmu/qemu-system-x86_64 -M pc,accel=kvm -m 1024
./littlefed20.img -device e1000e -device virtio-net -device
e1000 -device virtio-rng -device megasas -device megasas-gen2 -device
ioh3420 -device nec-usb-xhci

to an unmodified qemu.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20161214195829.18241-1-dgilbert@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/net/e1000e.c                    |  2 +-
 hw/net/vmxnet3.c                   |  2 +-
 hw/pci-bridge/ioh3420.c            |  2 +-
 hw/pci-bridge/xio3130_downstream.c |  2 +-
 hw/pci-bridge/xio3130_upstream.c   |  2 +-
 hw/pci/pci.c                       | 41 +++++++++++++++++---------------------
 hw/scsi/megasas.c                  |  2 +-
 hw/scsi/vmw_pvscsi.c               |  2 +-
 hw/usb/hcd-xhci.c                  |  2 +-
 include/hw/pci/pcie.h              | 10 ----------
 10 files changed, 26 insertions(+), 41 deletions(-)

diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 77a4b3e..0e9a25b 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -593,7 +593,7 @@ static const VMStateDescription e1000e_vmstate = {
     .pre_save = e1000e_pre_save,
     .post_load = e1000e_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_PCIE_DEVICE(parent_obj, E1000EState),
+        VMSTATE_PCI_DEVICE(parent_obj, E1000EState),
         VMSTATE_MSIX(parent_obj, E1000EState),
 
         VMSTATE_UINT32(ioaddr, E1000EState),
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 4163ca8..2cb2731 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2631,7 +2631,7 @@ static const VMStateDescription vmstate_vmxnet3_pcie_device = {
     .minimum_version_id = 1,
     .needed = vmxnet3_vmstate_need_pcie_device,
     .fields = (VMStateField[]) {
-        VMSTATE_PCIE_DEVICE(parent_obj, VMXNET3State),
+        VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index 84b7946..0eef87a 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -180,7 +180,7 @@ static const VMStateDescription vmstate_ioh3420 = {
     .minimum_version_id = 1,
     .post_load = pcie_cap_slot_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
+        VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
         VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
                        PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
         VMSTATE_END_OF_LIST()
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index 04b8e5b..cfe8a36 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -166,7 +166,7 @@ static const VMStateDescription vmstate_xio3130_downstream = {
     .minimum_version_id = 1,
     .post_load = pcie_cap_slot_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
+        VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
         VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
                        PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
         VMSTATE_END_OF_LIST()
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index d1f59c8..401c784 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -138,7 +138,7 @@ static const VMStateDescription vmstate_xio3130_upstream = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
-        VMSTATE_PCIE_DEVICE(parent_obj.parent_obj, PCIEPort),
+        VMSTATE_PCI_DEVICE(parent_obj.parent_obj, PCIEPort),
         VMSTATE_STRUCT(parent_obj.parent_obj.exp.aer_log, PCIEPort, 0,
                        vmstate_pcie_aer_log, PCIEAERLog),
         VMSTATE_END_OF_LIST()
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index c35d011..47ca3af 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -542,30 +542,29 @@ static VMStateInfo vmstate_info_pci_irq_state = {
     .put  = put_pci_irq_state,
 };
 
+static bool migrate_is_pcie(void *opaque, int version_id)
+{
+    return pci_is_express((PCIDevice *)opaque);
+}
+
+static bool migrate_is_not_pcie(void *opaque, int version_id)
+{
+    return !pci_is_express((PCIDevice *)opaque);
+}
+
 const VMStateDescription vmstate_pci_device = {
     .name = "PCIDevice",
     .version_id = 2,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
         VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
-        VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
-                                   vmstate_info_pci_config,
+        VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
+                                   migrate_is_not_pcie,
+                                   0, vmstate_info_pci_config,
                                    PCI_CONFIG_SPACE_SIZE),
-        VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
-				   vmstate_info_pci_irq_state,
-				   PCI_NUM_PINS * sizeof(int32_t)),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-const VMStateDescription vmstate_pcie_device = {
-    .name = "PCIEDevice",
-    .version_id = 2,
-    .minimum_version_id = 1,
-    .fields = (VMStateField[]) {
-        VMSTATE_INT32_POSITIVE_LE(version_id, PCIDevice),
-        VMSTATE_BUFFER_UNSAFE_INFO(config, PCIDevice, 0,
-                                   vmstate_info_pci_config,
+        VMSTATE_BUFFER_UNSAFE_INFO_TEST(config, PCIDevice,
+                                   migrate_is_pcie,
+                                   0, vmstate_info_pci_config,
                                    PCIE_CONFIG_SPACE_SIZE),
         VMSTATE_BUFFER_UNSAFE_INFO(irq_state, PCIDevice, 2,
 				   vmstate_info_pci_irq_state,
@@ -574,10 +573,6 @@ const VMStateDescription vmstate_pcie_device = {
     }
 };
 
-static inline const VMStateDescription *pci_get_vmstate(PCIDevice *s)
-{
-    return pci_is_express(s) ? &vmstate_pcie_device : &vmstate_pci_device;
-}
 
 void pci_device_save(PCIDevice *s, QEMUFile *f)
 {
@@ -586,7 +581,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
      * This makes us compatible with old devices
      * which never set or clear this bit. */
     s->config[PCI_STATUS] &= ~PCI_STATUS_INTERRUPT;
-    vmstate_save_state(f, pci_get_vmstate(s), s, NULL);
+    vmstate_save_state(f, &vmstate_pci_device, s, NULL);
     /* Restore the interrupt status bit. */
     pci_update_irq_status(s);
 }
@@ -594,7 +589,7 @@ void pci_device_save(PCIDevice *s, QEMUFile *f)
 int pci_device_load(PCIDevice *s, QEMUFile *f)
 {
     int ret;
-    ret = vmstate_load_state(f, pci_get_vmstate(s), s, s->version_id);
+    ret = vmstate_load_state(f, &vmstate_pci_device, s, s->version_id);
     /* Restore the interrupt status bit. */
     pci_update_irq_status(s);
     return ret;
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 6233865..6aad7c9 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2289,7 +2289,7 @@ static const VMStateDescription vmstate_megasas_gen2 = {
     .minimum_version_id = 0,
     .minimum_version_id_old = 0,
     .fields      = (VMStateField[]) {
-        VMSTATE_PCIE_DEVICE(parent_obj, MegasasState),
+        VMSTATE_PCI_DEVICE(parent_obj, MegasasState),
         VMSTATE_MSIX(parent_obj, MegasasState),
 
         VMSTATE_INT32(fw_state, MegasasState),
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index a5ce7de..7557546 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1207,7 +1207,7 @@ static const VMStateDescription vmstate_pvscsi_pcie_device = {
     .name = "pvscsi/pcie",
     .needed = pvscsi_vmstate_need_pcie_device,
     .fields = (VMStateField[]) {
-        VMSTATE_PCIE_DEVICE(parent_obj, PVSCSIState),
+        VMSTATE_PCI_DEVICE(parent_obj, PVSCSIState),
         VMSTATE_END_OF_LIST()
     }
 };
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4acf0c6..e0b5169 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3894,7 +3894,7 @@ static const VMStateDescription vmstate_xhci = {
     .version_id = 1,
     .post_load = usb_xhci_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_PCIE_DEVICE(parent_obj, XHCIState),
+        VMSTATE_PCI_DEVICE(parent_obj, XHCIState),
         VMSTATE_MSIX(parent_obj, XHCIState),
 
         VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1,
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index b08451d..163c519 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -125,16 +125,6 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offset, uint16_t nextfn);
 void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t ser_num);
 void pcie_ats_init(PCIDevice *dev, uint16_t offset);
 
-extern const VMStateDescription vmstate_pcie_device;
-
-#define VMSTATE_PCIE_DEVICE(_field, _state) {                        \
-    .name       = (stringify(_field)),                               \
-    .size       = sizeof(PCIDevice),                                 \
-    .vmsd       = &vmstate_pcie_device,                              \
-    .flags      = VMS_STRUCT,                                        \
-    .offset     = vmstate_offset_value(_state, _field, PCIDevice),   \
-}
-
 void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
                               Error **errp);
 void pcie_cap_slot_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 14/15] migration: transform remaining DPRINTF into trace_
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (12 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 13/15] PCI/migration merge vmstate_pci_device and vmstate_pcie_device Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 15/15] migration/tracing: Add tracing on save Dr. David Alan Gilbert (git)
  2017-01-25 10:41 ` [Qemu-devel] [PULL 00/15] migration queue Peter Maydell
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: Juan Quintela <quintela@redhat.com>

So we can remove DPRINTF() macro

Signed-off-by: Juan Quintela <quintela@redhat.com>
Message-Id: <1485207141-1941-2-git-send-email-quintela@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
  Fixed up 'remained/remaining' as requested by Eric
---
 migration/ram.c        | 18 ++++--------------
 migration/trace-events |  4 ++++
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index a1c8089..ef8fadf 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -45,14 +45,6 @@
 #include "qemu/rcu_queue.h"
 #include "migration/colo.h"
 
-#ifdef DEBUG_MIGRATION_RAM
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stdout, "migration_ram: " fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
-
 static int dirty_rate_high_cnt;
 
 static uint64_t bitmap_sync_count;
@@ -507,10 +499,10 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t **current_data,
                                        TARGET_PAGE_SIZE, XBZRLE.encoded_buf,
                                        TARGET_PAGE_SIZE);
     if (encoded_len == 0) {
-        DPRINTF("Skipping unmodified page\n");
+        trace_save_xbzrle_page_skipping();
         return 0;
     } else if (encoded_len == -1) {
-        DPRINTF("Overflow\n");
+        trace_save_xbzrle_page_overflow();
         acct_info.xbzrle_overflows++;
         /* update data in the cache */
         if (!last_stage) {
@@ -2020,8 +2012,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
         if ((i & 63) == 0) {
             uint64_t t1 = (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - t0) / 1000000;
             if (t1 > MAX_WAIT) {
-                DPRINTF("big wait: %" PRIu64 " milliseconds, %d iterations\n",
-                        t1, i);
+                trace_ram_save_iterate_big_wait(t1, i);
                 break;
             }
         }
@@ -2594,8 +2585,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
 
     wait_for_decompress_done();
     rcu_read_unlock();
-    DPRINTF("Completed load of VM with exit code %d seq iteration "
-            "%" PRIu64 "\n", ret, seq_iter);
+    trace_ram_load_complete(ret, seq_iter);
     return ret;
 }
 
diff --git a/migration/trace-events b/migration/trace-events
index c46f9e9..ba6bee4 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -190,6 +190,10 @@ postcopy_ram_incoming_cleanup_closeuf(void) ""
 postcopy_ram_incoming_cleanup_entry(void) ""
 postcopy_ram_incoming_cleanup_exit(void) ""
 postcopy_ram_incoming_cleanup_join(void) ""
+save_xbzrle_page_skipping(void) ""
+save_xbzrle_page_overflow(void) ""
+ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
+ram_load_complete(int ret, uint64_t seq_iter) "exit_code %d seq iteration %" PRIu64
 
 # migration/exec.c
 migration_exec_outgoing(const char *cmd) "cmd=%s"
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [Qemu-devel] [PULL 15/15] migration/tracing: Add tracing on save
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (13 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 14/15] migration: transform remaining DPRINTF into trace_ Dr. David Alan Gilbert (git)
@ 2017-01-24 18:47 ` Dr. David Alan Gilbert (git)
  2017-01-25 10:41 ` [Qemu-devel] [PULL 00/15] migration queue Peter Maydell
  15 siblings, 0 replies; 28+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2017-01-24 18:47 UTC (permalink / raw)
  To: qemu-devel

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Add some tracing to vmstate_subsection_save and vmstate_save_state
to help in debugging when you're not sure if a conditional piece
of data is being saved.

In vmstate_subsection_save I renamed the inner vmsd to avoid the aliasing
and be able to print both names.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20161212125838.14425-1-dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/trace-events |  4 ++++
 migration/vmstate.c    | 15 ++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/migration/trace-events b/migration/trace-events
index ba6bee4..48e531d 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -40,6 +40,10 @@ 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_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"
+vmstate_subsection_save_top(const char *idstr) "%s"
 vmstate_load(const char *idstr, const char *vmsd_name) "%s, %s"
 qemu_announce_self_iter(const char *mac) "%s"
 
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 8ddd230..2b2b3a5 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -306,6 +306,8 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
 {
     VMStateField *field = vmsd->fields;
 
+    trace_vmstate_save_state_top(vmsd->name);
+
     if (vmsd->pre_save) {
         vmsd->pre_save(opaque);
     }
@@ -325,6 +327,7 @@ void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
             int64_t old_offset, written_bytes;
             QJSON *vmdesc_loop = vmdesc;
 
+            trace_vmstate_save_state_loop(vmsd->name, field->name, n_elems);
             for (i = 0; i < n_elems; i++) {
                 void *addr = base_addr + size * i;
 
@@ -434,11 +437,13 @@ static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
     const VMStateDescription **sub = vmsd->subsections;
     bool subsection_found = false;
 
+    trace_vmstate_subsection_save_top(vmsd->name);
     while (sub && *sub && (*sub)->needed) {
         if ((*sub)->needed(opaque)) {
-            const VMStateDescription *vmsd = *sub;
+            const VMStateDescription *vmsdsub = *sub;
             uint8_t len;
 
+            trace_vmstate_subsection_save_loop(vmsd->name, vmsdsub->name);
             if (vmdesc) {
                 /* Only create subsection array when we have any */
                 if (!subsection_found) {
@@ -450,11 +455,11 @@ static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
             }
 
             qemu_put_byte(f, QEMU_VM_SUBSECTION);
-            len = strlen(vmsd->name);
+            len = strlen(vmsdsub->name);
             qemu_put_byte(f, len);
-            qemu_put_buffer(f, (uint8_t *)vmsd->name, len);
-            qemu_put_be32(f, vmsd->version_id);
-            vmstate_save_state(f, vmsd, opaque, vmdesc);
+            qemu_put_buffer(f, (uint8_t *)vmsdsub->name, len);
+            qemu_put_be32(f, vmsdsub->version_id);
+            vmstate_save_state(f, vmsdsub, opaque, vmdesc);
 
             if (vmdesc) {
                 json_end_object(vmdesc);
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 00/15] migration queue
  2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
                   ` (14 preceding siblings ...)
  2017-01-24 18:47 ` [Qemu-devel] [PULL 15/15] migration/tracing: Add tracing on save Dr. David Alan Gilbert (git)
@ 2017-01-25 10:41 ` Peter Maydell
  15 siblings, 0 replies; 28+ messages in thread
From: Peter Maydell @ 2017-01-25 10:41 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: QEMU Developers

On 24 January 2017 at 18:47, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> The following changes since commit a678502e4f7580a6f143f680404aaee57ac3f4b5:
>
>   Merge remote-tracking branch 'remotes/ehabkost/tags/x86-and-machine-pull-request' into staging (2017-01-24 15:39:09 +0000)
>
> are available in the git repository at:
>
>   git://github.com/dagrh/qemu.git tags/pull-migration-20170124b
>
> for you to fetch changes up to 46a02a745176e1ef30de41706b5da4f1bc7fc495:
>
>   migration/tracing: Add tracing on save (2017-01-24 18:00:32 +0000)
>
> ----------------------------------------------------------------
> Migration
>
> 1 My maintainer change
> 2 Jianjun's qtailq
> 3 Ashijeet's only-migratable
> 4 Zhanghailiang's re-active images
> 5 Pankaj's change name of migration thread
> 6 My PCI migration merge
> 7 Juan's debug to tracing
> 8 My tracing on save
>

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-24 18:47 ` [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo Dr. David Alan Gilbert (git)
@ 2017-01-25 11:46   ` Fam Zheng
  2017-01-25 12:00     ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 28+ messages in thread
From: Fam Zheng @ 2017-01-25 11:46 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git); +Cc: qemu-devel, Jianjun Duan

On Tue, 01/24 18:47, Dr. David Alan Gilbert (git) wrote:
> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> index c313166..da8e4df 100644
> --- a/hw/intc/s390_flic_kvm.c
> +++ b/hw/intc/s390_flic_kvm.c
> @@ -286,7 +286,8 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
>   * increase until buffer is sufficient or maxium size is
>   * reached
>   */
> -static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> +static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> +                         VMStateField *field, QJSON *vmdesc)
>  {
>      KVMS390FLICState *flic = opaque;
>      int len = FLIC_SAVE_INITIAL_SIZE;
> @@ -319,6 +320,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
>                          count * sizeof(struct kvm_s390_irq));
>      }
>      g_free(buf);
> +
> +    return 0;
>  }

This hunk left one 'return' behind in the function, which should have been
changed to 'return 0' as well, and now the compiler is not happy:

/var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c: In function ‘kvm_flic_save’:
/var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:306:9: error: ‘return’ with no value, in function returning non-void [-Werror]
         return;
         ^~~~~~
/var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:289:12: note: declared here
 static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
            ^~~~~~~~~~~~~
cc1: all warnings being treated as errors

Fam

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-25 11:46   ` Fam Zheng
@ 2017-01-25 12:00     ` Dr. David Alan Gilbert
  2017-01-25 12:07       ` Cornelia Huck
                         ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Dr. David Alan Gilbert @ 2017-01-25 12:00 UTC (permalink / raw)
  To: Fam Zheng, cornelia.huck; +Cc: qemu-devel, Jianjun Duan

* Fam Zheng (famz@redhat.com) wrote:
> On Tue, 01/24 18:47, Dr. David Alan Gilbert (git) wrote:
> > diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> > index c313166..da8e4df 100644
> > --- a/hw/intc/s390_flic_kvm.c
> > +++ b/hw/intc/s390_flic_kvm.c
> > @@ -286,7 +286,8 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
> >   * increase until buffer is sufficient or maxium size is
> >   * reached
> >   */
> > -static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > +static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > +                         VMStateField *field, QJSON *vmdesc)
> >  {
> >      KVMS390FLICState *flic = opaque;
> >      int len = FLIC_SAVE_INITIAL_SIZE;
> > @@ -319,6 +320,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> >                          count * sizeof(struct kvm_s390_irq));
> >      }
> >      g_free(buf);
> > +
> > +    return 0;
> >  }
> 
> This hunk left one 'return' behind in the function, which should have been
> changed to 'return 0' as well, and now the compiler is not happy:
> 
> /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c: In function ‘kvm_flic_save’:
> /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:306:9: error: ‘return’ with no value, in function returning non-void [-Werror]
>          return;
>          ^~~~~~
> /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:289:12: note: declared here
>  static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
>             ^~~~~~~~~~~~~
> cc1: all warnings being treated as errors

OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
sense there.

Do you have a way of build testing that on x86, or can it only be build
tested on s390?
(My build test included an s390x-softmmu build on x86-64).

Dave
> Fam
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-25 12:00     ` Dr. David Alan Gilbert
@ 2017-01-25 12:07       ` Cornelia Huck
  2017-01-25 12:12       ` Fam Zheng
  2017-01-25 12:19       ` Cornelia Huck
  2 siblings, 0 replies; 28+ messages in thread
From: Cornelia Huck @ 2017-01-25 12:07 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: Fam Zheng, qemu-devel, Jianjun Duan

On Wed, 25 Jan 2017 12:00:53 +0000
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Fam Zheng (famz@redhat.com) wrote:
> > On Tue, 01/24 18:47, Dr. David Alan Gilbert (git) wrote:
> > > diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> > > index c313166..da8e4df 100644
> > > --- a/hw/intc/s390_flic_kvm.c
> > > +++ b/hw/intc/s390_flic_kvm.c
> > > @@ -286,7 +286,8 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
> > >   * increase until buffer is sufficient or maxium size is
> > >   * reached
> > >   */
> > > -static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > > +static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > > +                         VMStateField *field, QJSON *vmdesc)
> > >  {
> > >      KVMS390FLICState *flic = opaque;
> > >      int len = FLIC_SAVE_INITIAL_SIZE;
> > > @@ -319,6 +320,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > >                          count * sizeof(struct kvm_s390_irq));
> > >      }
> > >      g_free(buf);
> > > +
> > > +    return 0;
> > >  }
> > 
> > This hunk left one 'return' behind in the function, which should have been
> > changed to 'return 0' as well, and now the compiler is not happy:
> > 
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c: In function ‘kvm_flic_save’:
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:306:9: error: ‘return’ with no value, in function returning non-void [-Werror]
> >          return;
> >          ^~~~~~
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:289:12: note: declared here
> >  static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> >             ^~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> 
> OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
> sense there.
> 
> Do you have a way of build testing that on x86, or can it only be build
> tested on s390?
> (My build test included an s390x-softmmu build on x86-64).

That's a kvm file, and building on non-s390 unfortunately will not
catch these.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-25 12:00     ` Dr. David Alan Gilbert
  2017-01-25 12:07       ` Cornelia Huck
@ 2017-01-25 12:12       ` Fam Zheng
  2017-01-25 12:19       ` Cornelia Huck
  2 siblings, 0 replies; 28+ messages in thread
From: Fam Zheng @ 2017-01-25 12:12 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: cornelia.huck, qemu-devel, Jianjun Duan

On Wed, 01/25 12:00, Dr. David Alan Gilbert wrote:
> * Fam Zheng (famz@redhat.com) wrote:
> > On Tue, 01/24 18:47, Dr. David Alan Gilbert (git) wrote:
> > > diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> > > index c313166..da8e4df 100644
> > > --- a/hw/intc/s390_flic_kvm.c
> > > +++ b/hw/intc/s390_flic_kvm.c
> > > @@ -286,7 +286,8 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
> > >   * increase until buffer is sufficient or maxium size is
> > >   * reached
> > >   */
> > > -static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > > +static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > > +                         VMStateField *field, QJSON *vmdesc)
> > >  {
> > >      KVMS390FLICState *flic = opaque;
> > >      int len = FLIC_SAVE_INITIAL_SIZE;
> > > @@ -319,6 +320,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > >                          count * sizeof(struct kvm_s390_irq));
> > >      }
> > >      g_free(buf);
> > > +
> > > +    return 0;
> > >  }
> > 
> > This hunk left one 'return' behind in the function, which should have been
> > changed to 'return 0' as well, and now the compiler is not happy:
> > 
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c: In function ‘kvm_flic_save’:
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:306:9: error: ‘return’ with no value, in function returning non-void [-Werror]
> >          return;
> >          ^~~~~~
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:289:12: note: declared here
> >  static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> >             ^~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> 
> OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
> sense there.

OK! I was wrong.

> 
> Do you have a way of build testing that on x86, or can it only be build
> tested on s390?
> (My build test included an s390x-softmmu build on x86-64).

Unfortunately I don't know. This is reported by the newly connected s390x
patchew tester, which is in staging so not replying publicly yet. Hopefully we
can get s390x native build covered in the future.

Fam

> 
> Dave
> > Fam
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-25 12:00     ` Dr. David Alan Gilbert
  2017-01-25 12:07       ` Cornelia Huck
  2017-01-25 12:12       ` Fam Zheng
@ 2017-01-25 12:19       ` Cornelia Huck
  2017-01-25 13:22         ` Dr. David Alan Gilbert
  2 siblings, 1 reply; 28+ messages in thread
From: Cornelia Huck @ 2017-01-25 12:19 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: Fam Zheng, qemu-devel, Jianjun Duan

On Wed, 25 Jan 2017 12:00:53 +0000
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Fam Zheng (famz@redhat.com) wrote:
> > On Tue, 01/24 18:47, Dr. David Alan Gilbert (git) wrote:
> > > diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> > > index c313166..da8e4df 100644
> > > --- a/hw/intc/s390_flic_kvm.c
> > > +++ b/hw/intc/s390_flic_kvm.c
> > > @@ -286,7 +286,8 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
> > >   * increase until buffer is sufficient or maxium size is
> > >   * reached
> > >   */
> > > -static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > > +static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > > +                         VMStateField *field, QJSON *vmdesc)
> > >  {
> > >      KVMS390FLICState *flic = opaque;
> > >      int len = FLIC_SAVE_INITIAL_SIZE;
> > > @@ -319,6 +320,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > >                          count * sizeof(struct kvm_s390_irq));
> > >      }
> > >      g_free(buf);
> > > +
> > > +    return 0;
> > >  }
> > 
> > This hunk left one 'return' behind in the function, which should have been
> > changed to 'return 0' as well, and now the compiler is not happy:
> > 
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c: In function ‘kvm_flic_save’:
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:306:9: error: ‘return’ with no value, in function returning non-void [-Werror]
> >          return;
> >          ^~~~~~
> > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:289:12: note: declared here
> >  static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> >             ^~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> 
> OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
> sense there.

Just saw this. I don't think we want -ENOMEM, as that would change the
actual state being saved, no?

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-25 12:19       ` Cornelia Huck
@ 2017-01-25 13:22         ` Dr. David Alan Gilbert
  2017-01-25 14:20           ` Cornelia Huck
  0 siblings, 1 reply; 28+ messages in thread
From: Dr. David Alan Gilbert @ 2017-01-25 13:22 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: Fam Zheng, qemu-devel, Jianjun Duan

* Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> On Wed, 25 Jan 2017 12:00:53 +0000
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> 
> > * Fam Zheng (famz@redhat.com) wrote:
> > > On Tue, 01/24 18:47, Dr. David Alan Gilbert (git) wrote:
> > > > diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> > > > index c313166..da8e4df 100644
> > > > --- a/hw/intc/s390_flic_kvm.c
> > > > +++ b/hw/intc/s390_flic_kvm.c
> > > > @@ -286,7 +286,8 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
> > > >   * increase until buffer is sufficient or maxium size is
> > > >   * reached
> > > >   */
> > > > -static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > > > +static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > > > +                         VMStateField *field, QJSON *vmdesc)
> > > >  {
> > > >      KVMS390FLICState *flic = opaque;
> > > >      int len = FLIC_SAVE_INITIAL_SIZE;
> > > > @@ -319,6 +320,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > > >                          count * sizeof(struct kvm_s390_irq));
> > > >      }
> > > >      g_free(buf);
> > > > +
> > > > +    return 0;
> > > >  }
> > > 
> > > This hunk left one 'return' behind in the function, which should have been
> > > changed to 'return 0' as well, and now the compiler is not happy:
> > > 
> > > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c: In function ‘kvm_flic_save’:
> > > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:306:9: error: ‘return’ with no value, in function returning non-void [-Werror]
> > >          return;
> > >          ^~~~~~
> > > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:289:12: note: declared here
> > >  static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > >             ^~~~~~~~~~~~~
> > > cc1: all warnings being treated as errors
> > 
> > OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
> > sense there.
> 
> Just saw this. I don't think we want -ENOMEM, as that would change the
> actual state being saved, no?

But isn't that the intention of this function?

    buf = g_try_malloc0(len);
    if (!buf) {
        /* Storing FLIC_FAILED into the count field here will cause the
         * target system to fail when attempting to load irqs from the
         * migration state */
        error_report("flic: couldn't allocate memory");
        qemu_put_be64(f, FLIC_FAILED);
        return;
    }

What should happen on the destination - should the migration fail?
If we want the migration to fail then we should now return an error
status rather than 0, and then we see a failed migration on the source
as well.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-25 13:22         ` Dr. David Alan Gilbert
@ 2017-01-25 14:20           ` Cornelia Huck
  2017-01-25 14:44             ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 28+ messages in thread
From: Cornelia Huck @ 2017-01-25 14:20 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: Fam Zheng, qemu-devel, Jianjun Duan

On Wed, 25 Jan 2017 13:22:55 +0000
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> > On Wed, 25 Jan 2017 12:00:53 +0000
> > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > 
> > > * Fam Zheng (famz@redhat.com) wrote:
> > > > On Tue, 01/24 18:47, Dr. David Alan Gilbert (git) wrote:
> > > > > diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> > > > > index c313166..da8e4df 100644
> > > > > --- a/hw/intc/s390_flic_kvm.c
> > > > > +++ b/hw/intc/s390_flic_kvm.c
> > > > > @@ -286,7 +286,8 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
> > > > >   * increase until buffer is sufficient or maxium size is
> > > > >   * reached
> > > > >   */
> > > > > -static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > > > > +static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > > > > +                         VMStateField *field, QJSON *vmdesc)
> > > > >  {
> > > > >      KVMS390FLICState *flic = opaque;
> > > > >      int len = FLIC_SAVE_INITIAL_SIZE;
> > > > > @@ -319,6 +320,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > > > >                          count * sizeof(struct kvm_s390_irq));
> > > > >      }
> > > > >      g_free(buf);
> > > > > +
> > > > > +    return 0;
> > > > >  }
> > > > 
> > > > This hunk left one 'return' behind in the function, which should have been
> > > > changed to 'return 0' as well, and now the compiler is not happy:
> > > > 
> > > > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c: In function ‘kvm_flic_save’:
> > > > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:306:9: error: ‘return’ with no value, in function returning non-void [-Werror]
> > > >          return;
> > > >          ^~~~~~
> > > > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:289:12: note: declared here
> > > >  static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > > >             ^~~~~~~~~~~~~
> > > > cc1: all warnings being treated as errors
> > > 
> > > OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
> > > sense there.
> > 
> > Just saw this. I don't think we want -ENOMEM, as that would change the
> > actual state being saved, no?
> 
> But isn't that the intention of this function?
> 
>     buf = g_try_malloc0(len);
>     if (!buf) {
>         /* Storing FLIC_FAILED into the count field here will cause the
>          * target system to fail when attempting to load irqs from the
>          * migration state */
>         error_report("flic: couldn't allocate memory");
>         qemu_put_be64(f, FLIC_FAILED);
>         return;
>     }
> 
> What should happen on the destination - should the migration fail?
> If we want the migration to fail then we should now return an error
> status rather than 0, and then we see a failed migration on the source
> as well.

Yes. There's also another error case below where we should return an
error instead of putting FLIC_FAILED, then.

The problem is that this is rather hard to test: So I'd prefer to fix
the compile for now and introduce error return codes in a separate
patch.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-25 14:20           ` Cornelia Huck
@ 2017-01-25 14:44             ` Dr. David Alan Gilbert
  2017-01-26 12:14               ` Cornelia Huck
  0 siblings, 1 reply; 28+ messages in thread
From: Dr. David Alan Gilbert @ 2017-01-25 14:44 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: Fam Zheng, qemu-devel, Jianjun Duan

* Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> On Wed, 25 Jan 2017 13:22:55 +0000
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> 
> > * Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> > > On Wed, 25 Jan 2017 12:00:53 +0000
> > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > > 
> > > > * Fam Zheng (famz@redhat.com) wrote:
> > > > > On Tue, 01/24 18:47, Dr. David Alan Gilbert (git) wrote:
> > > > > > diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> > > > > > index c313166..da8e4df 100644
> > > > > > --- a/hw/intc/s390_flic_kvm.c
> > > > > > +++ b/hw/intc/s390_flic_kvm.c
> > > > > > @@ -286,7 +286,8 @@ static void kvm_s390_release_adapter_routes(S390FLICState *fs,
> > > > > >   * increase until buffer is sufficient or maxium size is
> > > > > >   * reached
> > > > > >   */
> > > > > > -static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > > > > > +static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > > > > > +                         VMStateField *field, QJSON *vmdesc)
> > > > > >  {
> > > > > >      KVMS390FLICState *flic = opaque;
> > > > > >      int len = FLIC_SAVE_INITIAL_SIZE;
> > > > > > @@ -319,6 +320,8 @@ static void kvm_flic_save(QEMUFile *f, void *opaque, size_t size)
> > > > > >                          count * sizeof(struct kvm_s390_irq));
> > > > > >      }
> > > > > >      g_free(buf);
> > > > > > +
> > > > > > +    return 0;
> > > > > >  }
> > > > > 
> > > > > This hunk left one 'return' behind in the function, which should have been
> > > > > changed to 'return 0' as well, and now the compiler is not happy:
> > > > > 
> > > > > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c: In function ‘kvm_flic_save’:
> > > > > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:306:9: error: ‘return’ with no value, in function returning non-void [-Werror]
> > > > >          return;
> > > > >          ^~~~~~
> > > > > /var/tmp/patchew-tester-tmp-itftfkl9/src/hw/intc/s390_flic_kvm.c:289:12: note: declared here
> > > > >  static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> > > > >             ^~~~~~~~~~~~~
> > > > > cc1: all warnings being treated as errors
> > > > 
> > > > OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
> > > > sense there.
> > > 
> > > Just saw this. I don't think we want -ENOMEM, as that would change the
> > > actual state being saved, no?
> > 
> > But isn't that the intention of this function?
> > 
> >     buf = g_try_malloc0(len);
> >     if (!buf) {
> >         /* Storing FLIC_FAILED into the count field here will cause the
> >          * target system to fail when attempting to load irqs from the
> >          * migration state */
> >         error_report("flic: couldn't allocate memory");
> >         qemu_put_be64(f, FLIC_FAILED);
> >         return;
> >     }
> > 
> > What should happen on the destination - should the migration fail?
> > If we want the migration to fail then we should now return an error
> > status rather than 0, and then we see a failed migration on the source
> > as well.
> 
> Yes. There's also another error case below where we should return an
> error instead of putting FLIC_FAILED, then.
> 
> The problem is that this is rather hard to test: So I'd prefer to fix
> the compile for now and introduce error return codes in a separate
> patch.

OK, that's fair.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-25 14:44             ` Dr. David Alan Gilbert
@ 2017-01-26 12:14               ` Cornelia Huck
  2017-01-27 18:20                 ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 28+ messages in thread
From: Cornelia Huck @ 2017-01-26 12:14 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: Fam Zheng, qemu-devel, Jianjun Duan

On Wed, 25 Jan 2017 14:44:20 +0000
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> > On Wed, 25 Jan 2017 13:22:55 +0000
> > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > 
> > > * Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> > > > On Wed, 25 Jan 2017 12:00:53 +0000
> > > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> > > > > OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
> > > > > sense there.
> > > > 
> > > > Just saw this. I don't think we want -ENOMEM, as that would change the
> > > > actual state being saved, no?
> > > 
> > > But isn't that the intention of this function?
> > > 
> > >     buf = g_try_malloc0(len);
> > >     if (!buf) {
> > >         /* Storing FLIC_FAILED into the count field here will cause the
> > >          * target system to fail when attempting to load irqs from the
> > >          * migration state */
> > >         error_report("flic: couldn't allocate memory");
> > >         qemu_put_be64(f, FLIC_FAILED);
> > >         return;
> > >     }
> > > 
> > > What should happen on the destination - should the migration fail?
> > > If we want the migration to fail then we should now return an error
> > > status rather than 0, and then we see a failed migration on the source
> > > as well.
> > 
> > Yes. There's also another error case below where we should return an
> > error instead of putting FLIC_FAILED, then.
> > 
> > The problem is that this is rather hard to test: So I'd prefer to fix
> > the compile for now and introduce error return codes in a separate
> > patch.
> 
> OK, that's fair.

I've coded something up and tried to test it with error injection to
trigger the failed case, but I can't really see an improvement :(

Before: source logs error, target fails to load the flic with 'invalid
argument'

After: source logs error, target fails to load the flic with 'could not
allocate memory'

The migration code does not seem to do anything with the return code of
put methods for now, so that's not too surprising. Is anything in the
works?

For now, I'd prefer to keep the old behaviour as 'invalid argument'
seems like a more obvious error on the target.

diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index e86a84e49a..3c62ef8258 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -293,27 +293,21 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
     int len = FLIC_SAVE_INITIAL_SIZE;
     void *buf;
     int count;
+    int r = 0;
 
     flic_disable_wait_pfault((struct KVMS390FLICState *) opaque);
 
     buf = g_try_malloc0(len);
     if (!buf) {
-        /* Storing FLIC_FAILED into the count field here will cause the
-         * target system to fail when attempting to load irqs from the
-         * migration state */
         error_report("flic: couldn't allocate memory");
-        qemu_put_be64(f, FLIC_FAILED);
-        return 0;
+        return -ENOMEM;
     }
 
     count = __get_all_irqs(flic, &buf, len);
     if (count < 0) {
         error_report("flic: couldn't retrieve irqs from kernel, rc %d",
                      count);
-        /* Storing FLIC_FAILED into the count field here will cause the
-         * target system to fail when attempting to load irqs from the
-         * migration state */
-        qemu_put_be64(f, FLIC_FAILED);
+        r = count;
     } else {
         qemu_put_be64(f, count);
         qemu_put_buffer(f, (uint8_t *) buf,
@@ -321,7 +315,7 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
     }
     g_free(buf);
 
-    return 0;
+    return r;
 }
 
 /**

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-26 12:14               ` Cornelia Huck
@ 2017-01-27 18:20                 ` Dr. David Alan Gilbert
  2017-02-01 10:18                   ` Cornelia Huck
  0 siblings, 1 reply; 28+ messages in thread
From: Dr. David Alan Gilbert @ 2017-01-27 18:20 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: Fam Zheng, qemu-devel, Jianjun Duan

* Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> On Wed, 25 Jan 2017 14:44:20 +0000
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> 
> > * Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> > > On Wed, 25 Jan 2017 13:22:55 +0000
> > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > > 
> > > > * Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> > > > > On Wed, 25 Jan 2017 12:00:53 +0000
> > > > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> 
> > > > > > OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
> > > > > > sense there.
> > > > > 
> > > > > Just saw this. I don't think we want -ENOMEM, as that would change the
> > > > > actual state being saved, no?
> > > > 
> > > > But isn't that the intention of this function?
> > > > 
> > > >     buf = g_try_malloc0(len);
> > > >     if (!buf) {
> > > >         /* Storing FLIC_FAILED into the count field here will cause the
> > > >          * target system to fail when attempting to load irqs from the
> > > >          * migration state */
> > > >         error_report("flic: couldn't allocate memory");
> > > >         qemu_put_be64(f, FLIC_FAILED);
> > > >         return;
> > > >     }
> > > > 
> > > > What should happen on the destination - should the migration fail?
> > > > If we want the migration to fail then we should now return an error
> > > > status rather than 0, and then we see a failed migration on the source
> > > > as well.
> > > 
> > > Yes. There's also another error case below where we should return an
> > > error instead of putting FLIC_FAILED, then.
> > > 
> > > The problem is that this is rather hard to test: So I'd prefer to fix
> > > the compile for now and introduce error return codes in a separate
> > > patch.
> > 
> > OK, that's fair.
> 
> I've coded something up and tried to test it with error injection to
> trigger the failed case, but I can't really see an improvement :(
> 
> Before: source logs error, target fails to load the flic with 'invalid
> argument'
> 
> After: source logs error, target fails to load the flic with 'could not
> allocate memory'
> 
> The migration code does not seem to do anything with the return code of
> put methods for now, so that's not too surprising. Is anything in the
> works?

OK, I hadn't remembered that wasn't wired up.

> For now, I'd prefer to keep the old behaviour as 'invalid argument'
> seems like a more obvious error on the target.

Yes, agreed.

If you feel like, then I'd just change the return -ENOMEM but still
send the FLIC_FAILED.  That way the destination still fails
as before, and the destination will fail nicely when we wire up the
error checks.

Dave

> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
> index e86a84e49a..3c62ef8258 100644
> --- a/hw/intc/s390_flic_kvm.c
> +++ b/hw/intc/s390_flic_kvm.c
> @@ -293,27 +293,21 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
>      int len = FLIC_SAVE_INITIAL_SIZE;
>      void *buf;
>      int count;
> +    int r = 0;
>  
>      flic_disable_wait_pfault((struct KVMS390FLICState *) opaque);
>  
>      buf = g_try_malloc0(len);
>      if (!buf) {
> -        /* Storing FLIC_FAILED into the count field here will cause the
> -         * target system to fail when attempting to load irqs from the
> -         * migration state */
>          error_report("flic: couldn't allocate memory");
> -        qemu_put_be64(f, FLIC_FAILED);
> -        return 0;
> +        return -ENOMEM;
>      }
>  
>      count = __get_all_irqs(flic, &buf, len);
>      if (count < 0) {
>          error_report("flic: couldn't retrieve irqs from kernel, rc %d",
>                       count);
> -        /* Storing FLIC_FAILED into the count field here will cause the
> -         * target system to fail when attempting to load irqs from the
> -         * migration state */
> -        qemu_put_be64(f, FLIC_FAILED);
> +        r = count;
>      } else {
>          qemu_put_be64(f, count);
>          qemu_put_buffer(f, (uint8_t *) buf,
> @@ -321,7 +315,7 @@ static int kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
>      }
>      g_free(buf);
>  
> -    return 0;
> +    return r;
>  }
>  
>  /**
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo
  2017-01-27 18:20                 ` Dr. David Alan Gilbert
@ 2017-02-01 10:18                   ` Cornelia Huck
  0 siblings, 0 replies; 28+ messages in thread
From: Cornelia Huck @ 2017-02-01 10:18 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: Fam Zheng, qemu-devel, Jianjun Duan

On Fri, 27 Jan 2017 18:20:36 +0000
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> > On Wed, 25 Jan 2017 14:44:20 +0000
> > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > 
> > > * Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> > > > On Wed, 25 Jan 2017 13:22:55 +0000
> > > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > > > 
> > > > > * Cornelia Huck (cornelia.huck@de.ibm.com) wrote:
> > > > > > On Wed, 25 Jan 2017 12:00:53 +0000
> > > > > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > 
> > > > > > > OK, so it looks like that's a failure path, adding a return -ENOMEM would seem to make
> > > > > > > sense there.
> > > > > > 
> > > > > > Just saw this. I don't think we want -ENOMEM, as that would change the
> > > > > > actual state being saved, no?
> > > > > 
> > > > > But isn't that the intention of this function?
> > > > > 
> > > > >     buf = g_try_malloc0(len);
> > > > >     if (!buf) {
> > > > >         /* Storing FLIC_FAILED into the count field here will cause the
> > > > >          * target system to fail when attempting to load irqs from the
> > > > >          * migration state */
> > > > >         error_report("flic: couldn't allocate memory");
> > > > >         qemu_put_be64(f, FLIC_FAILED);
> > > > >         return;
> > > > >     }
> > > > > 
> > > > > What should happen on the destination - should the migration fail?
> > > > > If we want the migration to fail then we should now return an error
> > > > > status rather than 0, and then we see a failed migration on the source
> > > > > as well.
> > > > 
> > > > Yes. There's also another error case below where we should return an
> > > > error instead of putting FLIC_FAILED, then.
> > > > 
> > > > The problem is that this is rather hard to test: So I'd prefer to fix
> > > > the compile for now and introduce error return codes in a separate
> > > > patch.
> > > 
> > > OK, that's fair.
> > 
> > I've coded something up and tried to test it with error injection to
> > trigger the failed case, but I can't really see an improvement :(
> > 
> > Before: source logs error, target fails to load the flic with 'invalid
> > argument'
> > 
> > After: source logs error, target fails to load the flic with 'could not
> > allocate memory'
> > 
> > The migration code does not seem to do anything with the return code of
> > put methods for now, so that's not too surprising. Is anything in the
> > works?
> 
> OK, I hadn't remembered that wasn't wired up.
> 
> > For now, I'd prefer to keep the old behaviour as 'invalid argument'
> > seems like a more obvious error on the target.
> 
> Yes, agreed.
> 
> If you feel like, then I'd just change the return -ENOMEM but still
> send the FLIC_FAILED.  That way the destination still fails
> as before, and the destination will fail nicely when we wire up the
> error checks.

Yes, having both FLIC_FAILED and a nonzero return code looks like the
best approach for now.

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2017-02-01 10:18 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-24 18:47 [Qemu-devel] [PULL 00/15] migration queue Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 01/15] MAINTAINERS: Add myself as a migration submaintainer Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 02/15] migration: extend VMStateInfo Dr. David Alan Gilbert (git)
2017-01-25 11:46   ` Fam Zheng
2017-01-25 12:00     ` Dr. David Alan Gilbert
2017-01-25 12:07       ` Cornelia Huck
2017-01-25 12:12       ` Fam Zheng
2017-01-25 12:19       ` Cornelia Huck
2017-01-25 13:22         ` Dr. David Alan Gilbert
2017-01-25 14:20           ` Cornelia Huck
2017-01-25 14:44             ` Dr. David Alan Gilbert
2017-01-26 12:14               ` Cornelia Huck
2017-01-27 18:20                 ` Dr. David Alan Gilbert
2017-02-01 10:18                   ` Cornelia Huck
2017-01-24 18:47 ` [Qemu-devel] [PULL 03/15] migration: migrate QTAILQ Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 04/15] tests/migration: Add test for QTAILQ migration Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 05/15] migration: add error_report Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 06/15] block/vvfat: Remove the undesirable comment Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 07/15] migration: Add a new option to enable only-migratable Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 08/15] migration: Allow "device add" options to only add migratable devices Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 09/15] migration: disallow migrate_add_blocker during migration Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 10/15] migration: Fail migration blocker for --only-migratable Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 11/15] migration: re-active images while migration been canceled after inactive them Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 12/15] migration: Change name of live migration thread Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 13/15] PCI/migration merge vmstate_pci_device and vmstate_pcie_device Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 14/15] migration: transform remaining DPRINTF into trace_ Dr. David Alan Gilbert (git)
2017-01-24 18:47 ` [Qemu-devel] [PULL 15/15] migration/tracing: Add tracing on save Dr. David Alan Gilbert (git)
2017-01-25 10:41 ` [Qemu-devel] [PULL 00/15] migration queue Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.