[Spice-devel] [PATCH spice] server: do not fail to read all agent data when called recursively

[Hans de Goede]

We can loose (not handle) a virtio port write from the guest:

- server/reds.c: vdagent_char_device_wakeup get called
  by hw/spice-vmc.c because data has arrived from the guest,
- hw/spice-vmc.c: vmc_read get calls
- If the vmc_read call depletes the current buffer it calls
  virtio_serial_throttle_port(&svc->port, false)
- This causes vmc_have_data to get called, which if in the
  mean time another buffer has arrived causes
  vdagent_char_device_wakeup to gets called again
  (so recursively)
- vdagent_char_device_wakeup is protected against recursive
  calling and ignores the second call (a nasty hack)
- if no other data arrives, the arrived data will not get read

Together with another nasty hack in the linux guest virtio_console
driver, where it waits for a write to be acked by the host before
continuing with the next one, this can lead to the guest
getting stuck / hang (until the write is read by the spice-server
which never happens becaus of the above issues).
---
 server/reds.c |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index a88ca95..f4dfa46 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1231,21 +1231,14 @@ static void dispatch_vdi_port_data(int port, VDIReadBuf *buf)
 
 static int read_from_vdi_port(void)
 {
-    // FIXME: UGLY HACK. Result of spice-vmc vmc_read triggering flush of throttled data, and recalling this.
-    static int inside_call = 0;
     int quit_loop = 0;
     VDIPortState *state = &reds->agent_state;
     SpiceCharDeviceInterface *sif;
     VDIReadBuf *dispatch_buf;
     int total = 0;
     int n;
-    if (inside_call) {
-        return 0;
-    }
-    inside_call = 1;
 
     if (!reds->agent_state.connected || reds->mig_target) {
-        inside_call = 0;
         return 0;
     }
 
@@ -1314,13 +1307,25 @@ static int read_from_vdi_port(void)
             dispatch_vdi_port_data(state->vdi_chunk_header.port, dispatch_buf);
         }
     }
-    inside_call = 0;
     return total;
 }
 
 void vdagent_char_device_wakeup(SpiceCharDeviceInstance *sin)
 {
-    while (read_from_vdi_port());
+    static int vdagent_char_device_wakeups = 0;
+
+    vdagent_char_device_wakeups++;
+
+    if (vdagent_char_device_wakeups != 1) {
+        /* When spice-vmc vmc_read triggers flush of throttled data, we may get
+           recalled and we may not call read_from_vdi_port() recursively */
+        return;
+    }
+
+    while (vdagent_char_device_wakeups) {
+        while (read_from_vdi_port()) {}
+        vdagent_char_device_wakeups--;
+    }
 }
 
 static void reds_handle_agent_mouse_event()
-- 
1.7.3.1