Re: [Qemu-devel] [PATCH v2] virtio: ring sizes vs. reset

[Jason Wang]

On 09/10/2015 07:48 PM, Cornelia Huck wrote:
> We allow guests to change the size of the virtqueue rings by supplying
> a number of buffers that is different from the number of buffers the
> device was initialized with. Current code has some problems, however,
> since reset does not reset the ringsizes to the default values (as this
> is not saved anywhere).
>
> Let's extend the core code to keep track of the default ringsizes and
> migrate them once the guest changed them for any of the virtqueues
> for a device.
>
> Signed-off-by: Cornelia Huck <address@hidden>
> ---
>
> v1->v2: lose the changed ringsize tracking, calculate directly before
>         migrating

Reviewed-by: Jason Wang <address@hidden>

>
> ---
>  hw/virtio/virtio.c | 64 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 788b556..458fcbe 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -60,6 +60,7 @@ typedef struct VRingUsed
>  typedef struct VRing
>  {
>      unsigned int num;
> +    unsigned int num_default;
>      unsigned int align;
>      hwaddr desc;
>      hwaddr avail;
> @@ -633,6 +634,7 @@ void virtio_reset(void *opaque)
>          vdev->vq[i].signalled_used = 0;
>          vdev->vq[i].signalled_used_valid = false;
>          vdev->vq[i].notification = true;
> +        vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
>      }
>  }
>  
> @@ -964,6 +966,7 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int 
> queue_size,
>          abort();
>  
>      vdev->vq[i].vring.num = queue_size;
> +    vdev->vq[i].vring.num_default = queue_size;
>      vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN;
>      vdev->vq[i].handle_output = handle_output;
>  
> @@ -977,6 +980,7 @@ void virtio_del_queue(VirtIODevice *vdev, int n)
>      }
>  
>      vdev->vq[n].vring.num = 0;
> +    vdev->vq[n].vring.num_default = 0;
>  }
>  
>  void virtio_irq(VirtQueue *vq)
> @@ -1056,6 +1060,19 @@ static bool virtio_virtqueue_needed(void *opaque)
>      return virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
>  }
>  
> +static bool virtio_ringsize_needed(void *opaque)
> +{
> +    VirtIODevice *vdev = opaque;
> +    int i;
> +
> +    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> +        if (vdev->vq[i].vring.num != vdev->vq[i].vring.num_default) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
>  static void put_virtqueue_state(QEMUFile *f, void *pv, size_t size)
>  {
>      VirtIODevice *vdev = pv;
> @@ -1104,6 +1121,52 @@ static const VMStateDescription 
> vmstate_virtio_virtqueues = {
>      }
>  };
>  
> +static void put_ringsize_state(QEMUFile *f, void *pv, size_t size)
> +{
> +    VirtIODevice *vdev = pv;
> +    int i;
> +
> +    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> +        qemu_put_be32(f, vdev->vq[i].vring.num_default);
> +    }
> +}
> +
> +static int get_ringsize_state(QEMUFile *f, void *pv, size_t size)
> +{
> +    VirtIODevice *vdev = pv;
> +    int i;
> +
> +    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> +        vdev->vq[i].vring.num_default = qemu_get_be32(f);
> +    }
> +    return 0;
> +}
> +
> +static VMStateInfo vmstate_info_ringsize = {
> +    .name = "ringsize_state",
> +    .get = get_ringsize_state,
> +    .put = put_ringsize_state,
> +};
> +
> +static const VMStateDescription vmstate_virtio_ringsize = {
> +    .name = "virtio/ringsize",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = &virtio_ringsize_needed,
> +    .fields = (VMStateField[]) {
> +        {
> +            .name         = "ringsize",
> +            .version_id   = 0,
> +            .field_exists = NULL,
> +            .size         = 0,
> +            .info         = &vmstate_info_ringsize,
> +            .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 +1201,7 @@ static const VMStateDescription vmstate_virtio = {
>          &vmstate_virtio_device_endian,
>          &vmstate_virtio_64bit_features,
>          &vmstate_virtio_virtqueues,
> +        &vmstate_virtio_ringsize,
>          NULL
>      }
>  };