[Qemu-devel] Re: [PATCH 15/41] virtio: remove save/load_queue for virtio

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Dec 02, 2009 at 01:04:13PM +0100, Juan Quintela wrote:
> It was used only for PCI virtio devices, state that explicitely
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hw/virtio-pci.c |   24 ++++++------------------
>  hw/virtio.c     |   22 ++++++++++++++++------
>  hw/virtio.h     |    4 ++--
>  3 files changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 45d0adc..12f3961 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -151,28 +151,18 @@ const VMStateDescription vmstate_virtio_pci_config = {
>      }
>  };
> 
> -static void virtio_pci_save_queue(void * opaque, int n, QEMUFile *f)
> +bool virtio_pci_msix_present(void *opaque)
>  {
>      VirtIOPCIProxy *proxy = opaque;
> -    if (msix_present(&proxy->pci_dev))
> -        qemu_put_be16(f, virtio_queue_vector(proxy->vdev, n));
> -}
> 
> +    return msix_present(&proxy->pci_dev);
> +}
> 
> -static int virtio_pci_load_queue(void * opaque, int n, QEMUFile *f)
> +int virtio_pci_msix_vector_use(void *opaque, unsigned vector)
>  {
>      VirtIOPCIProxy *proxy = opaque;
> -    uint16_t vector;
> -    if (msix_present(&proxy->pci_dev)) {
> -        qemu_get_be16s(f, &vector);
> -    } else {
> -        vector = VIRTIO_NO_VECTOR;
> -    }
> -    virtio_queue_set_vector(proxy->vdev, n, vector);
> -    if (vector != VIRTIO_NO_VECTOR) {
> -        return msix_vector_use(&proxy->pci_dev, vector);
> -    }
> -    return 0;
> +
> +    return msix_vector_use(&proxy->pci_dev, vector);
>  }
> 
>  static void virtio_pci_reset(DeviceState *d)
> @@ -402,8 +392,6 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
> 
>  static const VirtIOBindings virtio_pci_bindings = {
>      .notify = virtio_pci_notify,
> -    .save_queue = virtio_pci_save_queue,
> -    .load_queue = virtio_pci_load_queue,
>  };
> 
>  static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
> diff --git a/hw/virtio.c b/hw/virtio.c
> index c136005..b565bf9 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -643,8 +643,10 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>          qemu_put_be32(f, vdev->vq[i].vring.num);
>          qemu_put_be64(f, vdev->vq[i].pa);
>          qemu_put_be16s(f, &vdev->vq[i].last_avail_idx);
> -        if (vdev->binding->save_queue)
> -            vdev->binding->save_queue(vdev->binding_opaque, i, f);
> +        if (vdev->type == VIRTIO_PCI &&
> +            virtio_pci_msix_present(vdev->binding_opaque)) {
> +            qemu_put_be16s(f, &vdev->vq[i].vector);
> +        }
>      }
>  }
> 

I think this will break build on systems without PCI
because virtio_pci.c is not linked in there.
Correct?

Making generic virtio.c depend on virtio_pci.c looks
wrong in any case. We have bindings to
resolve exactly this dependency problem.


> @@ -676,10 +678,18 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>          if (vdev->vq[i].pa) {
>              virtqueue_init(&vdev->vq[i]);
>          }
> -        if (vdev->binding->load_queue) {
> -            ret = vdev->binding->load_queue(vdev->binding_opaque, i, f);
> -            if (ret)
> -                return ret;
> +        if (vdev->type == VIRTIO_PCI) {
> +            if (virtio_pci_msix_present(vdev->binding_opaque)) {
> +                qemu_get_be16s(f, &vdev->vq[i].vector);
> +            } else {
> +                vdev->vq[i].vector = VIRTIO_NO_VECTOR;
> +            }
> +            if (vdev->vq[i].vector != VIRTIO_NO_VECTOR) {
> +                ret = virtio_pci_msix_vector_use(vdev->binding_opaque,
> +                                                 vdev->vq[i].vector);
> +                if (ret)
> +                    return ret;
> +            }
>          }
>      }
> 
> diff --git a/hw/virtio.h b/hw/virtio.h
> index 9d2e2cc..91a6c10 100644
> --- a/hw/virtio.h
> +++ b/hw/virtio.h
> @@ -78,8 +78,6 @@ typedef struct VirtQueueElement
> 
>  typedef struct {
>      void (*notify)(void * opaque, uint16_t vector);
> -    void (*save_queue)(void * opaque, int n, QEMUFile *f);
> -    int (*load_queue)(void * opaque, int n, QEMUFile *f);
>  } VirtIOBindings;
> 
>  #define VIRTIO_PCI_QUEUE_MAX 16
> @@ -176,5 +174,7 @@ void virtio_net_exit(VirtIODevice *vdev);
> 
>  /* virtio-pci. */
>  extern const VMStateDescription vmstate_virtio_pci_config;
> +bool virtio_pci_msix_present(void *opaque);
> +int virtio_pci_msix_vector_use(void *opaque, unsigned vector);
> 
>  #endif
> -- 
> 1.6.5.2