[Qemu-devel] [PATCH 0/3] virtio-serial: Flow control

[Qemu-devel] [PATCH 0/3] virtio-serial: Flow control

[Amit Shah]

Hello,

This patchset adds support for virtio-serial flow control.  This is a
rework of the method used earlier.  The new method doesn't copy over
the entire guest buffers to host, instead it uses the guest buffer and
adds some state to the port struct.

save/restore support for the new state is to be done.  That involves
saving/restoring items from the VirtQueueElement struct, which needs a
bit of work.

Amit Shah (3):
  virtio-serial: Let virtio-serial-bus know if all data was consumed
  virtio-serial: Add support for flow control
  virtio-console: Enable port throttling when chardev is slow to
    consume data

 hw/virtio-console.c    |   15 ++++++++++++-
 hw/virtio-serial-bus.c |   50 +++++++++++++++++++++++++++++++++++++----------
 hw/virtio-serial.h     |   24 ++++++++++++++++++++--
 3 files changed, 73 insertions(+), 16 deletions(-)

-- 
1.7.3.4

[Qemu-devel] [PATCH 1/3] virtio-serial: Let virtio-serial-bus know if all data was consumed

[Amit Shah]

The have_data() API to hand off guest data to apps using virtio-serial
so far assumed all the data was consumed.  Relax this assumption.
Future commits will allow for incomplete writes.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c |    4 ++--
 hw/virtio-serial.h  |    7 ++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 3bd2b79..7c12230 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -20,11 +20,11 @@ typedef struct VirtConsole {
 
 
 /* Callback function that's called when the guest sends us data */
-static void flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
+static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
 {
     VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
 
-    qemu_chr_write(vcon->chr, buf, len);
+    return qemu_chr_write(vcon->chr, buf, len);
 }
 
 /* Readiness of the guest to accept data on a port */
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index ff08c40..9cc0fb3 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -137,10 +137,11 @@ struct VirtIOSerialPortInfo {
 
     /*
      * Guest wrote some data to the port. This data is handed over to
-     * the app via this callback.  The app is supposed to consume all
-     * the data that is presented to it.
+     * the app via this callback.  The app can return a size less than
+     * 'len'.  In this case, throttling will be enabled for this port.
      */
-    void (*have_data)(VirtIOSerialPort *port, const uint8_t *buf, size_t len);
+    ssize_t (*have_data)(VirtIOSerialPort *port, const uint8_t *buf,
+                         size_t len);
 };
 
 /* Interface to the virtio-serial bus */
-- 
1.7.3.4

[Qemu-devel] [PATCH 2/3] virtio-serial: Add support for flow control

[Amit Shah]

This commit lets apps signal an incomplete write.  When that happens,
stop stop sending out any more data to the app and wait for it to
unthrottle the port.

This involves storing some extra state in the port struct.  This state
will have to be migrated along in a new subsection.  That's an item for
the TODO list.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |   50 +++++++++++++++++++++++++++++++++++++----------
 hw/virtio-serial.h     |   17 ++++++++++++++++
 2 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 6726f72..c5ce60b 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -126,24 +126,49 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice *vdev)
 static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
                                  VirtIODevice *vdev)
 {
-    VirtQueueElement elem;
-
     assert(port);
     assert(virtio_queue_ready(vq));
 
-    while (!port->throttled && virtqueue_pop(vq, &elem)) {
+    while (!port->throttled) {
         unsigned int i;
 
-        for (i = 0; i < elem.out_num; i++) {
-            size_t buf_size;
-
-            buf_size = elem.out_sg[i].iov_len;
+        /* Pop an elem only if we haven't left off a previous one mid-way */
+        if (!port->elem.out_num) {
+            if (!virtqueue_pop(vq, &port->elem)) {
+                break;
+            }
+            port->iov_idx = 0;
+            port->iov_offset = 0;
+        }
 
-            port->info->have_data(port,
-                                  elem.out_sg[i].iov_base,
-                                  buf_size);
+        for (i = port->iov_idx; i < port->elem.out_num; i++) {
+            size_t buf_size;
+            ssize_t ret;
+
+            buf_size = port->elem.out_sg[i].iov_len - port->iov_offset;
+            ret = port->info->have_data(port,
+                                        port->elem.out_sg[i].iov_base
+                                          + port->iov_offset,
+                                        buf_size);
+            if (ret < 0 && ret != -EAGAIN) {
+                /* We don't handle any other type of errors here */
+                abort();
+            }
+            if (ret == -EAGAIN || (ret >= 0 && ret < buf_size)) {
+                virtio_serial_throttle_port(port, true);
+                port->iov_idx = i;
+                if (ret > 0) {
+                    port->iov_offset += ret;
+                }
+                break;
+            }
+            port->iov_offset = 0;
         }
-        virtqueue_push(vq, &elem, 0);
+        if (port->throttled) {
+            break;
+        }
+        virtqueue_push(vq, &port->elem, 0);
+        port->elem.out_num = 0;
     }
     virtio_notify(vdev, vq);
 }
@@ -505,6 +530,7 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
         qemu_put_be32s(f, &port->id);
         qemu_put_byte(f, port->guest_connected);
         qemu_put_byte(f, port->host_connected);
+        /* TODO: Save & load port->elem and port->iov* */
     }
 }
 
@@ -709,6 +735,8 @@ static int virtser_port_qdev_init(DeviceState *qdev, DeviceInfo *base)
         port->guest_connected = true;
     }
 
+    port->elem.out_num = 0;
+
     QTAILQ_INSERT_TAIL(&port->vser->ports, port, next);
     port->ivq = port->vser->ivqs[port->id];
     port->ovq = port->vser->ovqs[port->id];
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index 9cc0fb3..5f41706 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -102,6 +102,23 @@ struct VirtIOSerialPort {
      */
     uint32_t id;
 
+    /*
+     * This is the elem that we pop from the virtqueue.  A slow
+     * backend that consumes guest data (e.g. the file backend for
+     * qemu chardevs) can cause the guest to block till all the output
+     * is flushed.  This isn't desired, so we keep a note of the last
+     * element popped and continue consuming it once the backend
+     * becomes writable again.
+     */
+    VirtQueueElement elem;
+
+    /*
+     * The index and the offset into the iov buffer that was popped in
+     * elem above.
+     */
+    unsigned int iov_idx;
+    size_t iov_offset;
+
     /* Identify if this is a port that binds with hvc in the guest */
     uint8_t is_console;
 
-- 
1.7.3.4

[Qemu-devel] [PATCH 3/3] virtio-console: Enable port throttling when chardev is slow to consume data

[Amit Shah]

When a chardev indicates it can't accept more data, we tell the
virtio-serial code to stop sending us any more data till we tell
otherwise.  This helps in guests continuing to run normally while the vq
keeps getting full and eventually the guest stops queueing more data.
As soon as the chardev indicates it can accept more data, start pushing!

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 7c12230..4c27e4c 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -18,6 +18,16 @@ typedef struct VirtConsole {
     CharDriverState *chr;
 } VirtConsole;
 
+/*
+ * Callback function that's called from chardevs when backend becomes
+ * writable.
+ */
+static void chr_write_unblocked(void *opaque)
+{
+    VirtConsole *vcon = opaque;
+
+    virtio_serial_throttle_port(&vcon->port, false);
+}
 
 /* Callback function that's called when the guest sends us data */
 static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len)
@@ -61,6 +71,7 @@ static QemuChrHandlers chr_handlers = {
     .fd_can_read = chr_can_read,
     .fd_read = chr_read,
     .fd_event = chr_event,
+    .fd_write_unblocked = chr_write_unblocked,
 };
 
 static int generic_port_init(VirtConsole *vcon, VirtIOSerialDevice *dev)
-- 
1.7.3.4