On Wed, 2 Sep 2015 11:25:21 +0800
Jason Wang <jasow...@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 <m...@redhat.com>
> Signed-off-by: Jason Wang <jasow...@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,
