* [Qemu-devel] [QEMU PATCH v8 0/3] migration: migrate QTAILQ
@ 2016-10-25 20:47 Jianjun Duan
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 1/3] migration: extend VMStateInfo Jianjun Duan
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Jianjun Duan @ 2016-10-25 20:47 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, duanj, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth,
dgilbert
Hi all,
I fixed a style issue. Comments are welcome.
v8: - Fixed a style issue.
Previous versions are:
v7: - Fixed merge errors.
- Simplified macro definitions related to pointer arithmetic based QTAILQ access.
- Added test case for QTAILQ migration in tests/test-vmstate.c.
(link: http://lists.nongnu.org/archive/html/qemu-ppc/2016-10/msg00711.html)
v6: - Split from Power specific patches.
- Dropped VMS_LINKED flag.
- Rebased to master.
- Added comments to clarify about put/get in VMStateInfo.
(link: http://lists.nongnu.org/archive/html/qemu-ppc/2016-10/msg00336.html)
v5: - Rebased to David's ppc-for-2.8.
(link: https://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg00270.html)
v4: - Introduce a way to set customized instance_id in SaveStateEntry. Use it
to set instance_id for DRC using its unique index to address David
Gibson's concern.
- Rename VMS_CSTM to VMS_LINKED based on Paolo Bonzini's suggestions.
- Clean up qjson stuff in put_qtailq.
- Add trace for put_qtailq and get_qtailq based on David Gilbert's
suggestion.
- Based on David's ppc-for-2.7.
(link: https://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg07720.html)
v3: - Simplify overall design followng discussion with Paolo. No longer need
metadata to migrate QTAILQ.
- Extend VMStateInfo instead of adding similar fields to VMStateField.
- Clean up macros in qemu/queue.h.
(link: https://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg05695.html)
v2: - Introduce a general approach to migrate QTAILQ in qemu/queue.h.
- Migrate signalled field in the DRC state.
- Put the newly added migrating fields in subsections so that backward
migration is not broken.
- Set detach_cb field right after migration so that a migrated hot-unplug
event could finish its course.
(link: https://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg04188.html)
v1: - Inital version.
(link: https://lists.nongnu.org/archive/html/qemu-devel/2016-04/msg02601.html)
Jianjun Duan (3):
migration: extend VMStateInfo
migration: migrate QTAILQ
tests/migration: Add test for QTAILQ migration
hw/display/virtio-gpu.c | 6 +-
hw/intc/s390_flic_kvm.c | 6 +-
hw/net/vmxnet3.c | 18 +++--
hw/nvram/eeprom93xx.c | 6 +-
hw/nvram/fw_cfg.c | 6 +-
hw/pci/msix.c | 6 +-
hw/pci/pci.c | 12 ++-
hw/pci/shpc.c | 5 +-
hw/scsi/scsi-bus.c | 6 +-
hw/timer/twl92230.c | 6 +-
hw/usb/redirect.c | 18 +++--
hw/virtio/virtio-pci.c | 6 +-
hw/virtio/virtio.c | 12 ++-
include/migration/vmstate.h | 35 ++++++++-
include/qemu/queue.h | 46 ++++++++++++
migration/savevm.c | 5 +-
migration/trace-events | 4 +
migration/vmstate.c | 173 ++++++++++++++++++++++++++++++++++----------
target-alpha/machine.c | 5 +-
target-arm/machine.c | 12 ++-
target-i386/machine.c | 21 ++++--
target-mips/machine.c | 10 ++-
target-ppc/machine.c | 10 ++-
target-sparc/machine.c | 5 +-
tests/test-vmstate.c | 160 ++++++++++++++++++++++++++++++++++++++++
25 files changed, 495 insertions(+), 104 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [QEMU PATCH v8 1/3] migration: extend VMStateInfo
2016-10-25 20:47 [Qemu-devel] [QEMU PATCH v8 0/3] migration: migrate QTAILQ Jianjun Duan
@ 2016-10-25 20:47 ` Jianjun Duan
2016-10-26 12:14 ` Dr. David Alan Gilbert
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ Jianjun Duan
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 3/3] tests/migration: Add test for QTAILQ migration Jianjun Duan
2 siblings, 1 reply; 14+ messages in thread
From: Jianjun Duan @ 2016-10-25 20:47 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, duanj, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth,
dgilbert
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.
Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
---
hw/display/virtio-gpu.c | 6 ++-
hw/intc/s390_flic_kvm.c | 6 ++-
hw/net/vmxnet3.c | 18 +++++---
hw/nvram/eeprom93xx.c | 6 ++-
hw/nvram/fw_cfg.c | 6 ++-
hw/pci/msix.c | 6 ++-
hw/pci/pci.c | 12 +++--
hw/pci/shpc.c | 5 ++-
hw/scsi/scsi-bus.c | 6 ++-
hw/timer/twl92230.c | 6 ++-
hw/usb/redirect.c | 18 +++++---
hw/virtio/virtio-pci.c | 6 ++-
hw/virtio/virtio.c | 12 +++--
include/migration/vmstate.h | 15 +++++--
migration/savevm.c | 5 ++-
migration/vmstate.c | 106 ++++++++++++++++++++++++++++----------------
target-alpha/machine.c | 5 ++-
target-arm/machine.c | 12 +++--
target-i386/machine.c | 21 ++++++---
target-mips/machine.c | 10 +++--
target-ppc/machine.c | 10 +++--
target-sparc/machine.c | 5 ++-
22 files changed, 198 insertions(+), 104 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index fa6fd0e..2a21150 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -987,7 +987,8 @@ static const VMStateDescription vmstate_virtio_gpu_scanouts = {
},
};
-static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size)
+static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
VirtIOGPU *g = opaque;
struct virtio_gpu_simple_resource *res;
@@ -1014,7 +1015,8 @@ static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size)
vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL);
}
-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 21ac2e2..a80a812 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 void kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
KVMS390FLICState *flic = opaque;
int len = FLIC_SAVE_INITIAL_SIZE;
@@ -331,7 +332,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 90f6943..943a960 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2450,7 +2450,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;
@@ -2464,7 +2465,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 void vmxnet3_put_txq_descr(QEMUFile *f, void *pv, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
Vmxnet3TxqDescr *r = pv;
@@ -2511,7 +2513,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;
@@ -2529,7 +2532,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 void vmxnet3_put_rxq_descr(QEMUFile *f, void *pv, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
Vmxnet3RxqDescr *r = pv;
int i;
@@ -2574,7 +2578,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;
@@ -2585,7 +2590,8 @@ 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 void vmxnet3_put_int_state(QEMUFile *f, void *pv, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
Vmxnet3IntState *r = pv;
diff --git a/hw/nvram/eeprom93xx.c b/hw/nvram/eeprom93xx.c
index 2c16fc2..76d5f41 100644
--- a/hw/nvram/eeprom93xx.c
+++ b/hw/nvram/eeprom93xx.c
@@ -94,14 +94,16 @@ 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 void 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");
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 92aa563..a8a4a7a 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -524,14 +524,16 @@ 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 void 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");
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 0ec1cb1..69e7a50 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -587,12 +587,14 @@ 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 void put_msix_state(QEMUFile *f, void *pv, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
msix_save(pv, f);
}
-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 24fae16..08c4547 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,7 +485,8 @@ 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 void 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)));
@@ -497,7 +499,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 +521,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 void 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);
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 3dcd472..9f82aa6 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -695,13 +695,14 @@ 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 void 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));
}
-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..f40c10b 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 void 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);
@@ -1970,7 +1971,8 @@ static void put_scsi_requests(QEMUFile *f, void *pv, size_t size)
qemu_put_sbyte(f, 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 7ba4e9a..95eb7f3 100644
--- a/hw/timer/twl92230.c
+++ b/hw/timer/twl92230.c
@@ -747,14 +747,16 @@ 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 void put_int32_as_uint16(QEMUFile *f, void *pv, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
int *v = pv;
qemu_put_be16(f, *v);
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index d4ca026..7f28c3e 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -2158,7 +2158,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 void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused,
+ VMStateField *field, QJSON *vmdesc)
{
USBRedirDevice *dev = priv;
uint8_t *data;
@@ -2178,7 +2179,8 @@ static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused)
free(data);
}
-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;
@@ -2221,7 +2223,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 void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused,
+ VMStateField *field, QJSON *vmdesc)
{
struct endp_data *endp = priv;
USBRedirDevice *dev = endp->dev;
@@ -2241,7 +2244,8 @@ static void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused)
assert(i == endp->bufpq_size);
}
-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;
@@ -2344,7 +2348,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 void usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused,
+ VMStateField *field, QJSON *vmdesc)
{
struct PacketIdQueue *q = priv;
USBRedirDevice *dev = q->dev;
@@ -2360,7 +2365,8 @@ static void usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused)
assert(remain == 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 06831de..4bd12f0 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 void put_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
VirtIOPCIProxy *proxy = pv;
int i;
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d48d1a9..c799c5c 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1490,7 +1490,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));
@@ -1503,7 +1504,8 @@ 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 void 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));
@@ -1640,13 +1642,15 @@ 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 void virtio_device_put(QEMUFile *f, void *opaque, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
virtio_save(VIRTIO_DEVICE(opaque), f);
}
/* 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 1638ee5..d0e37b5 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -81,11 +81,18 @@ 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 can be passed
+ * into get and put in terms of field and vmdesc parameters.
+ * 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);
+ void (*put)(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc);
};
enum VMStateFlags {
@@ -186,7 +193,7 @@ enum VMStateFlags {
VMS_MULTIPLY_ELEMENTS = 0x4000,
};
-typedef struct {
+struct VMStateField {
const char *name;
size_t offset;
size_t size;
@@ -199,7 +206,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 33a2911..12b7f8d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -220,14 +220,15 @@ 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 void put_timer(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
QEMUTimer *v = pv;
timer_put(f, v);
diff --git a/migration/vmstate.c b/migration/vmstate.c
index fc29acf..d188afa 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);
@@ -83,6 +84,7 @@ 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 %s", vmsd->name, "too new");
trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL);
return -EINVAL;
}
@@ -93,6 +95,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
trace_vmstate_load_state_end(vmsd->name, "old path", ret);
return ret;
}
+ error_report("%s %s", vmsd->name, "too old");
trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL);
return -EINVAL;
}
@@ -122,8 +125,10 @@ 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);
-
+ /* field is always passed in. But it should be ignored by
+ * get when not needed. It is only needed in cases* of
+ * customized handling, such as migrating QTAILQ. */
+ ret = field->info->get(f, addr, size, field);
}
if (ret >= 0) {
ret = qemu_file_get_error(f);
@@ -328,7 +333,11 @@ 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 and vmdesc_loop are always passed in. But they
+ * should be ignored by put when not needed. They are
+ * only needed in cases f customized handling, such as
+ * migrating QTAILQ. */
+ field->info->put(f, addr, size, field, vmdesc_loop);
}
written_bytes = qemu_ftell_fast(f) - old_offset;
@@ -461,14 +470,15 @@ 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 void put_bool(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
bool *v = pv;
qemu_put_byte(f, *v);
@@ -482,14 +492,15 @@ 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 void put_int8(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
int8_t *v = pv;
qemu_put_s8s(f, v);
@@ -503,14 +514,15 @@ 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 void put_int16(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
int16_t *v = pv;
qemu_put_sbe16s(f, v);
@@ -524,14 +536,15 @@ 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 void put_int32(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
int32_t *v = pv;
qemu_put_sbe32s(f, v);
@@ -546,7 +559,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;
@@ -568,7 +582,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;
@@ -589,14 +603,15 @@ 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 void put_int64(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
int64_t *v = pv;
qemu_put_sbe64s(f, v);
@@ -610,14 +625,15 @@ 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 void put_uint8(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
uint8_t *v = pv;
qemu_put_8s(f, v);
@@ -631,14 +647,15 @@ 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 void put_uint16(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
uint16_t *v = pv;
qemu_put_be16s(f, v);
@@ -652,14 +669,15 @@ 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 void put_uint32(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
uint32_t *v = pv;
qemu_put_be32s(f, v);
@@ -674,7 +692,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;
@@ -694,14 +713,15 @@ 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 void put_uint64(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
uint64_t *v = pv;
qemu_put_be64s(f, v);
@@ -716,7 +736,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;
@@ -737,7 +758,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;
@@ -758,7 +780,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;
@@ -778,7 +801,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;
@@ -786,7 +810,8 @@ 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 void put_float64(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
uint64_t *v = pv;
@@ -801,7 +826,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);
@@ -809,7 +835,8 @@ 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 void put_cpudouble(QEMUFile *f, void *pv, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
CPU_DoubleU *v = pv;
qemu_put_be32s(f, &v->l.upper);
@@ -824,14 +851,16 @@ 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 void put_buffer(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
uint8_t *v = pv;
qemu_put_buffer(f, v, size);
@@ -846,7 +875,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;
@@ -859,7 +889,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 void put_unused_buffer(QEMUFile *f, void *pv, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
static const uint8_t buf[1024];
int block_len;
@@ -884,7 +915,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;
@@ -898,7 +929,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 void put_bitmap(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
unsigned long *bmp = pv;
int i, idx = 0;
diff --git a/target-alpha/machine.c b/target-alpha/machine.c
index 710b783..48e3278 100644
--- a/target-alpha/machine.c
+++ b/target-alpha/machine.c
@@ -5,14 +5,15 @@
#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 void 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));
diff --git a/target-arm/machine.c b/target-arm/machine.c
index d90943b..96ff2da 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,7 +28,8 @@ 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 void put_fpscr(QEMUFile *f, void *opaque, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
ARMCPU *cpu = opaque;
CPUARMState *env = &cpu->env;
@@ -163,7 +165,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 +183,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 void put_cpsr(QEMUFile *f, void *opaque, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
ARMCPU *cpu = opaque;
CPUARMState *env = &cpu->env;
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 71c0e4d..1df19e2 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -139,7 +139,8 @@ 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 void 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);
@@ -167,7 +168,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;
@@ -179,7 +181,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 void put_fpreg(QEMUFile *f, void *opaque, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
FPReg *fp_reg = opaque;
uint64_t mant;
@@ -197,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;
@@ -214,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;
@@ -276,14 +281,16 @@ 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 void put_uint64_as_uint32(QEMUFile *f, void *pv, size_t size,
+ VMStateField *field, QJSON *vmdesc)
{
uint64_t *v = pv;
qemu_put_be32(f, *v);
diff --git a/target-mips/machine.c b/target-mips/machine.c
index a27f2f1..179084c 100644
--- a/target-mips/machine.c
+++ b/target-mips/machine.c
@@ -20,7 +20,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;
@@ -31,7 +31,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 void put_fpr(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
int i;
fpr_t *v = pv;
@@ -125,7 +126,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;
@@ -152,7 +153,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 void put_tlb(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
r4k_tlb_t *v = pv;
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 4820f22..0e1822c 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -106,7 +106,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;
@@ -116,7 +116,8 @@ 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 void put_avr(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
ppc_avr_t *v = pv;
@@ -324,7 +325,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;
@@ -334,7 +335,8 @@ 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 void put_slbe(QEMUFile *f, void *pv, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
ppc_slb_t *v = pv;
diff --git a/target-sparc/machine.c b/target-sparc/machine.c
index 59c92f7..3194e03 100644
--- a/target-sparc/machine.c
+++ b/target-sparc/machine.c
@@ -59,7 +59,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;
@@ -72,7 +72,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 void put_psr(QEMUFile *f, void *opaque, size_t size, VMStateField *field,
+ QJSON *vmdesc)
{
SPARCCPU *cpu = opaque;
CPUSPARCState *env = &cpu->env;
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ
2016-10-25 20:47 [Qemu-devel] [QEMU PATCH v8 0/3] migration: migrate QTAILQ Jianjun Duan
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 1/3] migration: extend VMStateInfo Jianjun Duan
@ 2016-10-25 20:47 ` Jianjun Duan
2016-10-26 16:29 ` Dr. David Alan Gilbert
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 3/3] tests/migration: Add test for QTAILQ migration Jianjun Duan
2 siblings, 1 reply; 14+ messages in thread
From: Jianjun Duan @ 2016-10-25 20:47 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, duanj, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth,
dgilbert
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.
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.
Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
---
include/migration/vmstate.h | 20 ++++++++++++++
include/qemu/queue.h | 46 +++++++++++++++++++++++++++++++
migration/trace-events | 4 +++
migration/vmstate.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 137 insertions(+)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index d0e37b5..318a6f1 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -251,6 +251,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)
@@ -662,6 +663,25 @@ extern const VMStateInfo vmstate_info_bitmap;
.offset = offsetof(_state, _field), \
}
+/* For QTAILQ that need customized handling.
+ * 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..e9378fa 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -438,4 +438,50 @@ struct { \
#define QTAILQ_PREV(elm, headname, field) \
(*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
+#define RAW_FIELD(base, offset) \
+ ((char *) (base) + offset)
+
+/*
+ * Offsets of layout of a tail queue head.
+ */
+#define QTAILQ_FIRST_OFFSET 0
+#define QTAILQ_LAST_OFFSET (sizeof(void *))
+/*
+ * Raw access of elements of a tail queue
+ */
+#define QTAILQ_RAW_FIRST(head) \
+ (*((void **) (RAW_FIELD(head, QTAILQ_FIRST_OFFSET))))
+#define QTAILQ_RAW_LAST(head) \
+ (*((void ***) (RAW_FIELD(head, QTAILQ_LAST_OFFSET))))
+
+/*
+ * Offsets of layout of a tail queue element.
+ */
+#define QTAILQ_NEXT_OFFSET 0
+#define QTAILQ_PREV_OFFSET (sizeof(void *))
+
+/*
+ * Raw access of elements of a tail entry
+ */
+#define QTAILQ_RAW_NEXT(elm, entry) \
+ (*((void **) (RAW_FIELD(elm, entry + QTAILQ_NEXT_OFFSET))))
+#define QTAILQ_RAW_PREV(elm, entry) \
+ (*((void ***) (RAW_FIELD(elm, entry + QTAILQ_PREV_OFFSET))))
+/*
+ * 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_PREV(elm, entry) = QTAILQ_RAW_LAST(head); \
+ *QTAILQ_RAW_LAST(head) = (elm); \
+ QTAILQ_RAW_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 dfee75a..9a6ec59 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 d188afa..fcf808e 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"
@@ -948,3 +949,69 @@ 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 void 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");
+}
+const VMStateInfo vmstate_info_qtailq = {
+ .name = "qtailq",
+ .get = get_qtailq,
+ .put = put_qtailq,
+};
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Qemu-devel] [QEMU PATCH v8 3/3] tests/migration: Add test for QTAILQ migration
2016-10-25 20:47 [Qemu-devel] [QEMU PATCH v8 0/3] migration: migrate QTAILQ Jianjun Duan
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 1/3] migration: extend VMStateInfo Jianjun Duan
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ Jianjun Duan
@ 2016-10-25 20:47 ` Jianjun Duan
2016-10-26 16:48 ` Dr. David Alan Gilbert
2 siblings, 1 reply; 14+ messages in thread
From: Jianjun Duan @ 2016-10-25 20:47 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-ppc, duanj, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth,
dgilbert
Add a test for QTAILQ migration to tests/test-vmstate.c.
Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
---
tests/test-vmstate.c | 160 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 160 insertions(+)
diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index d8da26f..a992408 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -475,6 +475,164 @@ static void test_load_skip(void)
qemu_fclose(loading);
}
+
+/* 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()
+ }
+};
+
+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,
+ };
+
+ 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 */
+ };
+
+ 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,
+ };
+
+ 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,
+ };
+
+ 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));
+ qemu_put_byte(fsave, QEMU_VM_EOF);
+ 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((uint64_t) qele_from, ==, (uint64_t) qlast_from);
+ g_assert_cmpint((uint64_t) qele_to, ==, (uint64_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);
@@ -489,6 +647,8 @@ int main(int argc, char **argv)
g_test_add_func("/vmstate/field_exists/load/skip", test_load_skip);
g_test_add_func("/vmstate/field_exists/save/noskip", test_save_noskip);
g_test_add_func("/vmstate/field_exists/save/skip", test_save_skip);
+ 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);
--
1.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v8 1/3] migration: extend VMStateInfo
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 1/3] migration: extend VMStateInfo Jianjun Duan
@ 2016-10-26 12:14 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2016-10-26 12:14 UTC (permalink / raw)
To: Jianjun Duan
Cc: qemu-devel, qemu-ppc, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth
* Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
> 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.
>
> Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hw/display/virtio-gpu.c | 6 ++-
> hw/intc/s390_flic_kvm.c | 6 ++-
> hw/net/vmxnet3.c | 18 +++++---
> hw/nvram/eeprom93xx.c | 6 ++-
> hw/nvram/fw_cfg.c | 6 ++-
> hw/pci/msix.c | 6 ++-
> hw/pci/pci.c | 12 +++--
> hw/pci/shpc.c | 5 ++-
> hw/scsi/scsi-bus.c | 6 ++-
> hw/timer/twl92230.c | 6 ++-
> hw/usb/redirect.c | 18 +++++---
> hw/virtio/virtio-pci.c | 6 ++-
> hw/virtio/virtio.c | 12 +++--
> include/migration/vmstate.h | 15 +++++--
> migration/savevm.c | 5 ++-
> migration/vmstate.c | 106 ++++++++++++++++++++++++++++----------------
> target-alpha/machine.c | 5 ++-
> target-arm/machine.c | 12 +++--
> target-i386/machine.c | 21 ++++++---
> target-mips/machine.c | 10 +++--
> target-ppc/machine.c | 10 +++--
> target-sparc/machine.c | 5 ++-
> 22 files changed, 198 insertions(+), 104 deletions(-)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index fa6fd0e..2a21150 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -987,7 +987,8 @@ static const VMStateDescription vmstate_virtio_gpu_scanouts = {
> },
> };
>
> -static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size)
> +static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> VirtIOGPU *g = opaque;
> struct virtio_gpu_simple_resource *res;
> @@ -1014,7 +1015,8 @@ static void virtio_gpu_save(QEMUFile *f, void *opaque, size_t size)
> vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL);
> }
>
> -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 21ac2e2..a80a812 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 void kvm_flic_save(QEMUFile *f, void *opaque, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> KVMS390FLICState *flic = opaque;
> int len = FLIC_SAVE_INITIAL_SIZE;
> @@ -331,7 +332,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 90f6943..943a960 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2450,7 +2450,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;
>
> @@ -2464,7 +2465,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 void vmxnet3_put_txq_descr(QEMUFile *f, void *pv, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> Vmxnet3TxqDescr *r = pv;
>
> @@ -2511,7 +2513,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;
> @@ -2529,7 +2532,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 void vmxnet3_put_rxq_descr(QEMUFile *f, void *pv, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> Vmxnet3RxqDescr *r = pv;
> int i;
> @@ -2574,7 +2578,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;
>
> @@ -2585,7 +2590,8 @@ 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 void vmxnet3_put_int_state(QEMUFile *f, void *pv, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> Vmxnet3IntState *r = pv;
>
> diff --git a/hw/nvram/eeprom93xx.c b/hw/nvram/eeprom93xx.c
> index 2c16fc2..76d5f41 100644
> --- a/hw/nvram/eeprom93xx.c
> +++ b/hw/nvram/eeprom93xx.c
> @@ -94,14 +94,16 @@ 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 void 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");
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 92aa563..a8a4a7a 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -524,14 +524,16 @@ 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 void 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");
> diff --git a/hw/pci/msix.c b/hw/pci/msix.c
> index 0ec1cb1..69e7a50 100644
> --- a/hw/pci/msix.c
> +++ b/hw/pci/msix.c
> @@ -587,12 +587,14 @@ 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 void put_msix_state(QEMUFile *f, void *pv, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> msix_save(pv, f);
> }
>
> -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 24fae16..08c4547 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,7 +485,8 @@ 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 void 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)));
> @@ -497,7 +499,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 +521,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 void 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);
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index 3dcd472..9f82aa6 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -695,13 +695,14 @@ 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 void 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));
> }
>
> -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..f40c10b 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 void 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);
> @@ -1970,7 +1971,8 @@ static void put_scsi_requests(QEMUFile *f, void *pv, size_t size)
> qemu_put_sbyte(f, 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 7ba4e9a..95eb7f3 100644
> --- a/hw/timer/twl92230.c
> +++ b/hw/timer/twl92230.c
> @@ -747,14 +747,16 @@ 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 void put_int32_as_uint16(QEMUFile *f, void *pv, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> int *v = pv;
> qemu_put_be16(f, *v);
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index d4ca026..7f28c3e 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -2158,7 +2158,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 void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused,
> + VMStateField *field, QJSON *vmdesc)
> {
> USBRedirDevice *dev = priv;
> uint8_t *data;
> @@ -2178,7 +2179,8 @@ static void usbredir_put_parser(QEMUFile *f, void *priv, size_t unused)
> free(data);
> }
>
> -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;
> @@ -2221,7 +2223,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 void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused,
> + VMStateField *field, QJSON *vmdesc)
> {
> struct endp_data *endp = priv;
> USBRedirDevice *dev = endp->dev;
> @@ -2241,7 +2244,8 @@ static void usbredir_put_bufpq(QEMUFile *f, void *priv, size_t unused)
> assert(i == endp->bufpq_size);
> }
>
> -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;
> @@ -2344,7 +2348,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 void usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused,
> + VMStateField *field, QJSON *vmdesc)
> {
> struct PacketIdQueue *q = priv;
> USBRedirDevice *dev = q->dev;
> @@ -2360,7 +2365,8 @@ static void usbredir_put_packet_id_q(QEMUFile *f, void *priv, size_t unused)
> assert(remain == 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 06831de..4bd12f0 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 void put_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> VirtIOPCIProxy *proxy = pv;
> int i;
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index d48d1a9..c799c5c 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1490,7 +1490,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));
> @@ -1503,7 +1504,8 @@ 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 void 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));
> @@ -1640,13 +1642,15 @@ 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 void virtio_device_put(QEMUFile *f, void *opaque, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> virtio_save(VIRTIO_DEVICE(opaque), f);
> }
>
> /* 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 1638ee5..d0e37b5 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -81,11 +81,18 @@ 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 can be passed
> + * into get and put in terms of field and vmdesc parameters.
> + * 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);
> + void (*put)(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc);
> };
>
> enum VMStateFlags {
> @@ -186,7 +193,7 @@ enum VMStateFlags {
> VMS_MULTIPLY_ELEMENTS = 0x4000,
> };
>
> -typedef struct {
> +struct VMStateField {
> const char *name;
> size_t offset;
> size_t size;
> @@ -199,7 +206,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 33a2911..12b7f8d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -220,14 +220,15 @@ 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 void put_timer(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> QEMUTimer *v = pv;
> timer_put(f, v);
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index fc29acf..d188afa 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);
> @@ -83,6 +84,7 @@ 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 %s", vmsd->name, "too new");
> trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL);
> return -EINVAL;
> }
> @@ -93,6 +95,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
> trace_vmstate_load_state_end(vmsd->name, "old path", ret);
> return ret;
> }
> + error_report("%s %s", vmsd->name, "too old");
> trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL);
> return -EINVAL;
> }
> @@ -122,8 +125,10 @@ 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);
> -
> + /* field is always passed in. But it should be ignored by
> + * get when not needed. It is only needed in cases* of
> + * customized handling, such as migrating QTAILQ. */
> + ret = field->info->get(f, addr, size, field);
> }
> if (ret >= 0) {
> ret = qemu_file_get_error(f);
> @@ -328,7 +333,11 @@ 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 and vmdesc_loop are always passed in. But they
> + * should be ignored by put when not needed. They are
> + * only needed in cases f customized handling, such as
> + * migrating QTAILQ. */
> + field->info->put(f, addr, size, field, vmdesc_loop);
> }
>
> written_bytes = qemu_ftell_fast(f) - old_offset;
> @@ -461,14 +470,15 @@ 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 void put_bool(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> bool *v = pv;
> qemu_put_byte(f, *v);
> @@ -482,14 +492,15 @@ 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 void put_int8(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> int8_t *v = pv;
> qemu_put_s8s(f, v);
> @@ -503,14 +514,15 @@ 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 void put_int16(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> int16_t *v = pv;
> qemu_put_sbe16s(f, v);
> @@ -524,14 +536,15 @@ 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 void put_int32(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> int32_t *v = pv;
> qemu_put_sbe32s(f, v);
> @@ -546,7 +559,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;
> @@ -568,7 +582,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;
> @@ -589,14 +603,15 @@ 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 void put_int64(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> int64_t *v = pv;
> qemu_put_sbe64s(f, v);
> @@ -610,14 +625,15 @@ 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 void put_uint8(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> uint8_t *v = pv;
> qemu_put_8s(f, v);
> @@ -631,14 +647,15 @@ 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 void put_uint16(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> uint16_t *v = pv;
> qemu_put_be16s(f, v);
> @@ -652,14 +669,15 @@ 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 void put_uint32(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> uint32_t *v = pv;
> qemu_put_be32s(f, v);
> @@ -674,7 +692,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;
> @@ -694,14 +713,15 @@ 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 void put_uint64(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> uint64_t *v = pv;
> qemu_put_be64s(f, v);
> @@ -716,7 +736,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;
> @@ -737,7 +758,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;
> @@ -758,7 +780,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;
> @@ -778,7 +801,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;
>
> @@ -786,7 +810,8 @@ 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 void put_float64(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> uint64_t *v = pv;
>
> @@ -801,7 +826,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);
> @@ -809,7 +835,8 @@ 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 void put_cpudouble(QEMUFile *f, void *pv, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> CPU_DoubleU *v = pv;
> qemu_put_be32s(f, &v->l.upper);
> @@ -824,14 +851,16 @@ 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 void put_buffer(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> uint8_t *v = pv;
> qemu_put_buffer(f, v, size);
> @@ -846,7 +875,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;
> @@ -859,7 +889,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 void put_unused_buffer(QEMUFile *f, void *pv, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> static const uint8_t buf[1024];
> int block_len;
> @@ -884,7 +915,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;
> @@ -898,7 +929,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 void put_bitmap(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> unsigned long *bmp = pv;
> int i, idx = 0;
> diff --git a/target-alpha/machine.c b/target-alpha/machine.c
> index 710b783..48e3278 100644
> --- a/target-alpha/machine.c
> +++ b/target-alpha/machine.c
> @@ -5,14 +5,15 @@
> #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 void 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));
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index d90943b..96ff2da 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,7 +28,8 @@ 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 void put_fpscr(QEMUFile *f, void *opaque, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> ARMCPU *cpu = opaque;
> CPUARMState *env = &cpu->env;
> @@ -163,7 +165,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 +183,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 void put_cpsr(QEMUFile *f, void *opaque, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> ARMCPU *cpu = opaque;
> CPUARMState *env = &cpu->env;
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 71c0e4d..1df19e2 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -139,7 +139,8 @@ 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 void 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);
> @@ -167,7 +168,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;
> @@ -179,7 +181,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 void put_fpreg(QEMUFile *f, void *opaque, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> FPReg *fp_reg = opaque;
> uint64_t mant;
> @@ -197,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;
> @@ -214,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;
> @@ -276,14 +281,16 @@ 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 void put_uint64_as_uint32(QEMUFile *f, void *pv, size_t size,
> + VMStateField *field, QJSON *vmdesc)
> {
> uint64_t *v = pv;
> qemu_put_be32(f, *v);
> diff --git a/target-mips/machine.c b/target-mips/machine.c
> index a27f2f1..179084c 100644
> --- a/target-mips/machine.c
> +++ b/target-mips/machine.c
> @@ -20,7 +20,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;
> @@ -31,7 +31,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 void put_fpr(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> int i;
> fpr_t *v = pv;
> @@ -125,7 +126,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;
> @@ -152,7 +153,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 void put_tlb(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> r4k_tlb_t *v = pv;
>
> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> index 4820f22..0e1822c 100644
> --- a/target-ppc/machine.c
> +++ b/target-ppc/machine.c
> @@ -106,7 +106,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;
>
> @@ -116,7 +116,8 @@ 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 void put_avr(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> ppc_avr_t *v = pv;
>
> @@ -324,7 +325,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;
>
> @@ -334,7 +335,8 @@ 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 void put_slbe(QEMUFile *f, void *pv, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> ppc_slb_t *v = pv;
>
> diff --git a/target-sparc/machine.c b/target-sparc/machine.c
> index 59c92f7..3194e03 100644
> --- a/target-sparc/machine.c
> +++ b/target-sparc/machine.c
> @@ -59,7 +59,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;
> @@ -72,7 +72,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 void put_psr(QEMUFile *f, void *opaque, size_t size, VMStateField *field,
> + QJSON *vmdesc)
> {
> SPARCCPU *cpu = opaque;
> CPUSPARCState *env = &cpu->env;
> --
> 1.9.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ Jianjun Duan
@ 2016-10-26 16:29 ` Dr. David Alan Gilbert
2016-10-26 16:50 ` Jianjun Duan
0 siblings, 1 reply; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2016-10-26 16:29 UTC (permalink / raw)
To: Jianjun Duan
Cc: qemu-devel, qemu-ppc, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth
* Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
> 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.
>
> 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.
>
> Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
> ---
> include/migration/vmstate.h | 20 ++++++++++++++
> include/qemu/queue.h | 46 +++++++++++++++++++++++++++++++
> migration/trace-events | 4 +++
> migration/vmstate.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 137 insertions(+)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index d0e37b5..318a6f1 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -251,6 +251,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)
> @@ -662,6 +663,25 @@ extern const VMStateInfo vmstate_info_bitmap;
> .offset = offsetof(_state, _field), \
> }
>
> +/* For QTAILQ that need customized handling.
> + * 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..e9378fa 100644
> --- a/include/qemu/queue.h
> +++ b/include/qemu/queue.h
> @@ -438,4 +438,50 @@ struct { \
> #define QTAILQ_PREV(elm, headname, field) \
> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
>
> +#define RAW_FIELD(base, offset) \
> + ((char *) (base) + offset)
> +
> +/*
> + * Offsets of layout of a tail queue head.
> + */
> +#define QTAILQ_FIRST_OFFSET 0
> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
OK, you might want to add some asserts somewhere in a .c just to catch if
any of these offsets change.
> + * Raw access of elements of a tail queue
> + */
> +#define QTAILQ_RAW_FIRST(head) \
> + (*((void **) (RAW_FIELD(head, QTAILQ_FIRST_OFFSET))))
> +#define QTAILQ_RAW_LAST(head) \
> + (*((void ***) (RAW_FIELD(head, QTAILQ_LAST_OFFSET))))
> +
> +/*
> + * Offsets of layout of a tail queue element.
> + */
> +#define QTAILQ_NEXT_OFFSET 0
> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
> +
> +/*
> + * Raw access of elements of a tail entry
> + */
> +#define QTAILQ_RAW_NEXT(elm, entry) \
> + (*((void **) (RAW_FIELD(elm, entry + QTAILQ_NEXT_OFFSET))))
> +#define QTAILQ_RAW_PREV(elm, entry) \
> + (*((void ***) (RAW_FIELD(elm, entry + QTAILQ_PREV_OFFSET))))
> +/*
> + * 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_PREV(elm, entry) = QTAILQ_RAW_LAST(head); \
> + *QTAILQ_RAW_LAST(head) = (elm); \
> + QTAILQ_RAW_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 dfee75a..9a6ec59 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 d188afa..fcf808e 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"
>
> @@ -948,3 +949,69 @@ 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;
You could just make that a break since you're returning ret
(that's otherwise all 0). Still, not important, but could
be tidied if you need to regenerate the patch.
(and you could remove the 2nd space after elm =).
> + }
> + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry_offset);
> + }
> +
> + trace_get_qtailq_end(vmsd->name, "end", ret);
> + return ret;
> +}
> +
> +/* put for QTAILQ */
> +static void 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");
> +}
> +const VMStateInfo vmstate_info_qtailq = {
> + .name = "qtailq",
> + .get = get_qtailq,
> + .put = put_qtailq,
> +};
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> --
> 1.9.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v8 3/3] tests/migration: Add test for QTAILQ migration
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 3/3] tests/migration: Add test for QTAILQ migration Jianjun Duan
@ 2016-10-26 16:48 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2016-10-26 16:48 UTC (permalink / raw)
To: Jianjun Duan
Cc: qemu-devel, qemu-ppc, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth
* Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
> Add a test for QTAILQ migration to tests/test-vmstate.c.
>
> Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
> ---
> tests/test-vmstate.c | 160 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 160 insertions(+)
>
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index d8da26f..a992408 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -475,6 +475,164 @@ static void test_load_skip(void)
> qemu_fclose(loading);
> }
>
> +
> +/* 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()
> + }
> +};
> +
> +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,
> + };
> +
> + 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 */
> + };
> +
> + 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,
> + };
> +
> + 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,
> + };
OK, there would probably have been a way to avoid duplicating
all that from above.
Probably even have just done it as one test - save, compare the data, load.
> + 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));
> + qemu_put_byte(fsave, QEMU_VM_EOF);
> + 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((uint64_t) qele_from, ==, (uint64_t) qlast_from);
> + g_assert_cmpint((uint64_t) qele_to, ==, (uint64_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);
> @@ -489,6 +647,8 @@ int main(int argc, char **argv)
> g_test_add_func("/vmstate/field_exists/load/skip", test_load_skip);
> g_test_add_func("/vmstate/field_exists/save/noskip", test_save_noskip);
> g_test_add_func("/vmstate/field_exists/save/skip", test_save_skip);
> + 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);
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> --
> 1.9.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ
2016-10-26 16:29 ` Dr. David Alan Gilbert
@ 2016-10-26 16:50 ` Jianjun Duan
2016-10-26 16:53 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 14+ messages in thread
From: Jianjun Duan @ 2016-10-26 16:50 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, qemu-ppc, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth
On 10/26/2016 09:29 AM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>> 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.
>>
>> 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.
>>
>> Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
>> ---
>> include/migration/vmstate.h | 20 ++++++++++++++
>> include/qemu/queue.h | 46 +++++++++++++++++++++++++++++++
>> migration/trace-events | 4 +++
>> migration/vmstate.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 137 insertions(+)
>>
>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> index d0e37b5..318a6f1 100644
>> --- a/include/migration/vmstate.h
>> +++ b/include/migration/vmstate.h
>> @@ -251,6 +251,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)
>> @@ -662,6 +663,25 @@ extern const VMStateInfo vmstate_info_bitmap;
>> .offset = offsetof(_state, _field), \
>> }
>>
>> +/* For QTAILQ that need customized handling.
>> + * 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..e9378fa 100644
>> --- a/include/qemu/queue.h
>> +++ b/include/qemu/queue.h
>> @@ -438,4 +438,50 @@ struct { \
>> #define QTAILQ_PREV(elm, headname, field) \
>> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
>>
>> +#define RAW_FIELD(base, offset) \
>> + ((char *) (base) + offset)
>> +
>> +/*
>> + * Offsets of layout of a tail queue head.
>> + */
>> +#define QTAILQ_FIRST_OFFSET 0
>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>
> OK, you might want to add some asserts somewhere in a .c just to catch if
> any of these offsets change.
>
if the layout of QTAILQ changes, the version id for the vmsd should also
be changed. It will break migration between different versions. However
the operation doesn't depend on these values.
>> + * Raw access of elements of a tail queue
>> + */
>> +#define QTAILQ_RAW_FIRST(head) \
>> + (*((void **) (RAW_FIELD(head, QTAILQ_FIRST_OFFSET))))
>> +#define QTAILQ_RAW_LAST(head) \
>> + (*((void ***) (RAW_FIELD(head, QTAILQ_LAST_OFFSET))))
>> +
>> +/*
>> + * Offsets of layout of a tail queue element.
>> + */
>> +#define QTAILQ_NEXT_OFFSET 0
>> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
>> +
>> +/*
>> + * Raw access of elements of a tail entry
>> + */
>> +#define QTAILQ_RAW_NEXT(elm, entry) \
>> + (*((void **) (RAW_FIELD(elm, entry + QTAILQ_NEXT_OFFSET))))
>> +#define QTAILQ_RAW_PREV(elm, entry) \
>> + (*((void ***) (RAW_FIELD(elm, entry + QTAILQ_PREV_OFFSET))))
>> +/*
>> + * 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_PREV(elm, entry) = QTAILQ_RAW_LAST(head); \
>> + *QTAILQ_RAW_LAST(head) = (elm); \
>> + QTAILQ_RAW_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 dfee75a..9a6ec59 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 d188afa..fcf808e 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"
>>
>> @@ -948,3 +949,69 @@ 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;
>
> You could just make that a break since you're returning ret
> (that's otherwise all 0). Still, not important, but could
> be tidied if you need to regenerate the patch.
> (and you could remove the 2nd space after elm =).
By returning it, the trace file will miss "get_qtailq_end', that
is useful debug information.
You are right about the space.
Thanks,
Jianjun
>
>> + }
>> + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry_offset);
>> + }
>> +
>> + trace_get_qtailq_end(vmsd->name, "end", ret);
>> + return ret;
>> +}
>> +
>> +/* put for QTAILQ */
>> +static void 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");
>> +}
>> +const VMStateInfo vmstate_info_qtailq = {
>> + .name = "qtailq",
>> + .get = get_qtailq,
>> + .put = put_qtailq,
>> +};
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
>> --
>> 1.9.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ
2016-10-26 16:50 ` Jianjun Duan
@ 2016-10-26 16:53 ` Dr. David Alan Gilbert
2016-10-26 17:04 ` Jianjun Duan
0 siblings, 1 reply; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2016-10-26 16:53 UTC (permalink / raw)
To: Jianjun Duan
Cc: qemu-devel, qemu-ppc, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth
* Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>
>
> On 10/26/2016 09:29 AM, Dr. David Alan Gilbert wrote:
> > * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
> >> 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.
> >>
> >> 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.
> >>
> >> Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
> >> ---
> >> include/migration/vmstate.h | 20 ++++++++++++++
> >> include/qemu/queue.h | 46 +++++++++++++++++++++++++++++++
> >> migration/trace-events | 4 +++
> >> migration/vmstate.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 137 insertions(+)
> >>
> >> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> >> index d0e37b5..318a6f1 100644
> >> --- a/include/migration/vmstate.h
> >> +++ b/include/migration/vmstate.h
> >> @@ -251,6 +251,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)
> >> @@ -662,6 +663,25 @@ extern const VMStateInfo vmstate_info_bitmap;
> >> .offset = offsetof(_state, _field), \
> >> }
> >>
> >> +/* For QTAILQ that need customized handling.
> >> + * 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..e9378fa 100644
> >> --- a/include/qemu/queue.h
> >> +++ b/include/qemu/queue.h
> >> @@ -438,4 +438,50 @@ struct { \
> >> #define QTAILQ_PREV(elm, headname, field) \
> >> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
> >>
> >> +#define RAW_FIELD(base, offset) \
> >> + ((char *) (base) + offset)
> >> +
> >> +/*
> >> + * Offsets of layout of a tail queue head.
> >> + */
> >> +#define QTAILQ_FIRST_OFFSET 0
> >> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
> >
> > OK, you might want to add some asserts somewhere in a .c just to catch if
> > any of these offsets change.
> >
> if the layout of QTAILQ changes, the version id for the vmsd should also
> be changed. It will break migration between different versions. However
> the operation doesn't depend on these values.
No, changing layout of QTAILQ doesn't need to change the version id of vmsd;
it's an internal change. But if someone does make the change and forgets
to update your OFFSET macros it'll cause memory corruption.
You could catch that with an assert (possibly build time).
> >> + * Raw access of elements of a tail queue
> >> + */
> >> +#define QTAILQ_RAW_FIRST(head) \
> >> + (*((void **) (RAW_FIELD(head, QTAILQ_FIRST_OFFSET))))
> >> +#define QTAILQ_RAW_LAST(head) \
> >> + (*((void ***) (RAW_FIELD(head, QTAILQ_LAST_OFFSET))))
> >> +
> >> +/*
> >> + * Offsets of layout of a tail queue element.
> >> + */
> >> +#define QTAILQ_NEXT_OFFSET 0
> >> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
> >> +
> >> +/*
> >> + * Raw access of elements of a tail entry
> >> + */
> >> +#define QTAILQ_RAW_NEXT(elm, entry) \
> >> + (*((void **) (RAW_FIELD(elm, entry + QTAILQ_NEXT_OFFSET))))
> >> +#define QTAILQ_RAW_PREV(elm, entry) \
> >> + (*((void ***) (RAW_FIELD(elm, entry + QTAILQ_PREV_OFFSET))))
> >> +/*
> >> + * 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_PREV(elm, entry) = QTAILQ_RAW_LAST(head); \
> >> + *QTAILQ_RAW_LAST(head) = (elm); \
> >> + QTAILQ_RAW_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 dfee75a..9a6ec59 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 d188afa..fcf808e 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"
> >>
> >> @@ -948,3 +949,69 @@ 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;
> >
> > You could just make that a break since you're returning ret
> > (that's otherwise all 0). Still, not important, but could
> > be tidied if you need to regenerate the patch.
> > (and you could remove the 2nd space after elm =).
> By returning it, the trace file will miss "get_qtailq_end', that
> is useful debug information.
> You are right about the space.
But that's what you're doing now, you're missing the trace call
and returning 'ret' that will always be 0 if it gets there.
Dave
>
> Thanks,
> Jianjun
>
> >
> >> + }
> >> + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry_offset);
> >> + }
> >> +
> >> + trace_get_qtailq_end(vmsd->name, "end", ret);
> >> + return ret;
> >> +}
> >> +
> >> +/* put for QTAILQ */
> >> +static void 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");
> >> +}
> >> +const VMStateInfo vmstate_info_qtailq = {
> >> + .name = "qtailq",
> >> + .get = get_qtailq,
> >> + .put = put_qtailq,
> >> +};
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
> >> --
> >> 1.9.1
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ
2016-10-26 16:53 ` Dr. David Alan Gilbert
@ 2016-10-26 17:04 ` Jianjun Duan
2016-10-26 17:09 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 14+ messages in thread
From: Jianjun Duan @ 2016-10-26 17:04 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, qemu-ppc, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth
On 10/26/2016 09:53 AM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>>
>>
>> On 10/26/2016 09:29 AM, Dr. David Alan Gilbert wrote:
>>> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>>>> 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.
>>>>
>>>> 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.
>>>>
>>>> Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
>>>> ---
>>>> include/migration/vmstate.h | 20 ++++++++++++++
>>>> include/qemu/queue.h | 46 +++++++++++++++++++++++++++++++
>>>> migration/trace-events | 4 +++
>>>> migration/vmstate.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
>>>> 4 files changed, 137 insertions(+)
>>>>
>>>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>>>> index d0e37b5..318a6f1 100644
>>>> --- a/include/migration/vmstate.h
>>>> +++ b/include/migration/vmstate.h
>>>> @@ -251,6 +251,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)
>>>> @@ -662,6 +663,25 @@ extern const VMStateInfo vmstate_info_bitmap;
>>>> .offset = offsetof(_state, _field), \
>>>> }
>>>>
>>>> +/* For QTAILQ that need customized handling.
>>>> + * 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..e9378fa 100644
>>>> --- a/include/qemu/queue.h
>>>> +++ b/include/qemu/queue.h
>>>> @@ -438,4 +438,50 @@ struct { \
>>>> #define QTAILQ_PREV(elm, headname, field) \
>>>> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
>>>>
>>>> +#define RAW_FIELD(base, offset) \
>>>> + ((char *) (base) + offset)
>>>> +
>>>> +/*
>>>> + * Offsets of layout of a tail queue head.
>>>> + */
>>>> +#define QTAILQ_FIRST_OFFSET 0
>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>>>
>>> OK, you might want to add some asserts somewhere in a .c just to catch if
>>> any of these offsets change.
>>>
>> if the layout of QTAILQ changes, the version id for the vmsd should also
>> be changed. It will break migration between different versions. However
>> the operation doesn't depend on these values.
>
> No, changing layout of QTAILQ doesn't need to change the version id of vmsd;
> it's an internal change. But if someone does make the change and forgets
> to update your OFFSET macros it'll cause memory corruption.
> You could catch that with an assert (possibly build time).
>
If we use these constant I would agree an assertion is necessary. By
using a macro we leave the door open for change. and if someone changes
the layout, he or she better change the macros and the version id. If an
assertion is added, then that assertion needs to be changed to reflect
the change, then in the unlikely situations we could have several
version of layout/macro/assertions floating around. That is too much. SO
version id is the best guard here.
Thanks,
Jianjun
>>>> + * Raw access of elements of a tail queue
>>>> + */
>>>> +#define QTAILQ_RAW_FIRST(head) \
>>>> + (*((void **) (RAW_FIELD(head, QTAILQ_FIRST_OFFSET))))
>>>> +#define QTAILQ_RAW_LAST(head) \
>>>> + (*((void ***) (RAW_FIELD(head, QTAILQ_LAST_OFFSET))))
>>>> +
>>>> +/*
>>>> + * Offsets of layout of a tail queue element.
>>>> + */
>>>> +#define QTAILQ_NEXT_OFFSET 0
>>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
>>>> +
>>>> +/*
>>>> + * Raw access of elements of a tail entry
>>>> + */
>>>> +#define QTAILQ_RAW_NEXT(elm, entry) \
>>>> + (*((void **) (RAW_FIELD(elm, entry + QTAILQ_NEXT_OFFSET))))
>>>> +#define QTAILQ_RAW_PREV(elm, entry) \
>>>> + (*((void ***) (RAW_FIELD(elm, entry + QTAILQ_PREV_OFFSET))))
>>>> +/*
>>>> + * 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_PREV(elm, entry) = QTAILQ_RAW_LAST(head); \
>>>> + *QTAILQ_RAW_LAST(head) = (elm); \
>>>> + QTAILQ_RAW_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 dfee75a..9a6ec59 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 d188afa..fcf808e 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"
>>>>
>>>> @@ -948,3 +949,69 @@ 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;
>>>
>>> You could just make that a break since you're returning ret
>>> (that's otherwise all 0). Still, not important, but could
>>> be tidied if you need to regenerate the patch.
>>> (and you could remove the 2nd space after elm =).
>
>> By returning it, the trace file will miss "get_qtailq_end', that
>> is useful debug information.
>> You are right about the space.
>
> But that's what you're doing now, you're missing the trace call
> and returning 'ret' that will always be 0 if it gets there.
>
> Dave
>
>>
>> Thanks,
>> Jianjun
>>
>>>
>>>> + }
>>>> + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry_offset);
>>>> + }
>>>> +
>>>> + trace_get_qtailq_end(vmsd->name, "end", ret);
>>>> + return ret;
>>>> +}
>>>> +
>>>> +/* put for QTAILQ */
>>>> +static void 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");
>>>> +}
>>>> +const VMStateInfo vmstate_info_qtailq = {
>>>> + .name = "qtailq",
>>>> + .get = get_qtailq,
>>>> + .put = put_qtailq,
>>>> +};
>>>
>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>
>>>> --
>>>> 1.9.1
>>>>
>>> --
>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ
2016-10-26 17:04 ` Jianjun Duan
@ 2016-10-26 17:09 ` Dr. David Alan Gilbert
2016-10-26 17:33 ` Jianjun Duan
0 siblings, 1 reply; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2016-10-26 17:09 UTC (permalink / raw)
To: Jianjun Duan
Cc: qemu-devel, qemu-ppc, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth
* Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>
>
> On 10/26/2016 09:53 AM, Dr. David Alan Gilbert wrote:
> > * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
> >>
> >>
> >> On 10/26/2016 09:29 AM, Dr. David Alan Gilbert wrote:
> >>> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
> >>>> 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.
> >>>>
> >>>> 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.
> >>>>
> >>>> Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
> >>>> ---
> >>>> include/migration/vmstate.h | 20 ++++++++++++++
> >>>> include/qemu/queue.h | 46 +++++++++++++++++++++++++++++++
> >>>> migration/trace-events | 4 +++
> >>>> migration/vmstate.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
> >>>> 4 files changed, 137 insertions(+)
> >>>>
> >>>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> >>>> index d0e37b5..318a6f1 100644
> >>>> --- a/include/migration/vmstate.h
> >>>> +++ b/include/migration/vmstate.h
> >>>> @@ -251,6 +251,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)
> >>>> @@ -662,6 +663,25 @@ extern const VMStateInfo vmstate_info_bitmap;
> >>>> .offset = offsetof(_state, _field), \
> >>>> }
> >>>>
> >>>> +/* For QTAILQ that need customized handling.
> >>>> + * 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..e9378fa 100644
> >>>> --- a/include/qemu/queue.h
> >>>> +++ b/include/qemu/queue.h
> >>>> @@ -438,4 +438,50 @@ struct { \
> >>>> #define QTAILQ_PREV(elm, headname, field) \
> >>>> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
> >>>>
> >>>> +#define RAW_FIELD(base, offset) \
> >>>> + ((char *) (base) + offset)
> >>>> +
> >>>> +/*
> >>>> + * Offsets of layout of a tail queue head.
> >>>> + */
> >>>> +#define QTAILQ_FIRST_OFFSET 0
> >>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
> >>>
> >>> OK, you might want to add some asserts somewhere in a .c just to catch if
> >>> any of these offsets change.
> >>>
> >> if the layout of QTAILQ changes, the version id for the vmsd should also
> >> be changed. It will break migration between different versions. However
> >> the operation doesn't depend on these values.
> >
> > No, changing layout of QTAILQ doesn't need to change the version id of vmsd;
> > it's an internal change. But if someone does make the change and forgets
> > to update your OFFSET macros it'll cause memory corruption.
> > You could catch that with an assert (possibly build time).
> >
> If we use these constant I would agree an assertion is necessary. By
> using a macro we leave the door open for change. and if someone changes
> the layout, he or she better change the macros and the version id. If an
> assertion is added, then that assertion needs to be changed to reflect
> the change, then in the unlikely situations we could have several
> version of layout/macro/assertions floating around. That is too much. SO
> version id is the best guard here.
Version id's are irrelevant here.
The macros are irrelevant here.
I'm talking about a potential mismatch between the definition of QTAILQ_LAST_OFFSET
and the definition of Q_TAILQ_HEAD.
Dave
>
> Thanks,
> Jianjun
>
> >>>> + * Raw access of elements of a tail queue
> >>>> + */
> >>>> +#define QTAILQ_RAW_FIRST(head) \
> >>>> + (*((void **) (RAW_FIELD(head, QTAILQ_FIRST_OFFSET))))
> >>>> +#define QTAILQ_RAW_LAST(head) \
> >>>> + (*((void ***) (RAW_FIELD(head, QTAILQ_LAST_OFFSET))))
> >>>> +
> >>>> +/*
> >>>> + * Offsets of layout of a tail queue element.
> >>>> + */
> >>>> +#define QTAILQ_NEXT_OFFSET 0
> >>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
> >>>> +
> >>>> +/*
> >>>> + * Raw access of elements of a tail entry
> >>>> + */
> >>>> +#define QTAILQ_RAW_NEXT(elm, entry) \
> >>>> + (*((void **) (RAW_FIELD(elm, entry + QTAILQ_NEXT_OFFSET))))
> >>>> +#define QTAILQ_RAW_PREV(elm, entry) \
> >>>> + (*((void ***) (RAW_FIELD(elm, entry + QTAILQ_PREV_OFFSET))))
> >>>> +/*
> >>>> + * 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_PREV(elm, entry) = QTAILQ_RAW_LAST(head); \
> >>>> + *QTAILQ_RAW_LAST(head) = (elm); \
> >>>> + QTAILQ_RAW_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 dfee75a..9a6ec59 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 d188afa..fcf808e 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"
> >>>>
> >>>> @@ -948,3 +949,69 @@ 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;
> >>>
> >>> You could just make that a break since you're returning ret
> >>> (that's otherwise all 0). Still, not important, but could
> >>> be tidied if you need to regenerate the patch.
> >>> (and you could remove the 2nd space after elm =).
> >
> >> By returning it, the trace file will miss "get_qtailq_end', that
> >> is useful debug information.
> >> You are right about the space.
> >
> > But that's what you're doing now, you're missing the trace call
> > and returning 'ret' that will always be 0 if it gets there.
> >
> > Dave
> >
> >>
> >> Thanks,
> >> Jianjun
> >>
> >>>
> >>>> + }
> >>>> + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry_offset);
> >>>> + }
> >>>> +
> >>>> + trace_get_qtailq_end(vmsd->name, "end", ret);
> >>>> + return ret;
> >>>> +}
> >>>> +
> >>>> +/* put for QTAILQ */
> >>>> +static void 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");
> >>>> +}
> >>>> +const VMStateInfo vmstate_info_qtailq = {
> >>>> + .name = "qtailq",
> >>>> + .get = get_qtailq,
> >>>> + .put = put_qtailq,
> >>>> +};
> >>>
> >>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >>>
> >>>> --
> >>>> 1.9.1
> >>>>
> >>> --
> >>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >>>
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ
2016-10-26 17:09 ` Dr. David Alan Gilbert
@ 2016-10-26 17:33 ` Jianjun Duan
2016-10-27 11:16 ` Halil Pasic
0 siblings, 1 reply; 14+ messages in thread
From: Jianjun Duan @ 2016-10-26 17:33 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: qemu-devel, qemu-ppc, dmitry, peter.maydell, kraxel, mst, david,
pbonzini, veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth,
aurelien, leon.alrae, blauwirbel, mark.cave-ayland, mdroth
On 10/26/2016 10:09 AM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>>
>>
>> On 10/26/2016 09:53 AM, Dr. David Alan Gilbert wrote:
>>> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>>>>
>>>>
>>>> On 10/26/2016 09:29 AM, Dr. David Alan Gilbert wrote:
>>>>> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>>>>>> 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.
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>> Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
>>>>>> ---
>>>>>> include/migration/vmstate.h | 20 ++++++++++++++
>>>>>> include/qemu/queue.h | 46 +++++++++++++++++++++++++++++++
>>>>>> migration/trace-events | 4 +++
>>>>>> migration/vmstate.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>> 4 files changed, 137 insertions(+)
>>>>>>
>>>>>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>>>>>> index d0e37b5..318a6f1 100644
>>>>>> --- a/include/migration/vmstate.h
>>>>>> +++ b/include/migration/vmstate.h
>>>>>> @@ -251,6 +251,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)
>>>>>> @@ -662,6 +663,25 @@ extern const VMStateInfo vmstate_info_bitmap;
>>>>>> .offset = offsetof(_state, _field), \
>>>>>> }
>>>>>>
>>>>>> +/* For QTAILQ that need customized handling.
>>>>>> + * 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..e9378fa 100644
>>>>>> --- a/include/qemu/queue.h
>>>>>> +++ b/include/qemu/queue.h
>>>>>> @@ -438,4 +438,50 @@ struct { \
>>>>>> #define QTAILQ_PREV(elm, headname, field) \
>>>>>> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
>>>>>>
>>>>>> +#define RAW_FIELD(base, offset) \
>>>>>> + ((char *) (base) + offset)
>>>>>> +
>>>>>> +/*
>>>>>> + * Offsets of layout of a tail queue head.
>>>>>> + */
>>>>>> +#define QTAILQ_FIRST_OFFSET 0
>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>>>>>
>>>>> OK, you might want to add some asserts somewhere in a .c just to catch if
>>>>> any of these offsets change.
>>>>>
>>>> if the layout of QTAILQ changes, the version id for the vmsd should also
>>>> be changed. It will break migration between different versions. However
>>>> the operation doesn't depend on these values.
>>>
>>> No, changing layout of QTAILQ doesn't need to change the version id of vmsd;
>>> it's an internal change. But if someone does make the change and forgets
>>> to update your OFFSET macros it'll cause memory corruption.
>>> You could catch that with an assert (possibly build time).
>>>
>> If we use these constant I would agree an assertion is necessary. By
>> using a macro we leave the door open for change. and if someone changes
>> the layout, he or she better change the macros and the version id. If an
>> assertion is added, then that assertion needs to be changed to reflect
>> the change, then in the unlikely situations we could have several
>> version of layout/macro/assertions floating around. That is too much. SO
>> version id is the best guard here.
>
> Version id's are irrelevant here.
> The macros are irrelevant here.
> I'm talking about a potential mismatch between the definition of QTAILQ_LAST_OFFSET
> and the definition of Q_TAILQ_HEAD.
>
> Dave
>
I suppose anyone who changes the layout should also change the macro and
version ID of the relevant vmsd. Similar issue was discussed before as
the early version tried to generate all these offsets based on the
element and head type. You can see in version 2 discussion.
Thanks,
Jianjun
>>
>> Thanks,
>> Jianjun
>>
>>>>>> + * Raw access of elements of a tail queue
>>>>>> + */
>>>>>> +#define QTAILQ_RAW_FIRST(head) \
>>>>>> + (*((void **) (RAW_FIELD(head, QTAILQ_FIRST_OFFSET))))
>>>>>> +#define QTAILQ_RAW_LAST(head) \
>>>>>> + (*((void ***) (RAW_FIELD(head, QTAILQ_LAST_OFFSET))))
>>>>>> +
>>>>>> +/*
>>>>>> + * Offsets of layout of a tail queue element.
>>>>>> + */
>>>>>> +#define QTAILQ_NEXT_OFFSET 0
>>>>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
>>>>>> +
>>>>>> +/*
>>>>>> + * Raw access of elements of a tail entry
>>>>>> + */
>>>>>> +#define QTAILQ_RAW_NEXT(elm, entry) \
>>>>>> + (*((void **) (RAW_FIELD(elm, entry + QTAILQ_NEXT_OFFSET))))
>>>>>> +#define QTAILQ_RAW_PREV(elm, entry) \
>>>>>> + (*((void ***) (RAW_FIELD(elm, entry + QTAILQ_PREV_OFFSET))))
>>>>>> +/*
>>>>>> + * 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_PREV(elm, entry) = QTAILQ_RAW_LAST(head); \
>>>>>> + *QTAILQ_RAW_LAST(head) = (elm); \
>>>>>> + QTAILQ_RAW_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 dfee75a..9a6ec59 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 d188afa..fcf808e 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"
>>>>>>
>>>>>> @@ -948,3 +949,69 @@ 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;
>>>>>
>>>>> You could just make that a break since you're returning ret
>>>>> (that's otherwise all 0). Still, not important, but could
>>>>> be tidied if you need to regenerate the patch.
>>>>> (and you could remove the 2nd space after elm =).
>>>
>>>> By returning it, the trace file will miss "get_qtailq_end', that
>>>> is useful debug information.
>>>> You are right about the space.
>>>
>>> But that's what you're doing now, you're missing the trace call
>>> and returning 'ret' that will always be 0 if it gets there.
>>>
>>> Dave
>>>
>>>>
>>>> Thanks,
>>>> Jianjun
>>>>
>>>>>
>>>>>> + }
>>>>>> + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry_offset);
>>>>>> + }
>>>>>> +
>>>>>> + trace_get_qtailq_end(vmsd->name, "end", ret);
>>>>>> + return ret;
>>>>>> +}
>>>>>> +
>>>>>> +/* put for QTAILQ */
>>>>>> +static void 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");
>>>>>> +}
>>>>>> +const VMStateInfo vmstate_info_qtailq = {
>>>>>> + .name = "qtailq",
>>>>>> + .get = get_qtailq,
>>>>>> + .put = put_qtailq,
>>>>>> +};
>>>>>
>>>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>>>
>>>>>> --
>>>>>> 1.9.1
>>>>>>
>>>>> --
>>>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>>>>>
>>>>
>>> --
>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ
2016-10-26 17:33 ` Jianjun Duan
@ 2016-10-27 11:16 ` Halil Pasic
2016-10-27 16:49 ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
0 siblings, 1 reply; 14+ messages in thread
From: Halil Pasic @ 2016-10-27 11:16 UTC (permalink / raw)
To: Jianjun Duan, Dr. David Alan Gilbert
Cc: veroniabahaa, peter.maydell, mdroth, mst, quintela,
mark.cave-ayland, qemu-devel, mreitz, blauwirbel, amit.shah,
qemu-ppc, kraxel, kwolf, dmitry, pbonzini, rth, leon.alrae,
aurelien, david
On 10/26/2016 07:33 PM, Jianjun Duan wrote:
>>>>>>> +#define QTAILQ_FIRST_OFFSET 0
>>>>>>> >>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>>>>>> >>>>>
>>>>>> >>>>> OK, you might want to add some asserts somewhere in a .c just to catch if
>>>>>> >>>>> any of these offsets change.
>>>>>> >>>>>
>>>>> >>>> if the layout of QTAILQ changes, the version id for the vmsd should also
>>>>> >>>> be changed. It will break migration between different versions. However
>>>>> >>>> the operation doesn't depend on these values.
>>>> >>>
>>>> >>> No, changing layout of QTAILQ doesn't need to change the version id of vmsd;
>>>> >>> it's an internal change. But if someone does make the change and forgets
>>>> >>> to update your OFFSET macros it'll cause memory corruption.
>>>> >>> You could catch that with an assert (possibly build time).
>>>> >>>
>>> >> If we use these constant I would agree an assertion is necessary. By
>>> >> using a macro we leave the door open for change. and if someone changes
>>> >> the layout, he or she better change the macros and the version id. If an
>>> >> assertion is added, then that assertion needs to be changed to reflect
>>> >> the change, then in the unlikely situations we could have several
>>> >> version of layout/macro/assertions floating around. That is too much. SO
>>> >> version id is the best guard here.
>> >
>> > Version id's are irrelevant here.
>> > The macros are irrelevant here.
>> > I'm talking about a potential mismatch between the definition of QTAILQ_LAST_OFFSET
>> > and the definition of Q_TAILQ_HEAD.
>> >
>> > Dave
>> >
> I suppose anyone who changes the layout should also change the macro and
> version ID of the relevant vmsd. Similar issue was discussed before as
> the early version tried to generate all these offsets based on the
> element and head type. You can see in version 2 discussion.
>
>
> Thanks,
> Jianjun
IMHO Dave is right, although a change in head/entry does not seem too
likely to me. Just want to point out that Dave's proposal (discussed here
https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg03770.html)
does not have these issues and it appears more elegant to me. Jianjun
why do you prefer doing the offsets manually?
Halil
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v8 2/3] migration: migrate QTAILQ
2016-10-27 11:16 ` Halil Pasic
@ 2016-10-27 16:49 ` Jianjun Duan
0 siblings, 0 replies; 14+ messages in thread
From: Jianjun Duan @ 2016-10-27 16:49 UTC (permalink / raw)
To: Halil Pasic, Dr. David Alan Gilbert
Cc: veroniabahaa, peter.maydell, mdroth, mst, quintela,
mark.cave-ayland, qemu-devel, mreitz, blauwirbel, amit.shah,
qemu-ppc, kraxel, kwolf, dmitry, pbonzini, rth, leon.alrae,
aurelien, david
On 10/27/2016 04:16 AM, Halil Pasic wrote:
>
>
> On 10/26/2016 07:33 PM, Jianjun Duan wrote:
>>>>>>>> +#define QTAILQ_FIRST_OFFSET 0
>>>>>>>>>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>>>>>>>>>>>>
>>>>>>>>>>>> OK, you might want to add some asserts somewhere in a .c just to catch if
>>>>>>>>>>>> any of these offsets change.
>>>>>>>>>>>>
>>>>>>>>>> if the layout of QTAILQ changes, the version id for the vmsd should also
>>>>>>>>>> be changed. It will break migration between different versions. However
>>>>>>>>>> the operation doesn't depend on these values.
>>>>>>>>
>>>>>>>> No, changing layout of QTAILQ doesn't need to change the version id of vmsd;
>>>>>>>> it's an internal change. But if someone does make the change and forgets
>>>>>>>> to update your OFFSET macros it'll cause memory corruption.
>>>>>>>> You could catch that with an assert (possibly build time).
>>>>>>>>
>>>>>> If we use these constant I would agree an assertion is necessary. By
>>>>>> using a macro we leave the door open for change. and if someone changes
>>>>>> the layout, he or she better change the macros and the version id. If an
>>>>>> assertion is added, then that assertion needs to be changed to reflect
>>>>>> the change, then in the unlikely situations we could have several
>>>>>> version of layout/macro/assertions floating around. That is too much. SO
>>>>>> version id is the best guard here.
>>>>
>>>> Version id's are irrelevant here.
>>>> The macros are irrelevant here.
>>>> I'm talking about a potential mismatch between the definition of QTAILQ_LAST_OFFSET
>>>> and the definition of Q_TAILQ_HEAD.
>>>>
>>>> Dave
>>>>
>> I suppose anyone who changes the layout should also change the macro and
>> version ID of the relevant vmsd. Similar issue was discussed before as
>> the early version tried to generate all these offsets based on the
>> element and head type. You can see in version 2 discussion.
>>
>>
>> Thanks,
>> Jianjun
>
> IMHO Dave is right, although a change in head/entry does not seem too
> likely to me. Just want to point out that Dave's proposal (discussed here
> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg03770.html)
> does not have these issues and it appears more elegant to me. Jianjun
> why do you prefer doing the offsets manually?
>
It came to the current shape based on previous comments stretching back
almost 6 months. I can change it to whatever reviewers want.
Thanks,
Jianjun
> Halil
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-10-27 16:49 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-25 20:47 [Qemu-devel] [QEMU PATCH v8 0/3] migration: migrate QTAILQ Jianjun Duan
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 1/3] migration: extend VMStateInfo Jianjun Duan
2016-10-26 12:14 ` Dr. David Alan Gilbert
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ Jianjun Duan
2016-10-26 16:29 ` Dr. David Alan Gilbert
2016-10-26 16:50 ` Jianjun Duan
2016-10-26 16:53 ` Dr. David Alan Gilbert
2016-10-26 17:04 ` Jianjun Duan
2016-10-26 17:09 ` Dr. David Alan Gilbert
2016-10-26 17:33 ` Jianjun Duan
2016-10-27 11:16 ` Halil Pasic
2016-10-27 16:49 ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
2016-10-25 20:47 ` [Qemu-devel] [QEMU PATCH v8 3/3] tests/migration: Add test for QTAILQ migration Jianjun Duan
2016-10-26 16:48 ` Dr. David Alan Gilbert
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.