[3/3] virtio-serial: add missing virtio_detach_element() call

Commit Message

Stefan Hajnoczi Sept. 19, 2016, 1:28 p.m. UTC

Ports enter a "throttled" state when writing to the chardev would block.
The current output VirtQueueElement is kept around until the chardev
becomes writable again.

There are several places in the virtio-serial lifecycle where the
VirtQueueElement should be thrown away.  For example, if the virtio
device is reset then virtqueue elements are no longer valid.

This patch adds the discard_throttle_data() function to unmap the
scatter-gather list and decrement vq->inuse.  This ensures that the
VirtQueueElement is freed properly.

Cc: [e-mail]
Signed-off-by: Stefan Hajnoczi <[e-mail]>
---
 hw/char/virtio-serial-bus.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Ladi Prosek Sept. 30, 2016, 10:08 a.m. UTC | #1

On Mon, Sep 19, 2016 at 3:28 PM, Stefan Hajnoczi <[e-mail]> wrote:
> Ports enter a "throttled" state when writing to the chardev would block.
> The current output VirtQueueElement is kept around until the chardev
> becomes writable again.
>
> There are several places in the virtio-serial lifecycle where the
> VirtQueueElement should be thrown away.  For example, if the virtio
> device is reset then virtqueue elements are no longer valid.
>
> This patch adds the discard_throttle_data() function to unmap the
> scatter-gather list and decrement vq->inuse.  This ensures that the
> VirtQueueElement is freed properly.
>
> Cc: [e-mail]
> Signed-off-by: Stefan Hajnoczi <[e-mail]>

I have tested this patch with a simple serial stress test to exercise
the code paths.

Tested-by: Ladi Prosek <[e-mail]>
Reviewed-by: Ladi Prosek <[e-mail]>

Patch

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index db57a38..35aa3c7 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -132,6 +132,15 @@  static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev)
     virtio_notify(vdev, vq);
 }
 
+static void discard_throttle_data(VirtIOSerialPort *port)
+{
+    if (port->elem) {
+        virtqueue_detach_element(port->ovq, port->elem, 0);
+        g_free(port->elem);
+        port->elem = NULL;
+    }
+}
+
 static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
                                  VirtIODevice *vdev)
 {
@@ -254,6 +263,7 @@  int virtio_serial_close(VirtIOSerialPort *port)
      * consume, reset the throttling flag and discard the data.
      */
     port->throttled = false;
+    discard_throttle_data(port);
     discard_vq_data(port->ovq, VIRTIO_DEVICE(port->vser));
 
     send_control_event(port->vser, port->id, VIRTIO_CONSOLE_PORT_OPEN, 0);
@@ -554,6 +564,9 @@  static void guest_reset(VirtIOSerial *vser)
 
     QTAILQ_FOREACH(port, &vser->ports, next) {
         vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+
+        discard_throttle_data(port);
+
         if (port->guest_connected) {
             port->guest_connected = false;
             if (vsc->set_guest_connected) {
@@ -864,6 +877,7 @@  static void remove_port(VirtIOSerial *vser, uint32_t port_id)
     assert(port);
 
     /* Flush out any unconsumed buffers first */
+    discard_throttle_data(port);
     discard_vq_data(port->ovq, VIRTIO_DEVICE(port->vser));
 
     send_control_event(vser, port->id, VIRTIO_CONSOLE_PORT_REMOVE, 1);