* Halil Pasic (pa...@linux.vnet.ibm.com) wrote: > > > On 05/08/2017 07:36 PM, Dr. David Alan Gilbert wrote: > > * Halil Pasic (pa...@linux.vnet.ibm.com) wrote: > >> Let us use the freshly introduced vmstate migration helpers instead of > >> saving/loading the config manually. > >> > >> To achieve this we need to hack the config_vector which is a common > >> VirtIO state in the middle of the VirtioCcwDevice state representation. > >> This somewhat ugly but we have no choice because the stream format needs > >> to be preserved. > >> > >> Still no changes in behavior, but the dead code we added previously is > >> finally awakening to life. > >> > >> Signed-off-by: Halil Pasic <pa...@linux.vnet.ibm.com> > >> --- > >> --- > >> hw/s390x/virtio-ccw.c | 116 > >> +++++++++++++++++++------------------------------- > >> 1 file changed, 44 insertions(+), 72 deletions(-) > >> > >> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > >> index c2badfe..8ab655c 100644 > >> --- a/hw/s390x/virtio-ccw.c > >> +++ b/hw/s390x/virtio-ccw.c > >> @@ -57,6 +57,32 @@ static int virtio_ccw_dev_post_load(void *opaque, int > >> version_id) > >> return 0; > >> } > >> > >> +static int get_config_vector(QEMUFile *f, void *pv, size_t size, > >> + VMStateField *field) > >> +{ > >> + VirtioCcwDevice *dev = pv; > >> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); > >> + > >> + qemu_get_be16s(f, &vdev->config_vector); > >> + return 0; > >> +} > >> + > >> +static int put_config_vector(QEMUFile *f, void *pv, size_t size, > >> + VMStateField *field, QJSON *vmdesc) > >> +{ > >> + VirtioCcwDevice *dev = pv; > >> + VirtIODevice *vdev = virtio_bus_get_device(&dev->bus); > >> + > >> + qemu_put_be16(f, vdev->config_vector); > >> + return 0; > >> +} > >> + > >> +const VMStateInfo vmstate_info_config_vector = { > >> + .name = "config_vector", > >> + .get = get_config_vector, > >> + .put = put_config_vector, > >> +}; > >> + > >> const VMStateDescription vmstate_virtio_ccw_dev = { > >> .name = "s390_virtio_ccw_dev", > >> .version_id = 1, > >> @@ -67,6 +93,14 @@ const VMStateDescription vmstate_virtio_ccw_dev = { > >> VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice), > >> VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice), > >> VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice), > >> + { > >> + /* > >> + * Ugly hack because VirtIODevice does not migrate itself. > >> + * This also makes legacy via vmstate_save_state possible. > >> + */ > >> + .name = "virtio/config_vector", > >> + .info = &vmstate_info_config_vector, > >> + }, > > > > I'm a bit confused - isn't just this bit the bit going > > through the vdev->load_config hook? > > > > Exactly! If you examine the part underscored with ============== in > virtio_ccw_(load|save)_config (that is what is behind vdev->load_config) > you should see that we use vmstate_virtio_ccw_dev (that is this > VMStateDescription to implement these function. > > Actually virtio_ccw_(load|save)_config won't do anything after patch 9 > and for new machines since the VirtIOCcwDevice is going to migrate > itself via DeviceClass.vmsd like other "normal" devices, and we fall > back to the VirtIO specific callbacks only in "legacy mode". > > I hope this helps!
Hmm, still confused! Why are you changing a Virtio device not to use the same migration oddities as all the other virtio devices? I was assuming we'd have to change the virtio core code to solve that for all virtio devices. Dave > Halil > > > > > >> VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, > >> vmstate_adapter_routes, \ > >> AdapterRoutes), > >> VMSTATE_UINT8(thinint_isc, VirtioCcwDevice), > >> @@ -1272,85 +1306,23 @@ static int virtio_ccw_load_queue(DeviceState *d, > >> int n, QEMUFile *f) > >> static void virtio_ccw_save_config(DeviceState *d, QEMUFile *f) > >> { > >> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); > >> - CcwDevice *ccw_dev = CCW_DEVICE(d); > >> - SubchDev *s = ccw_dev->sch; > >> - VirtIODevice *vdev = virtio_ccw_get_vdev(s); > >> > >> - subch_device_save(s, f); > >> - if (dev->indicators != NULL) { > >> - qemu_put_be32(f, dev->indicators->len); > >> - qemu_put_be64(f, dev->indicators->addr); > >> - } else { > >> - qemu_put_be32(f, 0); > >> - qemu_put_be64(f, 0UL); > >> - } > >> - if (dev->indicators2 != NULL) { > >> - qemu_put_be32(f, dev->indicators2->len); > >> - qemu_put_be64(f, dev->indicators2->addr); > >> - } else { > >> - qemu_put_be32(f, 0); > >> - qemu_put_be64(f, 0UL); > >> - } > >> - if (dev->summary_indicator != NULL) { > >> - qemu_put_be32(f, dev->summary_indicator->len); > >> - qemu_put_be64(f, dev->summary_indicator->addr); > >> - } else { > >> - qemu_put_be32(f, 0); > >> - qemu_put_be64(f, 0UL); > >> - } > >> - qemu_put_be16(f, vdev->config_vector); > >> - qemu_put_be64(f, dev->routes.adapter.ind_offset); > >> - qemu_put_byte(f, dev->thinint_isc); > >> - qemu_put_be32(f, dev->revision); > >> + /* > >> + * We save in legacy mode. The components take care of their own > >> + * compat. representation (based on css_migration_enabled). > >> + */ > >> + vmstate_save_state(f, &vmstate_virtio_ccw_dev, dev, NULL); > ==================================================================== > > >> } > >> > >> static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) > >> { > >> VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); > >> - CcwDevice *ccw_dev = CCW_DEVICE(d); > >> - CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev); > >> - SubchDev *s = ccw_dev->sch; > >> - VirtIODevice *vdev = virtio_ccw_get_vdev(s); > >> - int len; > >> - > >> - s->driver_data = dev; > >> - subch_device_load(s, f); > >> - /* Re-fill subch_id after loading the subchannel states.*/ > >> - if (ck->refill_ids) { > >> - ck->refill_ids(ccw_dev); > >> - } > >> - len = qemu_get_be32(f); > >> - if (len != 0) { > >> - dev->indicators = get_indicator(qemu_get_be64(f), len); > >> - } else { > >> - qemu_get_be64(f); > >> - dev->indicators = NULL; > >> - } > >> - len = qemu_get_be32(f); > >> - if (len != 0) { > >> - dev->indicators2 = get_indicator(qemu_get_be64(f), len); > >> - } else { > >> - qemu_get_be64(f); > >> - dev->indicators2 = NULL; > >> - } > >> - len = qemu_get_be32(f); > >> - if (len != 0) { > >> - dev->summary_indicator = get_indicator(qemu_get_be64(f), len); > >> - } else { > >> - qemu_get_be64(f); > >> - dev->summary_indicator = NULL; > >> - } > >> - qemu_get_be16s(f, &vdev->config_vector); > >> - dev->routes.adapter.ind_offset = qemu_get_be64(f); > >> - dev->thinint_isc = qemu_get_byte(f); > >> - dev->revision = qemu_get_be32(f); > >> - if (s->thinint_active) { > >> - dev->routes.adapter.adapter_id = css_get_adapter_id( > >> - CSS_IO_ADAPTER_VIRTIO, > >> - dev->thinint_isc); > >> - } > >> > >> - return 0; > >> + /* > >> + * We load in legacy mode. The components take take care to read > >> + * only stuff which is actually there (based on > >> css_migration_enabled). > >> + */ > >> + return vmstate_load_state(f, &vmstate_virtio_ccw_dev, dev, 1); > ======================================================================== > > >> } > >> > >> static void virtio_ccw_pre_plugged(DeviceState *d, Error **errp) > >> -- > >> 2.10.2 > >> > > -- > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK