Message ID | 1441164325-14417-5-git-send-email-jasowang@redhat.com |
---|---|
State | New |
Headers | show |
On Wed, 2 Sep 2015 11:25:21 +0800 Jason Wang <jasowang@redhat.com> wrote: > We don't migrate the followings fields for virtio-pci: > > uint32_t dfselect; > uint32_t gfselect; > uint32_t guest_features[2]; > struct { > uint16_t num; > bool enabled; > uint32_t desc[2]; > uint32_t avail[2]; > uint32_t used[2]; > } vqs[VIRTIO_QUEUE_MAX]; > > This will confuse driver if migrating during initialization. Solves > this issue by: > > - introduce transport specific callbacks to load and store extra > virtqueue states. > - add a new subsection for virtio to migrate transport specific modern > device state. > - implement pci specific callbacks. > - add a new property for virtio-pci for whether or not to migrate > extra state. > - compat the migration for 2.4 and elder machine types > > Cc: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/virtio/virtio-pci.c | 129 +++++++++++++++++++++++++++++++++++++++++ > hw/virtio/virtio-pci.h | 20 ++++--- > hw/virtio/virtio.c | 57 ++++++++++++++++++ > include/hw/compat.h | 6 +- > include/hw/virtio/virtio-bus.h | 3 + > 5 files changed, 207 insertions(+), 8 deletions(-) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index c024161..a96890b 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -86,6 +86,129 @@ static void virtio_pci_save_config(DeviceState *d, QEMUFile *f) > qemu_put_be16(f, vdev->config_vector); > } > > +static void virtio_pci_load_modern_queue_state(VirtIOPCIQueue *vq, > + QEMUFile *f) > +{ > + vq->num = qemu_get_be16(f); > + vq->enabled = qemu_get_be16(f); > + vq->desc[0] = qemu_get_be32(f); > + vq->desc[1] = qemu_get_be32(f); > + vq->avail[0] = qemu_get_be32(f); > + vq->avail[1] = qemu_get_be32(f); > + vq->used[0] = qemu_get_be32(f); > + vq->used[1] = qemu_get_be32(f); > +} > + > +static bool virtio_pci_has_extra_state(DeviceState *d) > +{ > + VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); > + > + return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA; Hm, couldn't you already check whether this is a modern device here? You don't need to save state for a legacy device, do you? > +} > + (...) > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 788b556..dcaf022 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -1056,6 +1056,16 @@ static bool virtio_virtqueue_needed(void *opaque) > return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1); > } > > +static bool virtio_extra_state_needed(void *opaque) > +{ > + VirtIODevice *vdev = opaque; > + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); > + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > + > + return k->has_extra_state && > + k->has_extra_state(qbus->parent); > +} > + > static void put_virtqueue_state(QEMUFile *f, void *pv, size_t size) > { > VirtIODevice *vdev = pv; > @@ -1104,6 +1114,52 @@ static const VMStateDescription vmstate_virtio_virtqueues = { > } > }; > > +static int get_extra_state(QEMUFile *f, void *pv, size_t size) > +{ > + VirtIODevice *vdev = pv; > + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); > + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > + int ret = 0; > + > + ret = k->load_extra_state(qbus->parent, f); Should we check for ->load_extra_state() and return failure if it does not exist? > + > + return ret; > +} > + > +static void put_extra_state(QEMUFile *f, void *pv, size_t size) > +{ > + VirtIODevice *vdev = pv; > + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); > + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > + > + k->save_extra_state(qbus->parent, f); Maybe add an assert(k->save_extra_state) here, as this means the transport code is buggy? > +} > + > +static VMStateInfo vmstate_info_extra_state = { > + .name = "virtqueue_extra_state", > + .get = get_extra_state, > + .put = put_extra_state, > +}; > + > +static const VMStateDescription vmstate_virtio_modern_state = { Should probably be named "extra_state". > + .name = "virtio/extra_state", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = &virtio_extra_state_needed, > + .fields = (VMStateField[]) { > + { > + .name = "modern_state", Also here. > + .version_id = 0, > + .field_exists = NULL, > + .size = 0, > + .info = &vmstate_info_extra_state, > + .flags = VMS_SINGLE, > + .offset = 0, > + }, > + VMSTATE_END_OF_LIST() > + } > +}; > + > static const VMStateDescription vmstate_virtio_device_endian = { > .name = "virtio/device_endian", > .version_id = 1,
On 09/02/2015 07:06 PM, Cornelia Huck wrote: > On Wed, 2 Sep 2015 11:25:21 +0800 > Jason Wang <jasowang@redhat.com> wrote: > >> We don't migrate the followings fields for virtio-pci: >> >> uint32_t dfselect; >> uint32_t gfselect; >> uint32_t guest_features[2]; >> struct { >> uint16_t num; >> bool enabled; >> uint32_t desc[2]; >> uint32_t avail[2]; >> uint32_t used[2]; >> } vqs[VIRTIO_QUEUE_MAX]; >> >> This will confuse driver if migrating during initialization. Solves >> this issue by: >> >> - introduce transport specific callbacks to load and store extra >> virtqueue states. >> - add a new subsection for virtio to migrate transport specific modern >> device state. >> - implement pci specific callbacks. >> - add a new property for virtio-pci for whether or not to migrate >> extra state. >> - compat the migration for 2.4 and elder machine types >> >> Cc: Michael S. Tsirkin <mst@redhat.com> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> hw/virtio/virtio-pci.c | 129 +++++++++++++++++++++++++++++++++++++++++ >> hw/virtio/virtio-pci.h | 20 ++++--- >> hw/virtio/virtio.c | 57 ++++++++++++++++++ >> include/hw/compat.h | 6 +- >> include/hw/virtio/virtio-bus.h | 3 + >> 5 files changed, 207 insertions(+), 8 deletions(-) >> >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index c024161..a96890b 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -86,6 +86,129 @@ static void virtio_pci_save_config(DeviceState *d, QEMUFile *f) >> qemu_put_be16(f, vdev->config_vector); >> } >> >> +static void virtio_pci_load_modern_queue_state(VirtIOPCIQueue *vq, >> + QEMUFile *f) >> +{ >> + vq->num = qemu_get_be16(f); >> + vq->enabled = qemu_get_be16(f); >> + vq->desc[0] = qemu_get_be32(f); >> + vq->desc[1] = qemu_get_be32(f); >> + vq->avail[0] = qemu_get_be32(f); >> + vq->avail[1] = qemu_get_be32(f); >> + vq->used[0] = qemu_get_be32(f); >> + vq->used[1] = qemu_get_be32(f); >> +} >> + >> +static bool virtio_pci_has_extra_state(DeviceState *d) >> +{ >> + VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); >> + >> + return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA; > Hm, couldn't you already check whether this is a modern device here? > You don't need to save state for a legacy device, do you? It was prepared for future. If we have something more that needs to be migrated specifically for virtio-pci. We could just add a new subsection for vmstate_virtio_pci without touching the core again. So we check the modern in its subsection instead of here. > >> +} >> + > (...) > >> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c >> index 788b556..dcaf022 100644 >> --- a/hw/virtio/virtio.c >> +++ b/hw/virtio/virtio.c >> @@ -1056,6 +1056,16 @@ static bool virtio_virtqueue_needed(void *opaque) >> return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1); >> } >> >> +static bool virtio_extra_state_needed(void *opaque) >> +{ >> + VirtIODevice *vdev = opaque; >> + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); >> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); >> + >> + return k->has_extra_state && >> + k->has_extra_state(qbus->parent); >> +} >> + >> static void put_virtqueue_state(QEMUFile *f, void *pv, size_t size) >> { >> VirtIODevice *vdev = pv; >> @@ -1104,6 +1114,52 @@ static const VMStateDescription vmstate_virtio_virtqueues = { >> } >> }; >> >> +static int get_extra_state(QEMUFile *f, void *pv, size_t size) >> +{ >> + VirtIODevice *vdev = pv; >> + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); >> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); >> + int ret = 0; >> + >> + ret = k->load_extra_state(qbus->parent, f); > Should we check for ->load_extra_state() and return failure if it does > not exist? I think checking the existence of has_extra_state() in virtio_extra_state_needed() is enough for this? Or is there anything I miss? > >> + >> + return ret; >> +} >> + >> +static void put_extra_state(QEMUFile *f, void *pv, size_t size) >> +{ >> + VirtIODevice *vdev = pv; >> + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); >> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); >> + >> + k->save_extra_state(qbus->parent, f); > Maybe add an assert(k->save_extra_state) here, as this means the > transport code is buggy? > >> +} >> + >> +static VMStateInfo vmstate_info_extra_state = { >> + .name = "virtqueue_extra_state", >> + .get = get_extra_state, >> + .put = put_extra_state, >> +}; >> + >> +static const VMStateDescription vmstate_virtio_modern_state = { > Should probably be named "extra_state". Right. > >> + .name = "virtio/extra_state", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .needed = &virtio_extra_state_needed, >> + .fields = (VMStateField[]) { >> + { >> + .name = "modern_state", > Also here. Will fix this. Thanks. >> + .version_id = 0, >> + .field_exists = NULL, >> + .size = 0, >> + .info = &vmstate_info_extra_state, >> + .flags = VMS_SINGLE, >> + .offset = 0, >> + }, >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> static const VMStateDescription vmstate_virtio_device_endian = { >> .name = "virtio/device_endian", >> .version_id = 1, >
On Mon, 7 Sep 2015 15:39:59 +0800 Jason Wang <jasowang@redhat.com> wrote: > On 09/02/2015 07:06 PM, Cornelia Huck wrote: > > On Wed, 2 Sep 2015 11:25:21 +0800 > > Jason Wang <jasowang@redhat.com> wrote: > >> +static int get_extra_state(QEMUFile *f, void *pv, size_t size) > >> +{ > >> + VirtIODevice *vdev = pv; > >> + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); > >> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > >> + int ret = 0; > >> + > >> + ret = k->load_extra_state(qbus->parent, f); > > Should we check for ->load_extra_state() and return failure if it does > > not exist? > > I think checking the existence of has_extra_state() in > virtio_extra_state_needed() is enough for this? Or is there anything I miss? The has_extra_state() callback is called by the sender, not by the receiver. If the other side sent the extra state but we can't handle it, I think it would be better to fail the migration than to crash. Still broken code, but probably easier to debug :)
On 09/07/2015 04:21 PM, Cornelia Huck wrote: > On Mon, 7 Sep 2015 15:39:59 +0800 > Jason Wang <jasowang@redhat.com> wrote: > >> > On 09/02/2015 07:06 PM, Cornelia Huck wrote: >>> > > On Wed, 2 Sep 2015 11:25:21 +0800 >>> > > Jason Wang <jasowang@redhat.com> wrote: >>>> > >> +static int get_extra_state(QEMUFile *f, void *pv, size_t size) >>>> > >> +{ >>>> > >> + VirtIODevice *vdev = pv; >>>> > >> + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); >>>> > >> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); >>>> > >> + int ret = 0; >>>> > >> + >>>> > >> + ret = k->load_extra_state(qbus->parent, f); >>> > > Should we check for ->load_extra_state() and return failure if it does >>> > > not exist? >> > >> > I think checking the existence of has_extra_state() in >> > virtio_extra_state_needed() is enough for this? Or is there anything I miss? > The has_extra_state() callback is called by the sender, not by the > receiver. > > If the other side sent the extra state but we can't handle it, I think > it would be better to fail the migration than to crash. Still broken > code, but probably easier to debug :) > I see so it only happen when transport provides save_extra_state() but not get_extra_sate(). I'm ok to add the check in next version . Thanks
On Tue, Sep 08, 2015 at 03:27:59PM +0800, Jason Wang wrote: > > > On 09/07/2015 04:21 PM, Cornelia Huck wrote: > > On Mon, 7 Sep 2015 15:39:59 +0800 > > Jason Wang <jasowang@redhat.com> wrote: > > > >> > On 09/02/2015 07:06 PM, Cornelia Huck wrote: > >>> > > On Wed, 2 Sep 2015 11:25:21 +0800 > >>> > > Jason Wang <jasowang@redhat.com> wrote: > >>>> > >> +static int get_extra_state(QEMUFile *f, void *pv, size_t size) > >>>> > >> +{ > >>>> > >> + VirtIODevice *vdev = pv; > >>>> > >> + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); > >>>> > >> + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > >>>> > >> + int ret = 0; > >>>> > >> + > >>>> > >> + ret = k->load_extra_state(qbus->parent, f); > >>> > > Should we check for ->load_extra_state() and return failure if it does > >>> > > not exist? > >> > > >> > I think checking the existence of has_extra_state() in > >> > virtio_extra_state_needed() is enough for this? Or is there anything I miss? > > The has_extra_state() callback is called by the sender, not by the > > receiver. > > > > If the other side sent the extra state but we can't handle it, I think > > it would be better to fail the migration than to crash. Still broken > > code, but probably easier to debug :) > > > > I see so it only happen when transport provides save_extra_state() but > not get_extra_sate(). I'm ok to add the check in next version . > > Thanks This seems minor enough. I think I'll merge this as is, we can tweak things using patches on top.
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index c024161..a96890b 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -86,6 +86,129 @@ static void virtio_pci_save_config(DeviceState *d, QEMUFile *f) qemu_put_be16(f, vdev->config_vector); } +static void virtio_pci_load_modern_queue_state(VirtIOPCIQueue *vq, + QEMUFile *f) +{ + vq->num = qemu_get_be16(f); + vq->enabled = qemu_get_be16(f); + vq->desc[0] = qemu_get_be32(f); + vq->desc[1] = qemu_get_be32(f); + vq->avail[0] = qemu_get_be32(f); + vq->avail[1] = qemu_get_be32(f); + vq->used[0] = qemu_get_be32(f); + vq->used[1] = qemu_get_be32(f); +} + +static bool virtio_pci_has_extra_state(DeviceState *d) +{ + VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); + + return proxy->flags & VIRTIO_PCI_FLAG_MIGRATE_EXTRA; +} + +static int get_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size) +{ + VirtIOPCIProxy *proxy = pv; + int i; + + proxy->dfselect = qemu_get_be32(f); + proxy->gfselect = qemu_get_be32(f); + proxy->guest_features[0] = qemu_get_be32(f); + proxy->guest_features[1] = qemu_get_be32(f); + for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { + virtio_pci_load_modern_queue_state(&proxy->vqs[i], f); + } + + return 0; +} + +static void virtio_pci_save_modern_queue_state(VirtIOPCIQueue *vq, + QEMUFile *f) +{ + qemu_put_be16(f, vq->num); + qemu_put_be16(f, vq->enabled); + qemu_put_be32(f, vq->desc[0]); + qemu_put_be32(f, vq->desc[1]); + qemu_put_be32(f, vq->avail[0]); + qemu_put_be32(f, vq->avail[1]); + qemu_put_be32(f, vq->used[0]); + qemu_put_be32(f, vq->used[1]); +} + +static void put_virtio_pci_modern_state(QEMUFile *f, void *pv, size_t size) +{ + VirtIOPCIProxy *proxy = pv; + int i; + + qemu_put_be32(f, proxy->dfselect); + qemu_put_be32(f, proxy->gfselect); + qemu_put_be32(f, proxy->guest_features[0]); + qemu_put_be32(f, proxy->guest_features[1]); + for (i = 0; i < VIRTIO_QUEUE_MAX; i++) { + virtio_pci_save_modern_queue_state(&proxy->vqs[i], f); + } +} + +static VMStateInfo vmstate_info_virtio_pci_modern_state = { + .name = "virtqueue_state", + .get = get_virtio_pci_modern_state, + .put = put_virtio_pci_modern_state, +}; + +static bool virtio_pci_modern_state_needed(void *opaque) +{ + VirtIOPCIProxy *proxy = opaque; + + return !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN); +} + +static const VMStateDescription vmstate_virtio_pci_modern_state = { + .name = "virtio_pci/modern_state", + .version_id = 1, + .minimum_version_id = 1, + .needed = &virtio_pci_modern_state_needed, + .fields = (VMStateField[]) { + { + .name = "modern_state", + .version_id = 0, + .field_exists = NULL, + .size = 0, + .info = &vmstate_info_virtio_pci_modern_state, + .flags = VMS_SINGLE, + .offset = 0, + }, + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription vmstate_virtio_pci = { + .name = "virtio_pci", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField[]) { + VMSTATE_END_OF_LIST() + }, + .subsections = (const VMStateDescription*[]) { + &vmstate_virtio_pci_modern_state, + NULL + } +}; + +static void virtio_pci_save_extra_state(DeviceState *d, QEMUFile *f) +{ + VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); + + vmstate_save_state(f, &vmstate_virtio_pci, proxy, NULL); +} + +static int virtio_pci_load_extra_state(DeviceState *d, QEMUFile *f) +{ + VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); + + return vmstate_load_state(f, &vmstate_virtio_pci, proxy, 1); +} + static void virtio_pci_save_queue(DeviceState *d, int n, QEMUFile *f) { VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); @@ -133,6 +256,7 @@ static int virtio_pci_load_queue(DeviceState *d, int n, QEMUFile *f) if (vector != VIRTIO_NO_VECTOR) { return msix_vector_use(&proxy->pci_dev, vector); } + return 0; } @@ -1618,6 +1742,8 @@ static Property virtio_pci_properties[] = { VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT, false), DEFINE_PROP_BIT("disable-modern", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, true), + DEFINE_PROP_BIT("migrate-extra", VirtIOPCIProxy, flags, + VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT, true), DEFINE_PROP_END_OF_LIST(), }; @@ -2211,6 +2337,9 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data) k->load_config = virtio_pci_load_config; k->save_queue = virtio_pci_save_queue; k->load_queue = virtio_pci_load_queue; + k->save_extra_state = virtio_pci_save_extra_state; + k->load_extra_state = virtio_pci_load_extra_state; + k->has_extra_state = virtio_pci_has_extra_state; k->query_guest_notifiers = virtio_pci_query_guest_notifiers; k->set_host_notifier = virtio_pci_set_host_notifier; k->set_guest_notifiers = virtio_pci_set_guest_notifiers; diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index b6c442f..d890009 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -75,6 +75,10 @@ typedef struct VirtioBusClass VirtioPCIBusClass; #define VIRTIO_PCI_FLAG_DISABLE_LEGACY (1 << VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT) #define VIRTIO_PCI_FLAG_DISABLE_MODERN (1 << VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT) +/* migrate extra state */ +#define VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT 4 +#define VIRTIO_PCI_FLAG_MIGRATE_EXTRA (1 << VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT) + typedef struct { MSIMessage msg; int virq; @@ -104,6 +108,14 @@ typedef struct VirtIOPCIRegion { uint32_t type; } VirtIOPCIRegion; +typedef struct VirtIOPCIQueue { + uint16_t num; + bool enabled; + uint32_t desc[2]; + uint32_t avail[2]; + uint32_t used[2]; +} VirtIOPCIQueue; + struct VirtIOPCIProxy { PCIDevice pci_dev; MemoryRegion bar; @@ -124,13 +136,7 @@ struct VirtIOPCIProxy { uint32_t dfselect; uint32_t gfselect; uint32_t guest_features[2]; - struct { - uint16_t num; - bool enabled; - uint32_t desc[2]; - uint32_t avail[2]; - uint32_t used[2]; - } vqs[VIRTIO_QUEUE_MAX]; + VirtIOPCIQueue vqs[VIRTIO_QUEUE_MAX]; bool ioeventfd_disabled; bool ioeventfd_started; diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 788b556..dcaf022 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -1056,6 +1056,16 @@ static bool virtio_virtqueue_needed(void *opaque) return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1); } +static bool virtio_extra_state_needed(void *opaque) +{ + VirtIODevice *vdev = opaque; + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); + + return k->has_extra_state && + k->has_extra_state(qbus->parent); +} + static void put_virtqueue_state(QEMUFile *f, void *pv, size_t size) { VirtIODevice *vdev = pv; @@ -1104,6 +1114,52 @@ static const VMStateDescription vmstate_virtio_virtqueues = { } }; +static int get_extra_state(QEMUFile *f, void *pv, size_t size) +{ + VirtIODevice *vdev = pv; + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); + int ret = 0; + + ret = k->load_extra_state(qbus->parent, f); + + return ret; +} + +static void put_extra_state(QEMUFile *f, void *pv, size_t size) +{ + VirtIODevice *vdev = pv; + BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); + VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); + + k->save_extra_state(qbus->parent, f); +} + +static VMStateInfo vmstate_info_extra_state = { + .name = "virtqueue_extra_state", + .get = get_extra_state, + .put = put_extra_state, +}; + +static const VMStateDescription vmstate_virtio_modern_state = { + .name = "virtio/extra_state", + .version_id = 1, + .minimum_version_id = 1, + .needed = &virtio_extra_state_needed, + .fields = (VMStateField[]) { + { + .name = "modern_state", + .version_id = 0, + .field_exists = NULL, + .size = 0, + .info = &vmstate_info_extra_state, + .flags = VMS_SINGLE, + .offset = 0, + }, + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_virtio_device_endian = { .name = "virtio/device_endian", .version_id = 1, @@ -1138,6 +1194,7 @@ static const VMStateDescription vmstate_virtio = { &vmstate_virtio_device_endian, &vmstate_virtio_64bit_features, &vmstate_virtio_virtqueues, + &vmstate_virtio_modern_state, NULL } }; diff --git a/include/hw/compat.h b/include/hw/compat.h index 095de5d..47faab0 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -2,7 +2,11 @@ #define HW_COMPAT_H #define HW_COMPAT_2_4 \ - /* empty */ + {\ + .driver = "virtio-pci",\ + .property = "migrate-extra",\ + .value = "off",\ + }, #define HW_COMPAT_2_3 \ {\ diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h index 8811415..6c3d4cb 100644 --- a/include/hw/virtio/virtio-bus.h +++ b/include/hw/virtio/virtio-bus.h @@ -44,9 +44,12 @@ typedef struct VirtioBusClass { void (*notify)(DeviceState *d, uint16_t vector); void (*save_config)(DeviceState *d, QEMUFile *f); void (*save_queue)(DeviceState *d, int n, QEMUFile *f); + void (*save_extra_state)(DeviceState *d, QEMUFile *f); int (*load_config)(DeviceState *d, QEMUFile *f); int (*load_queue)(DeviceState *d, int n, QEMUFile *f); int (*load_done)(DeviceState *d, QEMUFile *f); + int (*load_extra_state)(DeviceState *d, QEMUFile *f); + bool (*has_extra_state)(DeviceState *d); bool (*query_guest_notifiers)(DeviceState *d); int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign); int (*set_host_notifier)(DeviceState *d, int n, bool assigned);
We don't migrate the followings fields for virtio-pci: uint32_t dfselect; uint32_t gfselect; uint32_t guest_features[2]; struct { uint16_t num; bool enabled; uint32_t desc[2]; uint32_t avail[2]; uint32_t used[2]; } vqs[VIRTIO_QUEUE_MAX]; This will confuse driver if migrating during initialization. Solves this issue by: - introduce transport specific callbacks to load and store extra virtqueue states. - add a new subsection for virtio to migrate transport specific modern device state. - implement pci specific callbacks. - add a new property for virtio-pci for whether or not to migrate extra state. - compat the migration for 2.4 and elder machine types Cc: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/virtio/virtio-pci.c | 129 +++++++++++++++++++++++++++++++++++++++++ hw/virtio/virtio-pci.h | 20 ++++--- hw/virtio/virtio.c | 57 ++++++++++++++++++ include/hw/compat.h | 6 +- include/hw/virtio/virtio-bus.h | 3 + 5 files changed, 207 insertions(+), 8 deletions(-)