diff mbox

[v2,2/6] virtio-balloon: make stats virtqueue length 1

Message ID 1471613966-7267-3-git-send-email-rkagan@virtuozzo.com
State New
Headers show

Commit Message

Roman Kagan Aug. 19, 2016, 1:39 p.m. UTC
The protocol for virtio-balloon stats virtqueue doesn't allow more than
one element in the virtqueue.

So, instead of trying to compensate for guest misbehavior if it sends
new data before the slot has been released by the host, just define the
stats virtqueue length to 1 initially and rely on the generic virtio
code to handle overflows.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Ladi Prosek <lprosek@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
---
v1 > v2:
 - added assert in virtio_balloon_receive_stats

 hw/virtio/virtio-balloon.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

Michael S. Tsirkin Aug. 22, 2016, 1:57 a.m. UTC | #1
On Fri, Aug 19, 2016 at 04:39:21PM +0300, Roman Kagan wrote:
> The protocol for virtio-balloon stats virtqueue doesn't allow more than
> one element in the virtqueue.
> 
> So, instead of trying to compensate for guest misbehavior if it sends
> new data before the slot has been released by the host, just define the
> stats virtqueue length to 1 initially and rely on the generic virtio
> code to handle overflows.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Ladi Prosek <lprosek@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>


This would need a bunch of compat handlers to
avoid changes for old machine types.

> ---
> v1 > v2:
>  - added assert in virtio_balloon_receive_stats
> 
>  hw/virtio/virtio-balloon.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 5af429a..fb8784e 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -262,12 +262,8 @@ static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
>          goto out;
>      }
>  
> -    if (s->stats_vq_elem != NULL) {
> -        /* This should never happen if the driver follows the spec. */
> -        virtqueue_push(vq, s->stats_vq_elem, 0);
> -        virtio_notify(vdev, vq);
> -        g_free(s->stats_vq_elem);
> -    }
> +    /* enforced by stats virtqueue depth being 1 */
> +    assert(!s->stats_vq_elem);
>  
>      s->stats_vq_elem = elem;
>  
> @@ -443,7 +439,7 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
>  
>      s->ivq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
>      s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
> -    s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
> +    s->svq = virtio_add_queue(vdev, 1, virtio_balloon_receive_stats);
>  
>      reset_stats(s);
>  }
> -- 
> 2.7.4
diff mbox

Patch

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 5af429a..fb8784e 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -262,12 +262,8 @@  static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
         goto out;
     }
 
-    if (s->stats_vq_elem != NULL) {
-        /* This should never happen if the driver follows the spec. */
-        virtqueue_push(vq, s->stats_vq_elem, 0);
-        virtio_notify(vdev, vq);
-        g_free(s->stats_vq_elem);
-    }
+    /* enforced by stats virtqueue depth being 1 */
+    assert(!s->stats_vq_elem);
 
     s->stats_vq_elem = elem;
 
@@ -443,7 +439,7 @@  static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
 
     s->ivq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
     s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
-    s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
+    s->svq = virtio_add_queue(vdev, 1, virtio_balloon_receive_stats);
 
     reset_stats(s);
 }