[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config mi
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration |
Date: |
Mon, 8 May 2017 18:59:07 +0100 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
* Halil Pasic (address@hidden) wrote:
>
>
> On 05/08/2017 07:36 PM, Dr. David Alan Gilbert wrote:
> > * Halil Pasic (address@hidden) 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 <address@hidden>
> >> ---
> >> ---
> >> 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 / address@hidden / Manchester, UK
> >
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, (continued)
- [Qemu-devel] [PATCH 05/10] virtio-ccw: add vmstate entities for VirtioCcwDevice, Halil Pasic, 2017/05/05
- [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Halil Pasic, 2017/05/05
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Dr. David Alan Gilbert, 2017/05/08
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Halil Pasic, 2017/05/08
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration,
Dr. David Alan Gilbert <=
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Halil Pasic, 2017/05/08
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Dr. David Alan Gilbert, 2017/05/08
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Halil Pasic, 2017/05/10
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Dr. David Alan Gilbert, 2017/05/15
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Halil Pasic, 2017/05/16
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Dr. David Alan Gilbert, 2017/05/19
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Halil Pasic, 2017/05/19
- Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Dr. David Alan Gilbert, 2017/05/19
[Qemu-devel] [PATCH 07/10] s390x/css: remove unused subch_dev_(load|save), Halil Pasic, 2017/05/05
[Qemu-devel] [PATCH 08/10] s390x/css: add ORB to SubchDev, Halil Pasic, 2017/05/05