* [Qemu-devel] [QEMU PATCH v6 0/2] migration: migrate QTAILQ
@ 2016-10-13 21:30 Jianjun Duan
2016-10-13 21:30 ` [Qemu-devel] [QEMU PATCH v6 1/2] migration: extend VMStateInfo Jianjun Duan
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Jianjun Duan @ 2016-10-13 21:30 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
v6: - Split from Power specific patches.
- Dropped VMS_LINKED flag.
- Rebased to master.
- Added comments to clarify about put/get in VMStateInfo.
Previous versions are:
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 (2):
migration: extend VMStateInfo
migration: migrate QTAILQ
hw/display/virtio-gpu.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 | 32 +++++++++
migration/savevm.c | 5 +-
migration/trace-events | 4 ++
migration/vmstate.c | 163 ++++++++++++++++++++++++++++++++++----------
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 +-
23 files changed, 307 insertions(+), 102 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [QEMU PATCH v6 1/2] migration: extend VMStateInfo
2016-10-13 21:30 [Qemu-devel] [QEMU PATCH v6 0/2] migration: migrate QTAILQ Jianjun Duan
@ 2016-10-13 21:30 ` Jianjun Duan
2016-10-14 9:23 ` Dr. David Alan Gilbert
2016-10-13 21:30 ` [Qemu-devel] [QEMU PATCH v6 2/2] migration: migrate QTAILQ Jianjun Duan
2016-10-14 2:01 ` [Qemu-devel] [QEMU PATCH v6 0/2] " no-reply
2 siblings, 1 reply; 18+ messages in thread
From: Jianjun Duan @ 2016-10-13 21:30 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/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 | 104 ++++++++++++++++++++++++++++----------------
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 ++-
21 files changed, 192 insertions(+), 102 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/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..1b85c51 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,
+ void *opaque, 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,
+ void *opaque)
{
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..e8eff3f 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -6,6 +6,7 @@
#include "qemu/bitops.h"
#include "qemu/error-report.h"
#include "trace.h"
+#include "migration/qjson.h"
static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, QJSON *vmdesc);
@@ -122,8 +123,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 +331,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 +468,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 +490,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 +512,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 +534,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 +557,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 +580,7 @@ const VMStateInfo vmstate_info_int32_equal = {
* and less than or equal to the one in the field.
*/
-static int get_int32_le(QEMUFile *f, void *pv, size_t size)
+static int get_int32_le(QEMUFile *f, void *pv, size_t size, VMStateField *field)
{
int32_t *cur = pv;
int32_t loaded;
@@ -589,14 +601,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 +623,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 +645,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 +667,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 +690,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 +711,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 +734,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 +756,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 +778,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 +799,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 +808,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 +824,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 +833,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 +849,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 +873,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 +887,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 +913,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 +927,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] 18+ messages in thread
* [Qemu-devel] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-13 21:30 [Qemu-devel] [QEMU PATCH v6 0/2] migration: migrate QTAILQ Jianjun Duan
2016-10-13 21:30 ` [Qemu-devel] [QEMU PATCH v6 1/2] migration: extend VMStateInfo Jianjun Duan
@ 2016-10-13 21:30 ` Jianjun Duan
2016-10-14 10:44 ` Dr. David Alan Gilbert
2016-10-14 2:01 ` [Qemu-devel] [QEMU PATCH v6 0/2] " no-reply
2 siblings, 1 reply; 18+ messages in thread
From: Jianjun Duan @ 2016-10-13 21:30 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 | 32 ++++++++++++++++++++++++
migration/trace-events | 4 +++
migration/vmstate.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 115 insertions(+)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index d0e37b5..4dd0aed 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
+ * _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, \
+ .flags = VMS_LINKED, \
+ .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..d672ae0 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -438,4 +438,36 @@ struct { \
#define QTAILQ_PREV(elm, headname, field) \
(*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
+/*
+ * Offsets of layout of a tail queue head.
+ */
+#define QTAILQ_FIRST_OFFSET 0
+#define QTAILQ_LAST_OFFSET (sizeof(void *))
+
+/*
+ * Offsets of layout of a tail queue element.
+ */
+#define QTAILQ_NEXT_OFFSET 0
+#define QTAILQ_PREV_OFFSET (sizeof(void *))
+
+/*
+ * Tail queue tranversal using pointer arithmetic.
+ */
+#define QTAILQ_RAW_FOREACH(elm, head, entry) \
+ for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \
+ (elm); \
+ (elm) = \
+ *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)))
+/*
+ * Tail queue insertion using pointer arithmetic.
+ */
+#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
+ *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
+ *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
+ *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
+ **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
+ *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
+ (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
+} 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 e8eff3f..eb68fc6 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"
@@ -946,3 +947,61 @@ const VMStateInfo vmstate_info_bitmap = {
.get = get_bitmap,
.put = put_bitmap,
};
+
+/*get for QTAILQ */
+static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size,
+ VMStateField *field)
+{
+ int ret = 0;
+ const VMStateDescription *vmsd = field->vmsd;
+ size_t size = field->size;
+ size_t entry = field->start;
+ int version_id = field->version_id;
+ void *elm;
+
+ trace_get_qtailq(vmsd->name, version_id);
+ if (version_id > vmsd->version_id) {
+ trace_get_qtailq_end(vmsd->name, "too new", -EINVAL);
+ return -EINVAL;
+ }
+ if (version_id < vmsd->minimum_version_id) {
+ 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);
+ }
+
+ 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;
+ size_t entry = field->start;
+ void *elm;
+
+ trace_put_qtailq(vmsd->name, vmsd->version_id);
+
+ QTAILQ_RAW_FOREACH(elm, pv, entry) {
+ 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] 18+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v6 0/2] migration: migrate QTAILQ
2016-10-13 21:30 [Qemu-devel] [QEMU PATCH v6 0/2] migration: migrate QTAILQ Jianjun Duan
2016-10-13 21:30 ` [Qemu-devel] [QEMU PATCH v6 1/2] migration: extend VMStateInfo Jianjun Duan
2016-10-13 21:30 ` [Qemu-devel] [QEMU PATCH v6 2/2] migration: migrate QTAILQ Jianjun Duan
@ 2016-10-14 2:01 ` no-reply
2 siblings, 0 replies; 18+ messages in thread
From: no-reply @ 2016-10-14 2:01 UTC (permalink / raw)
To: duanj
Cc: famz, qemu-devel, veroniabahaa, peter.maydell, dgilbert, mst,
quintela, mark.cave-ayland, mdroth, mreitz, blauwirbel,
amit.shah, qemu-ppc, kraxel, kwolf, dmitry, pbonzini, rth,
leon.alrae, aurelien, david
Hi,
Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.
Message-id: 1476394254-7987-1-git-send-email-duanj@linux.vnet.ibm.com
Type: series
Subject: [Qemu-devel] [QEMU PATCH v6 0/2] migration: migrate QTAILQ
=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=16
make docker-test-quick@centos6
make docker-test-mingw@fedora
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
1315f2a migration: migrate QTAILQ
48aed08 migration: extend VMStateInfo
=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
BUILD centos6
=== OUTPUT END ===
Abort: command timeout (>3600 seconds)
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v6 1/2] migration: extend VMStateInfo
2016-10-13 21:30 ` [Qemu-devel] [QEMU PATCH v6 1/2] migration: extend VMStateInfo Jianjun Duan
@ 2016-10-14 9:23 ` Dr. David Alan Gilbert
2016-10-14 16:58 ` Jianjun Duan
0 siblings, 1 reply; 18+ messages in thread
From: Dr. David Alan Gilbert @ 2016-10-14 9:23 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>
> ---
> hw/display/virtio-gpu.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 | 104 ++++++++++++++++++++++++++++----------------
> 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 ++-
> 21 files changed, 192 insertions(+), 102 deletions(-)
<snip>
> -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..1b85c51 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,
> + void *opaque, 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,
> + void *opaque)
The types here are wrong for usbredir_put_parser and usbredir_get_parser; the 'void *opaque'
should be a VMStateField *field; it causes a build failure for me (I suspect only
if you have the usbredir libraries)
Also, are you missing kvm_flic_load/save from hw/intc/s390_flic_kvm.c ?
Other than that, it looks good.
Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-13 21:30 ` [Qemu-devel] [QEMU PATCH v6 2/2] migration: migrate QTAILQ Jianjun Duan
@ 2016-10-14 10:44 ` Dr. David Alan Gilbert
2016-10-14 11:07 ` Paolo Bonzini
2016-10-14 16:56 ` Jianjun Duan
0 siblings, 2 replies; 18+ messages in thread
From: Dr. David Alan Gilbert @ 2016-10-14 10:44 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 | 32 ++++++++++++++++++++++++
> migration/trace-events | 4 +++
> migration/vmstate.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 115 insertions(+)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index d0e37b5..4dd0aed 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
> + * _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, \
> + .flags = VMS_LINKED, \
Hang on - you killed VMS_LINKED off in the previous patch; so this
doesn't work.
(Adding a test to tests/test-vmstate.c would have found that)
> + .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..d672ae0 100644
> --- a/include/qemu/queue.h
> +++ b/include/qemu/queue.h
> @@ -438,4 +438,36 @@ struct { \
> #define QTAILQ_PREV(elm, headname, field) \
> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
>
> +/*
> + * Offsets of layout of a tail queue head.
> + */
> +#define QTAILQ_FIRST_OFFSET 0
> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
> +
> +/*
> + * Offsets of layout of a tail queue element.
> + */
> +#define QTAILQ_NEXT_OFFSET 0
> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
> +
> +/*
> + * Tail queue tranversal using pointer arithmetic.
> + */
> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \
> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \
> + (elm); \
> + (elm) = \
> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)))
> +/*
> + * Tail queue insertion using pointer arithmetic.
> + */
> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
> +} while (/*CONSTCOND*/0)
I wonder if there's a simpler way to do this; I'm not sure this works, but something like:
struct QTAILQDummy {
char dummy;
};
QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
#define QTAILQ_RAW_FOREACH(elm, head, entry) \
for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \
(elm); \
(elm) = \
(elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next
and then I think elm gets declared as a struct QTAILQDummy.
But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
Would that work?
> #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 e8eff3f..eb68fc6 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"
>
> @@ -946,3 +947,61 @@ const VMStateInfo vmstate_info_bitmap = {
> .get = get_bitmap,
> .put = put_bitmap,
> };
> +
> +/*get for QTAILQ */
> +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size,
> + VMStateField *field)
> +{
> + int ret = 0;
> + const VMStateDescription *vmsd = field->vmsd;
> + size_t size = field->size;
> + size_t entry = field->start;
Please comment these two here; I'd prefer entry_offset or something
as a name to make clear what it is.
> + int version_id = field->version_id;
> + void *elm;
> +
> + trace_get_qtailq(vmsd->name, version_id);
> + if (version_id > vmsd->version_id) {
> + trace_get_qtailq_end(vmsd->name, "too new", -EINVAL);
> + return -EINVAL;
> + }
> + if (version_id < vmsd->minimum_version_id) {
> + trace_get_qtailq_end(vmsd->name, "too old", -EINVAL);
> + return -EINVAL;
> + }
Please make those error_report's - if anything fails migration
I like to see it in the stderr log.
> + 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);
> + }
> +
> + 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;
> + size_t entry = field->start;
> + void *elm;
> +
> + trace_put_qtailq(vmsd->name, vmsd->version_id);
> +
> + QTAILQ_RAW_FOREACH(elm, pv, entry) {
> + 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
Dave
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-14 10:44 ` Dr. David Alan Gilbert
@ 2016-10-14 11:07 ` Paolo Bonzini
2016-10-14 16:43 ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
2016-10-14 16:56 ` Jianjun Duan
1 sibling, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2016-10-14 11:07 UTC (permalink / raw)
To: Dr. David Alan Gilbert, Jianjun Duan
Cc: qemu-devel, qemu-ppc, dmitry, peter.maydell, kraxel, mst, david,
veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth, aurelien,
leon.alrae, blauwirbel, mark.cave-ayland, mdroth
On 14/10/2016 12:44, Dr. David Alan Gilbert wrote:
>> > +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
>> > + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
>> > + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
>> > + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
>> > + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
>> > + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
>> > + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
>> > +} while (/*CONSTCOND*/0)
> I wonder if there's a simpler way to do this; I'm not sure this works, but something like:
>
> struct QTAILQDummy {
> char dummy;
> };
>
> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
>
> #define QTAILQ_RAW_FOREACH(elm, head, entry) \
> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \
> (elm); \
> (elm) = \
> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next
>
> and then I think elm gets declared as a struct QTAILQDummy.
> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
Another possibility is a macro like
#define field_at_offset(base, offset, type) \
((type) (((char *) (base)) + (offset)))
so that you can do
*field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL;
*field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) =
*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET);
**field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm);
*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) =
field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET);
or something like that (note that I've always used the same type for
next and last, by the way).
Paolo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-14 11:07 ` Paolo Bonzini
@ 2016-10-14 16:43 ` Jianjun Duan
2016-10-14 18:39 ` Paolo Bonzini
0 siblings, 1 reply; 18+ messages in thread
From: Jianjun Duan @ 2016-10-14 16:43 UTC (permalink / raw)
To: Paolo Bonzini, Dr. David Alan Gilbert
Cc: qemu-devel, qemu-ppc, dmitry, peter.maydell, kraxel, mst, david,
veroniabahaa, quintela, amit.shah, mreitz, kwolf, rth, aurelien,
leon.alrae, blauwirbel, mark.cave-ayland, mdroth
I need to double check my code. My build passed and migration test also
succeeded.
On 10/14/2016 04:07 AM, Paolo Bonzini wrote:
>
>
> On 14/10/2016 12:44, Dr. David Alan Gilbert wrote:
>>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
>>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
>>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
>>>> +} while (/*CONSTCOND*/0)
>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like:
>>
>> struct QTAILQDummy {
>> char dummy;
>> };
>>
>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
>>
>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \
>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \
>> (elm); \
>> (elm) = \
>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next
>>
>> and then I think elm gets declared as a struct QTAILQDummy.
>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
>
> Another possibility is a macro like
>
> #define field_at_offset(base, offset, type) \
> ((type) (((char *) (base)) + (offset)))
>
> so that you can do
>
> *field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL;
> *field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) =
> *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET);
> **field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm);
> *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) =
> field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET);
The thing is that we don't know type at all. So only the offset values
is used. This is intended for QTAILQ of any type.
Thanks,
Jianjun
>
> or something like that (note that I've always used the same type for
> next and last, by the way).
>
> Paolo
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-14 10:44 ` Dr. David Alan Gilbert
2016-10-14 11:07 ` Paolo Bonzini
@ 2016-10-14 16:56 ` Jianjun Duan
2016-10-14 17:04 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 18+ messages in thread
From: Jianjun Duan @ 2016-10-14 16:56 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/14/2016 03:44 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 | 32 ++++++++++++++++++++++++
>> migration/trace-events | 4 +++
>> migration/vmstate.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 115 insertions(+)
>>
>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>> index d0e37b5..4dd0aed 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
>> + * _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, \
>> + .flags = VMS_LINKED, \
>
> Hang on - you killed VMS_LINKED off in the previous patch; so this
> doesn't work.
> (Adding a test to tests/test-vmstate.c would have found that)
>
Thanks for pointing it out. Since the actual application of this patch
is split off, my test didn't go through this path.
>> + .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..d672ae0 100644
>> --- a/include/qemu/queue.h
>> +++ b/include/qemu/queue.h
>> @@ -438,4 +438,36 @@ struct { \
>> #define QTAILQ_PREV(elm, headname, field) \
>> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
>>
>> +/*
>> + * Offsets of layout of a tail queue head.
>> + */
>> +#define QTAILQ_FIRST_OFFSET 0
>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>> +
>> +/*
>> + * Offsets of layout of a tail queue element.
>> + */
>> +#define QTAILQ_NEXT_OFFSET 0
>> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
>> +
>> +/*
>> + * Tail queue tranversal using pointer arithmetic.
>> + */
>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \
>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \
>> + (elm); \
>> + (elm) = \
>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)))
>> +/*
>> + * Tail queue insertion using pointer arithmetic.
>> + */
>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
>> +} while (/*CONSTCOND*/0)
>
> I wonder if there's a simpler way to do this; I'm not sure this works, but something like:
>
> struct QTAILQDummy {
> char dummy;
> };
>
> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
>
> #define QTAILQ_RAW_FOREACH(elm, head, entry) \
> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \
> (elm); \
> (elm) = \
> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next
>
> and then I think elm gets declared as a struct QTAILQDummy.
> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
>
> Would that work?
>
It is intended for QTAILQ of any type. So type is not available.
>> #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 e8eff3f..eb68fc6 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"
>>
>> @@ -946,3 +947,61 @@ const VMStateInfo vmstate_info_bitmap = {
>> .get = get_bitmap,
>> .put = put_bitmap,
>> };
>> +
>> +/*get for QTAILQ */
>> +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size,
>> + VMStateField *field)
>> +{
>> + int ret = 0;
>> + const VMStateDescription *vmsd = field->vmsd;
>> + size_t size = field->size;
>> + size_t entry = field->start;
>
> Please comment these two here; I'd prefer entry_offset or something
> as a name to make clear what it is.
>
OK.
>> + int version_id = field->version_id;
>> + void *elm;
>> +
>> + trace_get_qtailq(vmsd->name, version_id);
>> + if (version_id > vmsd->version_id) {
>> + trace_get_qtailq_end(vmsd->name, "too new", -EINVAL);
>> + return -EINVAL;
>> + }
>> + if (version_id < vmsd->minimum_version_id) {
>> + trace_get_qtailq_end(vmsd->name, "too old", -EINVAL);
>> + return -EINVAL;
>> + }
>
> Please make those error_report's - if anything fails migration
> I like to see it in the stderr log.
>
I know previously you wanted it to be error report. But in the migration
code the same error is always just put into trace (vmastate_load_state).
I just want to be consistent.
Thanks,
Jianjun
>> + 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);
>> + }
>> +
>> + 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;
>> + size_t entry = field->start;
>> + void *elm;
>> +
>> + trace_put_qtailq(vmsd->name, vmsd->version_id);
>> +
>> + QTAILQ_RAW_FOREACH(elm, pv, entry) {
>> + 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
>
> Dave
>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v6 1/2] migration: extend VMStateInfo
2016-10-14 9:23 ` Dr. David Alan Gilbert
@ 2016-10-14 16:58 ` Jianjun Duan
0 siblings, 0 replies; 18+ messages in thread
From: Jianjun Duan @ 2016-10-14 16:58 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/14/2016 02:23 AM, Dr. David Alan Gilbert wrote:
> * 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>
>> ---
>> hw/display/virtio-gpu.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 | 104 ++++++++++++++++++++++++++++----------------
>> 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 ++-
>> 21 files changed, 192 insertions(+), 102 deletions(-)
>
> <snip>
>
>> -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..1b85c51 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,
>> + void *opaque, 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,
>> + void *opaque)
>
> The types here are wrong for usbredir_put_parser and usbredir_get_parser; the 'void *opaque'
> should be a VMStateField *field; it causes a build failure for me (I suspect only
> if you have the usbredir libraries)
>
> Also, are you missing kvm_flic_load/save from hw/intc/s390_flic_kvm.c ?
>
> Other than that, it looks good.
>
Will double check.
Thanks,
Jianjun
> Dave
>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-14 16:56 ` Jianjun Duan
@ 2016-10-14 17:04 ` Dr. David Alan Gilbert
2016-10-14 17:18 ` Jianjun Duan
0 siblings, 1 reply; 18+ messages in thread
From: Dr. David Alan Gilbert @ 2016-10-14 17:04 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/14/2016 03:44 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 | 32 ++++++++++++++++++++++++
> >> migration/trace-events | 4 +++
> >> migration/vmstate.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 115 insertions(+)
> >>
> >> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> >> index d0e37b5..4dd0aed 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
> >> + * _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, \
> >> + .flags = VMS_LINKED, \
> >
> > Hang on - you killed VMS_LINKED off in the previous patch; so this
> > doesn't work.
> > (Adding a test to tests/test-vmstate.c would have found that)
> >
>
> Thanks for pointing it out. Since the actual application of this patch
> is split off, my test didn't go through this path.
Yes, but you could add a new entry into test-vmstate.c for this.
> >> + .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..d672ae0 100644
> >> --- a/include/qemu/queue.h
> >> +++ b/include/qemu/queue.h
> >> @@ -438,4 +438,36 @@ struct { \
> >> #define QTAILQ_PREV(elm, headname, field) \
> >> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
> >>
> >> +/*
> >> + * Offsets of layout of a tail queue head.
> >> + */
> >> +#define QTAILQ_FIRST_OFFSET 0
> >> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
> >> +
> >> +/*
> >> + * Offsets of layout of a tail queue element.
> >> + */
> >> +#define QTAILQ_NEXT_OFFSET 0
> >> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
> >> +
> >> +/*
> >> + * Tail queue tranversal using pointer arithmetic.
> >> + */
> >> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \
> >> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \
> >> + (elm); \
> >> + (elm) = \
> >> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)))
> >> +/*
> >> + * Tail queue insertion using pointer arithmetic.
> >> + */
> >> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
> >> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
> >> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
> >> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
> >> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
> >> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
> >> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
> >> +} while (/*CONSTCOND*/0)
> >
> > I wonder if there's a simpler way to do this; I'm not sure this works, but something like:
> >
> > struct QTAILQDummy {
> > char dummy;
> > };
> >
> > QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
> > typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
> >
> > #define QTAILQ_RAW_FOREACH(elm, head, entry) \
> > for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \
> > (elm); \
> > (elm) = \
> > (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next
> >
> > and then I think elm gets declared as a struct QTAILQDummy.
> > But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
> >
> > Would that work?
> >
> It is intended for QTAILQ of any type. So type is not available.
I think it might be possible to do it generally.
> >> #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 e8eff3f..eb68fc6 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"
> >>
> >> @@ -946,3 +947,61 @@ const VMStateInfo vmstate_info_bitmap = {
> >> .get = get_bitmap,
> >> .put = put_bitmap,
> >> };
> >> +
> >> +/*get for QTAILQ */
> >> +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size,
> >> + VMStateField *field)
> >> +{
> >> + int ret = 0;
> >> + const VMStateDescription *vmsd = field->vmsd;
> >> + size_t size = field->size;
> >> + size_t entry = field->start;
> >
> > Please comment these two here; I'd prefer entry_offset or something
> > as a name to make clear what it is.
> >
> OK.
> >> + int version_id = field->version_id;
> >> + void *elm;
> >> +
> >> + trace_get_qtailq(vmsd->name, version_id);
> >> + if (version_id > vmsd->version_id) {
> >> + trace_get_qtailq_end(vmsd->name, "too new", -EINVAL);
> >> + return -EINVAL;
> >> + }
> >> + if (version_id < vmsd->minimum_version_id) {
> >> + trace_get_qtailq_end(vmsd->name, "too old", -EINVAL);
> >> + return -EINVAL;
> >> + }
> >
> > Please make those error_report's - if anything fails migration
> > I like to see it in the stderr log.
> >
> I know previously you wanted it to be error report. But in the migration
> code the same error is always just put into trace (vmastate_load_state).
> I just want to be consistent.
I should fix those as well to add error_reports !
It's very difficult when we get an error back from a customer with just
a migration failure when the only thing we know is an -EINVAL, so I keep
adding error_report calls.
Dave
> Thanks,
> Jianjun
> >> + 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);
> >> + }
> >> +
> >> + 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;
> >> + size_t entry = field->start;
> >> + void *elm;
> >> +
> >> + trace_put_qtailq(vmsd->name, vmsd->version_id);
> >> +
> >> + QTAILQ_RAW_FOREACH(elm, pv, entry) {
> >> + 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
> >
> > Dave
> >
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-14 17:04 ` Dr. David Alan Gilbert
@ 2016-10-14 17:18 ` Jianjun Duan
2016-10-15 12:48 ` Halil Pasic
0 siblings, 1 reply; 18+ messages in thread
From: Jianjun Duan @ 2016-10-14 17:18 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/14/2016 10:04 AM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>>
>>
>> On 10/14/2016 03:44 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 | 32 ++++++++++++++++++++++++
>>>> migration/trace-events | 4 +++
>>>> migration/vmstate.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
>>>> 4 files changed, 115 insertions(+)
>>>>
>>>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>>>> index d0e37b5..4dd0aed 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
>>>> + * _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, \
>>>> + .flags = VMS_LINKED, \
>>>
>>> Hang on - you killed VMS_LINKED off in the previous patch; so this
>>> doesn't work.
>>> (Adding a test to tests/test-vmstate.c would have found that)
>>>
>>
>> Thanks for pointing it out. Since the actual application of this patch
>> is split off, my test didn't go through this path.
>
> Yes, but you could add a new entry into test-vmstate.c for this.
OK.
>
>>>> + .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..d672ae0 100644
>>>> --- a/include/qemu/queue.h
>>>> +++ b/include/qemu/queue.h
>>>> @@ -438,4 +438,36 @@ struct { \
>>>> #define QTAILQ_PREV(elm, headname, field) \
>>>> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
>>>>
>>>> +/*
>>>> + * Offsets of layout of a tail queue head.
>>>> + */
>>>> +#define QTAILQ_FIRST_OFFSET 0
>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>>>> +
>>>> +/*
>>>> + * Offsets of layout of a tail queue element.
>>>> + */
>>>> +#define QTAILQ_NEXT_OFFSET 0
>>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
>>>> +
>>>> +/*
>>>> + * Tail queue tranversal using pointer arithmetic.
>>>> + */
>>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \
>>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \
>>>> + (elm); \
>>>> + (elm) = \
>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)))
>>>> +/*
>>>> + * Tail queue insertion using pointer arithmetic.
>>>> + */
>>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
>>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
>>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
>>>> +} while (/*CONSTCOND*/0)
>>>
>>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like:
>>>
>>> struct QTAILQDummy {
>>> char dummy;
>>> };
>>>
>>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
>>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
>>>
>>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \
>>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \
>>> (elm); \
>>> (elm) = \
>>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next
>>>
>>> and then I think elm gets declared as a struct QTAILQDummy.
>>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
>>>
>>> Would that work?
>>>
>> It is intended for QTAILQ of any type. So type is not available.
>
> I think it might be possible to do it generally.
>
If we have type, then we can use what is there already, and don't need a
pointer arithmetic based approach. Inside put/get, we only get type
layout info from vmsd, which is all about size and offset. This macro
is used inside put/get, so I am not sure how we can directly use type
here.
>>>> #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 e8eff3f..eb68fc6 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"
>>>>
>>>> @@ -946,3 +947,61 @@ const VMStateInfo vmstate_info_bitmap = {
>>>> .get = get_bitmap,
>>>> .put = put_bitmap,
>>>> };
>>>> +
>>>> +/*get for QTAILQ */
>>>> +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size,
>>>> + VMStateField *field)
>>>> +{
>>>> + int ret = 0;
>>>> + const VMStateDescription *vmsd = field->vmsd;
>>>> + size_t size = field->size;
>>>> + size_t entry = field->start;
>>>
>>> Please comment these two here; I'd prefer entry_offset or something
>>> as a name to make clear what it is.
>>>
>> OK.
>>>> + int version_id = field->version_id;
>>>> + void *elm;
>>>> +
>>>> + trace_get_qtailq(vmsd->name, version_id);
>>>> + if (version_id > vmsd->version_id) {
>>>> + trace_get_qtailq_end(vmsd->name, "too new", -EINVAL);
>>>> + return -EINVAL;
>>>> + }
>>>> + if (version_id < vmsd->minimum_version_id) {
>>>> + trace_get_qtailq_end(vmsd->name, "too old", -EINVAL);
>>>> + return -EINVAL;
>>>> + }
>>>
>>> Please make those error_report's - if anything fails migration
>>> I like to see it in the stderr log.
>>>
>> I know previously you wanted it to be error report. But in the migration
>> code the same error is always just put into trace (vmastate_load_state).
>> I just want to be consistent.
>
> I should fix those as well to add error_reports !
> It's very difficult when we get an error back from a customer with just
> a migration failure when the only thing we know is an -EINVAL, so I keep
> adding error_report calls.
>
OK. I will add err reports in vmstate.c.
Thanks,
Jianjun
> Dave
>
>> Thanks,
>> Jianjun
>>>> + 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);
>>>> + }
>>>> +
>>>> + 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;
>>>> + size_t entry = field->start;
>>>> + void *elm;
>>>> +
>>>> + trace_put_qtailq(vmsd->name, vmsd->version_id);
>>>> +
>>>> + QTAILQ_RAW_FOREACH(elm, pv, entry) {
>>>> + 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
>>>
>>> Dave
>>>
>>>>
>>> --
>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-14 16:43 ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
@ 2016-10-14 18:39 ` Paolo Bonzini
2016-10-14 21:10 ` Jianjun Duan
0 siblings, 1 reply; 18+ messages in thread
From: Paolo Bonzini @ 2016-10-14 18:39 UTC (permalink / raw)
To: Jianjun Duan
Cc: Dr. David Alan Gilbert, qemu-devel, qemu-ppc, dmitry,
peter maydell, kraxel, mst, david, veroniabahaa, quintela,
amit shah, mreitz, kwolf, rth, aurelien, leon alrae, blauwirbel,
mark cave-ayland, mdroth
> > Another possibility is a macro like
> >
> > #define field_at_offset(base, offset, type) \
> > ((type) (((char *) (base)) + (offset)))
> >
> > so that you can do
> >
> > *field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL;
> > *field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) =
> > *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET);
> > **field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm);
> > *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) =
> > field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET);
>
> The thing is that we don't know type at all. So only the offset values
> is used. This is intended for QTAILQ of any type.
Sure, my code is identical to yours---just with more macros for readability,
and a little more type-safe. Another possibility:
#define QTAILQ_RAW_NEXT(elm, entry) \
(*field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET))
#define QTAILQ_RAW_PREV(elm, entry) \
(*field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET))
#define QTAILQ_RAW_LAST(head) \
(*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET))
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);
Thanks,
Paolo
> Thanks,
> Jianjun
>
> >
> > or something like that (note that I've always used the same type for
> > next and last, by the way).
> >
> > Paolo
> >
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-14 18:39 ` Paolo Bonzini
@ 2016-10-14 21:10 ` Jianjun Duan
0 siblings, 0 replies; 18+ messages in thread
From: Jianjun Duan @ 2016-10-14 21:10 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Dr. David Alan Gilbert, qemu-devel, qemu-ppc, dmitry,
peter maydell, kraxel, mst, david, veroniabahaa, quintela,
amit shah, mreitz, kwolf, rth, aurelien, leon alrae, blauwirbel,
mark cave-ayland, mdroth
On 10/14/2016 11:39 AM, Paolo Bonzini wrote:
>
>>> Another possibility is a macro like
>>>
>>> #define field_at_offset(base, offset, type) \
>>> ((type) (((char *) (base)) + (offset)))
>>>
>>> so that you can do
>>>
>>> *field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL;
>>> *field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) =
>>> *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET);
>>> **field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm);
>>> *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) =
>>> field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET);
>>
>> The thing is that we don't know type at all. So only the offset values
>> is used. This is intended for QTAILQ of any type.
>
> Sure, my code is identical to yours---just with more macros for readability,
> and a little more type-safe. Another possibility:
>
> #define QTAILQ_RAW_NEXT(elm, entry) \
> (*field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET))
> #define QTAILQ_RAW_PREV(elm, entry) \
> (*field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET))
> #define QTAILQ_RAW_LAST(head) \
> (*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET))
>
> 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);
>
You are right. The readability can be approved.
Thanks,
Jianjun
> Thanks,
>
> Paolo
>
>> Thanks,
>> Jianjun
>>
>>>
>>> or something like that (note that I've always used the same type for
>>> next and last, by the way).
>>>
>>> Paolo
>>>
>>
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-14 17:18 ` Jianjun Duan
@ 2016-10-15 12:48 ` Halil Pasic
2016-10-17 16:49 ` Jianjun Duan
0 siblings, 1 reply; 18+ messages in thread
From: Halil Pasic @ 2016-10-15 12:48 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/14/2016 07:18 PM, Jianjun Duan wrote:
>>>>> +/*
>>>>> >>>> + * Offsets of layout of a tail queue head.
>>>>> >>>> + */
>>>>> >>>> +#define QTAILQ_FIRST_OFFSET 0
>>>>> >>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>>>>> >>>> +
>>>>> >>>> +/*
>>>>> >>>> + * Offsets of layout of a tail queue element.
>>>>> >>>> + */
>>>>> >>>> +#define QTAILQ_NEXT_OFFSET 0
>>>>> >>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
>>>>> >>>> +
>>>>> >>>> +/*
>>>>> >>>> + * Tail queue tranversal using pointer arithmetic.
>>>>> >>>> + */
>>>>> >>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \
>>>>> >>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \
>>>>> >>>> + (elm); \
>>>>> >>>> + (elm) = \
>>>>> >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)))
>>>>> >>>> +/*
>>>>> >>>> + * Tail queue insertion using pointer arithmetic.
>>>>> >>>> + */
>>>>> >>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
>>>>> >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
>>>>> >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
>>>>> >>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
>>>>> >>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
>>>>> >>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
>>>>> >>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
>>>>> >>>> +} while (/*CONSTCOND*/0)
>>>> >>>
>>>> >>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like:
>>>> >>>
>>>> >>> struct QTAILQDummy {
>>>> >>> char dummy;
>>>> >>> };
>>>> >>>
>>>> >>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
>>>> >>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
>>>> >>>
>>>> >>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \
>>>> >>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \
>>>> >>> (elm); \
>>>> >>> (elm) = \
>>>> >>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next
>>>> >>>
>>>> >>> and then I think elm gets declared as a struct QTAILQDummy.
>>>> >>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
>>>> >>>
>>>> >>> Would that work?
>>>> >>>
>>> >> It is intended for QTAILQ of any type. So type is not available.
>> >
>> > I think it might be possible to do it generally.
>> >
> If we have type, then we can use what is there already, and don't need a
> pointer arithmetic based approach. Inside put/get, we only get type
> layout info from vmsd, which is all about size and offset. This macro
> is used inside put/get, so I am not sure how we can directly use type
> here.
>
Dave's approach seems perfectly sane to me.
Jianjun have you actually tried to make it work before writing this?
Your argument does not work, because what you need from vmsd for
QTAILQ_RAW_FOREACH is only .start which corresponds to the entry
parameter of the macro. Dave still does the pointer arithmetic to
get a pointer (char*) to the anonymous struct holding tqe_next
and tqe_prev. Now since no arithmetic is done wit tqe_next
and tqe_prev, only dereferencing, their pointer type does not matter
all that much so we can do the and follow the pointer. Same goes
for the head.
Actually the QTAILQDummy is not necessary in my opinion since we can
probably (did not try it out myself) do:
Q_TAILQ_HEAD(QTAILQRawHead, void,)
typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry;
Cheers,
Halil
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-15 12:48 ` Halil Pasic
@ 2016-10-17 16:49 ` Jianjun Duan
2016-10-21 18:51 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 18+ messages in thread
From: Jianjun Duan @ 2016-10-17 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/15/2016 05:48 AM, Halil Pasic wrote:
>
>
> On 10/14/2016 07:18 PM, Jianjun Duan wrote:
>>>>>> +/*
>>>>>>>>>> + * Offsets of layout of a tail queue head.
>>>>>>>>>> + */
>>>>>>>>>> +#define QTAILQ_FIRST_OFFSET 0
>>>>>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>>>>>>>>>> +
>>>>>>>>>> +/*
>>>>>>>>>> + * Offsets of layout of a tail queue element.
>>>>>>>>>> + */
>>>>>>>>>> +#define QTAILQ_NEXT_OFFSET 0
>>>>>>>>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
>>>>>>>>>> +
>>>>>>>>>> +/*
>>>>>>>>>> + * Tail queue tranversal using pointer arithmetic.
>>>>>>>>>> + */
>>>>>>>>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \
>>>>>>>>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \
>>>>>>>>>> + (elm); \
>>>>>>>>>> + (elm) = \
>>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)))
>>>>>>>>>> +/*
>>>>>>>>>> + * Tail queue insertion using pointer arithmetic.
>>>>>>>>>> + */
>>>>>>>>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
>>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
>>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
>>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
>>>>>>>>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
>>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
>>>>>>>>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
>>>>>>>>>> +} while (/*CONSTCOND*/0)
>>>>>>>>
>>>>>>>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like:
>>>>>>>>
>>>>>>>> struct QTAILQDummy {
>>>>>>>> char dummy;
>>>>>>>> };
>>>>>>>>
>>>>>>>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
>>>>>>>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
>>>>>>>>
>>>>>>>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \
>>>>>>>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \
>>>>>>>> (elm); \
>>>>>>>> (elm) = \
>>>>>>>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next
>>>>>>>>
>>>>>>>> and then I think elm gets declared as a struct QTAILQDummy.
>>>>>>>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
>>>>>>>>
>>>>>>>> Would that work?
>>>>>>>>
>>>>>> It is intended for QTAILQ of any type. So type is not available.
>>>>
>>>> I think it might be possible to do it generally.
>>>>
>> If we have type, then we can use what is there already, and don't need a
>> pointer arithmetic based approach. Inside put/get, we only get type
>> layout info from vmsd, which is all about size and offset. This macro
>> is used inside put/get, so I am not sure how we can directly use type
>> here.
>>
>
> Dave's approach seems perfectly sane to me.
>
> Jianjun have you actually tried to make it work before writing this?
> Your argument does not work, because what you need from vmsd for
> QTAILQ_RAW_FOREACH is only .start which corresponds to the entry
> parameter of the macro. Dave still does the pointer arithmetic to
> get a pointer (char*) to the anonymous struct holding tqe_next
> and tqe_prev. Now since no arithmetic is done wit tqe_next
> and tqe_prev, only dereferencing, their pointer type does not matter
> all that much so we can do the and follow the pointer. Same goes
> for the head.
>
> Actually the QTAILQDummy is not necessary in my opinion since we can
> probably (did not try it out myself) do:
>
> Q_TAILQ_HEAD(QTAILQRawHead, void,)
> typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry;
>
Now I see. I thought Dave was using QTAILQDummy as an example.
Thanks,
Jianjun
> Cheers,
> Halil
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-17 16:49 ` Jianjun Duan
@ 2016-10-21 18:51 ` Dr. David Alan Gilbert
2016-10-21 19:16 ` Jianjun Duan
0 siblings, 1 reply; 18+ messages in thread
From: Dr. David Alan Gilbert @ 2016-10-21 18:51 UTC (permalink / raw)
To: Jianjun Duan
Cc: Halil Pasic, 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
* Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>
>
> On 10/15/2016 05:48 AM, Halil Pasic wrote:
> >
> >
> > On 10/14/2016 07:18 PM, Jianjun Duan wrote:
> >>>>>> +/*
> >>>>>>>>>> + * Offsets of layout of a tail queue head.
> >>>>>>>>>> + */
> >>>>>>>>>> +#define QTAILQ_FIRST_OFFSET 0
> >>>>>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
> >>>>>>>>>> +
> >>>>>>>>>> +/*
> >>>>>>>>>> + * Offsets of layout of a tail queue element.
> >>>>>>>>>> + */
> >>>>>>>>>> +#define QTAILQ_NEXT_OFFSET 0
> >>>>>>>>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
> >>>>>>>>>> +
> >>>>>>>>>> +/*
> >>>>>>>>>> + * Tail queue tranversal using pointer arithmetic.
> >>>>>>>>>> + */
> >>>>>>>>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \
> >>>>>>>>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \
> >>>>>>>>>> + (elm); \
> >>>>>>>>>> + (elm) = \
> >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)))
> >>>>>>>>>> +/*
> >>>>>>>>>> + * Tail queue insertion using pointer arithmetic.
> >>>>>>>>>> + */
> >>>>>>>>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
> >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
> >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
> >>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
> >>>>>>>>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
> >>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
> >>>>>>>>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
> >>>>>>>>>> +} while (/*CONSTCOND*/0)
> >>>>>>>>
> >>>>>>>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like:
> >>>>>>>>
> >>>>>>>> struct QTAILQDummy {
> >>>>>>>> char dummy;
> >>>>>>>> };
> >>>>>>>>
> >>>>>>>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
> >>>>>>>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
> >>>>>>>>
> >>>>>>>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \
> >>>>>>>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \
> >>>>>>>> (elm); \
> >>>>>>>> (elm) = \
> >>>>>>>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next
> >>>>>>>>
> >>>>>>>> and then I think elm gets declared as a struct QTAILQDummy.
> >>>>>>>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
> >>>>>>>>
> >>>>>>>> Would that work?
> >>>>>>>>
> >>>>>> It is intended for QTAILQ of any type. So type is not available.
> >>>>
> >>>> I think it might be possible to do it generally.
> >>>>
> >> If we have type, then we can use what is there already, and don't need a
> >> pointer arithmetic based approach. Inside put/get, we only get type
> >> layout info from vmsd, which is all about size and offset. This macro
> >> is used inside put/get, so I am not sure how we can directly use type
> >> here.
> >>
> >
> > Dave's approach seems perfectly sane to me.
> >
> > Jianjun have you actually tried to make it work before writing this?
> > Your argument does not work, because what you need from vmsd for
> > QTAILQ_RAW_FOREACH is only .start which corresponds to the entry
> > parameter of the macro. Dave still does the pointer arithmetic to
> > get a pointer (char*) to the anonymous struct holding tqe_next
> > and tqe_prev. Now since no arithmetic is done wit tqe_next
> > and tqe_prev, only dereferencing, their pointer type does not matter
> > all that much so we can do the and follow the pointer. Same goes
> > for the head.
> >
> > Actually the QTAILQDummy is not necessary in my opinion since we can
> > probably (did not try it out myself) do:
> >
> > Q_TAILQ_HEAD(QTAILQRawHead, void,)
> > typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry;
> >
>
> Now I see. I thought Dave was using QTAILQDummy as an example.
If you have a new version with either that or Paolo's suggestion,
it would be good; I'd like to see this set go in because I've
now got a small pile of patches built on top of it using
my WITH_TMP macro.
Dave
>
> Thanks,
> Jianjun
> > Cheers,
> > Halil
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ
2016-10-21 18:51 ` Dr. David Alan Gilbert
@ 2016-10-21 19:16 ` Jianjun Duan
0 siblings, 0 replies; 18+ messages in thread
From: Jianjun Duan @ 2016-10-21 19:16 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Halil Pasic, 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/21/2016 11:51 AM, Dr. David Alan Gilbert wrote:
> * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote:
>>
>>
>> On 10/15/2016 05:48 AM, Halil Pasic wrote:
>>>
>>>
>>> On 10/14/2016 07:18 PM, Jianjun Duan wrote:
>>>>>>>> +/*
>>>>>>>>>>>> + * Offsets of layout of a tail queue head.
>>>>>>>>>>>> + */
>>>>>>>>>>>> +#define QTAILQ_FIRST_OFFSET 0
>>>>>>>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>>>>>>>>>>>> +
>>>>>>>>>>>> +/*
>>>>>>>>>>>> + * Offsets of layout of a tail queue element.
>>>>>>>>>>>> + */
>>>>>>>>>>>> +#define QTAILQ_NEXT_OFFSET 0
>>>>>>>>>>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *))
>>>>>>>>>>>> +
>>>>>>>>>>>> +/*
>>>>>>>>>>>> + * Tail queue tranversal using pointer arithmetic.
>>>>>>>>>>>> + */
>>>>>>>>>>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \
>>>>>>>>>>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \
>>>>>>>>>>>> + (elm); \
>>>>>>>>>>>> + (elm) = \
>>>>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)))
>>>>>>>>>>>> +/*
>>>>>>>>>>>> + * Tail queue insertion using pointer arithmetic.
>>>>>>>>>>>> + */
>>>>>>>>>>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \
>>>>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \
>>>>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \
>>>>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \
>>>>>>>>>>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \
>>>>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \
>>>>>>>>>>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \
>>>>>>>>>>>> +} while (/*CONSTCOND*/0)
>>>>>>>>>>
>>>>>>>>>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like:
>>>>>>>>>>
>>>>>>>>>> struct QTAILQDummy {
>>>>>>>>>> char dummy;
>>>>>>>>>> };
>>>>>>>>>>
>>>>>>>>>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
>>>>>>>>>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
>>>>>>>>>>
>>>>>>>>>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \
>>>>>>>>>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \
>>>>>>>>>> (elm); \
>>>>>>>>>> (elm) = \
>>>>>>>>>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next
>>>>>>>>>>
>>>>>>>>>> and then I think elm gets declared as a struct QTAILQDummy.
>>>>>>>>>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
>>>>>>>>>>
>>>>>>>>>> Would that work?
>>>>>>>>>>
>>>>>>>> It is intended for QTAILQ of any type. So type is not available.
>>>>>>
>>>>>> I think it might be possible to do it generally.
>>>>>>
>>>> If we have type, then we can use what is there already, and don't need a
>>>> pointer arithmetic based approach. Inside put/get, we only get type
>>>> layout info from vmsd, which is all about size and offset. This macro
>>>> is used inside put/get, so I am not sure how we can directly use type
>>>> here.
>>>>
>>>
>>> Dave's approach seems perfectly sane to me.
>>>
>>> Jianjun have you actually tried to make it work before writing this?
>>> Your argument does not work, because what you need from vmsd for
>>> QTAILQ_RAW_FOREACH is only .start which corresponds to the entry
>>> parameter of the macro. Dave still does the pointer arithmetic to
>>> get a pointer (char*) to the anonymous struct holding tqe_next
>>> and tqe_prev. Now since no arithmetic is done wit tqe_next
>>> and tqe_prev, only dereferencing, their pointer type does not matter
>>> all that much so we can do the and follow the pointer. Same goes
>>> for the head.
>>>
>>> Actually the QTAILQDummy is not necessary in my opinion since we can
>>> probably (did not try it out myself) do:
>>>
>>> Q_TAILQ_HEAD(QTAILQRawHead, void,)
>>> typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry;
>>>
>>
>> Now I see. I thought Dave was using QTAILQDummy as an example.
>
> If you have a new version with either that or Paolo's suggestion,
> it would be good; I'd like to see this set go in because I've
> now got a small pile of patches built on top of it using
> my WITH_TMP macro.
>
I am working on it.
Thanks,
Jianjun
> Dave
>
>>
>> Thanks,
>> Jianjun
>>> Cheers,
>>> Halil
>>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-10-21 19:17 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-13 21:30 [Qemu-devel] [QEMU PATCH v6 0/2] migration: migrate QTAILQ Jianjun Duan
2016-10-13 21:30 ` [Qemu-devel] [QEMU PATCH v6 1/2] migration: extend VMStateInfo Jianjun Duan
2016-10-14 9:23 ` Dr. David Alan Gilbert
2016-10-14 16:58 ` Jianjun Duan
2016-10-13 21:30 ` [Qemu-devel] [QEMU PATCH v6 2/2] migration: migrate QTAILQ Jianjun Duan
2016-10-14 10:44 ` Dr. David Alan Gilbert
2016-10-14 11:07 ` Paolo Bonzini
2016-10-14 16:43 ` [Qemu-devel] [Qemu-ppc] " Jianjun Duan
2016-10-14 18:39 ` Paolo Bonzini
2016-10-14 21:10 ` Jianjun Duan
2016-10-14 16:56 ` Jianjun Duan
2016-10-14 17:04 ` Dr. David Alan Gilbert
2016-10-14 17:18 ` Jianjun Duan
2016-10-15 12:48 ` Halil Pasic
2016-10-17 16:49 ` Jianjun Duan
2016-10-21 18:51 ` Dr. David Alan Gilbert
2016-10-21 19:16 ` Jianjun Duan
2016-10-14 2:01 ` [Qemu-devel] [QEMU PATCH v6 0/2] " no-reply
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.