From bffc95c205b473df9bba81dbf676504d0981d392 Mon Sep 17 00:00:00 2001
Message-Id: <bffc95c205b473df9bba81dbf676504d0981d392.1354903384.git.crobinso@redhat.com>
In-Reply-To: <9f0944a25bc1094fa7a74ac9df14e184e2c5c82d.1354903384.git.crobinso@redhat.com>
References: <9f0944a25bc1094fa7a74ac9df14e184e2c5c82d.1354903384.git.crobinso@redhat.com>
From: Alon Levy <alevy@redhat.com>
Date: Tue, 22 Mar 2011 12:27:59 +0200
Subject: [PATCH] spice-qemu-char.c: add throttling

BZ: 672191

upstream: not submitted (explained below)

Adds throttling support to spicevmc chardev. Uses a timer to avoid recursing:
1. spice-server: reds.c:            read_from_vdi_port
2. qemu:         spice-qemu-char.c: vmc_read
3.                                  chr_write_unblocked
                                (calls virtio_serial_throttle_port(port, false))
4. qemu:         virtio ...
5. qemu:         spice-qemu-char.c: spice_chr_write
6. qemu:         spice-qemu-char.c: wakeup (calls into spice-server)
7. spice-server: ...
8. qemu:         spice-qemu-char.c: vmc_read

Instead, in vmc_read if we were throttled and we are just about to return
all the bytes we will set a timer to be triggered immediately to call
chr_write_unblocked. Then we return after 2 above, and 3 is called from the
timer callback. This also means we can later remove some ugly recursion protection
from spice-server.

The other tricky point in this patch is not returning the leftover chunk twice.
When we throttle, by definition we have data that spice server didn't consume.
It is being kept by virtio-serial, and by us. The next vmc_read callback needs
to not return it, but just do unthrottling. Then virtio will give us the remaining
chunk as usual in spice_chr_write, and we will pass it to spice server in the
next vmc_read.

This patch relies on Amit's series to expose throttling to chardev's, which
was not accepted upstream, and will not be accepted upstream until the mainloop
is reworked to use glib.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
 spice-qemu-char.c | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 09aa22d..fba2bfb 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -1,4 +1,6 @@
 #include "config-host.h"
+#include "qemu-common.h"
+#include "qemu-timer.h"
 #include "trace.h"
 #include "ui/qemu-spice.h"
 #include <spice.h>
@@ -25,6 +27,7 @@ typedef struct SpiceCharDriver {
     uint8_t               *datapos;
     ssize_t               bufsize, datalen;
     uint32_t              debug;
+    QEMUTimer             *unblock_timer;
 } SpiceCharDriver;
 
 static int vmc_write(SpiceCharDeviceInstance *sin, const uint8_t *buf, int len)
@@ -50,6 +53,17 @@ static int vmc_write(SpiceCharDeviceInstance *sin, const uint8_t *buf, int len)
     return out;
 }
 
+static void spice_chr_unblock(void *opaque)
+{
+    SpiceCharDriver *scd = opaque;
+
+    if (scd->chr->chr_write_unblocked == NULL) {
+        dprintf(scd, 1, "%s: backend doesn't support unthrottling.\n", __func__);
+        return;
+    }
+    scd->chr->chr_write_unblocked(scd->chr->handler_opaque);
+}
+
 static int vmc_read(SpiceCharDeviceInstance *sin, uint8_t *buf, int len)
 {
     SpiceCharDriver *scd = container_of(sin, SpiceCharDriver, sin);
@@ -61,9 +75,16 @@ static int vmc_read(SpiceCharDeviceInstance *sin, uint8_t *buf, int len)
         scd->datapos += bytes;
         scd->datalen -= bytes;
         assert(scd->datalen >= 0);
-        if (scd->datalen == 0) {
-            scd->datapos = 0;
-        }
+    }
+    if (scd->datalen == 0 && scd->chr->write_blocked) {
+        dprintf(scd, 1, "%s: unthrottling (%d)\n", __func__, bytes);
+        scd->chr->write_blocked = false;
+        /*
+         * set a timer instead of calling scd->chr->chr_write_unblocked directly,
+         * because that will call back into spice_chr_write (see
+         * virtio-console.c:chr_write_unblocked), which is unwanted.
+         */
+        qemu_mod_timer(scd->unblock_timer, 0);
     }
     trace_spice_vmc_read(bytes, len);
     return bytes;
@@ -135,6 +156,7 @@ static void vmc_unregister_interface(SpiceCharDriver *scd)
 static int spice_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
 {
     SpiceCharDriver *s = chr->opaque;
+    int read_bytes;
 
     dprintf(s, 2, "%s: %d\n", __func__, len);
     vmc_register_interface(s);
@@ -147,7 +169,15 @@ static int spice_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
     s->datapos = s->buffer;
     s->datalen = len;
     spice_server_char_device_wakeup(&s->sin);
-    return len;
+    read_bytes = len - s->datalen;
+    if (read_bytes != len) {
+        dprintf(s, 1, "%s: throttling: %d < %d (%zd)\n", __func__,
+                read_bytes, len, s->bufsize);
+        s->chr->write_blocked = true;
+        /* We'll get passed in the unconsumed data with the next call */
+        s->datalen = 0;
+    }
+    return read_bytes;
 }
 
 static void spice_chr_close(struct CharDriverState *chr)
@@ -225,6 +255,7 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
     chr->chr_close = spice_chr_close;
     chr->chr_guest_open = spice_chr_guest_open;
     chr->chr_guest_close = spice_chr_guest_close;
+    s->unblock_timer = qemu_new_timer_ms(vm_clock, spice_chr_unblock, s);
 
 #if SPICE_SERVER_VERSION < 0x000901
     /* See comment in vmc_state() */
-- 
1.8.0