[Spice-devel] [PATCH spice 1/3] server: always call read_from_vdi_port() in a while loop

[Hans de Goede]

read_from_vdi_port() MUST always be called in a while loop until it returns 0.

This is needed because it can cause new data available events and its
recursion protection causes those to get lost. Calling it until it returns 0
ensures that all data has been consumed.

Example scenario where we can fail to read the available data otherwise:
- 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
---
 server/reds.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index a88ca95..8e63062 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1229,6 +1229,10 @@ static void dispatch_vdi_port_data(int port, VDIReadBuf *buf)
     }
 }
 
+/* Note this function MUST always be called in a while loop until it
+   returns 0. This is needed because it can cause new data available events
+   and its recursion protection causes those to get lost. Calling it until
+   it returns 0 ensures that all data has been consumed. */
 static int read_from_vdi_port(void)
 {
     // FIXME: UGLY HACK. Result of spice-vmc vmc_read triggering flush of throttled data, and recalling this.
@@ -1665,7 +1669,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
         agent_start = (SpiceMsgcMainAgentTokens *)message;
         reds->agent_state.client_agent_started = TRUE;
         reds->agent_state.send_tokens = agent_start->num_tokens; // TODO: sanitize? coming from guest!
-        read_from_vdi_port();
+        while (read_from_vdi_port());
         break;
     }
     case SPICE_MSGC_MAIN_AGENT_DATA: {
@@ -1721,7 +1725,7 @@ static void reds_main_handle_message(void *opaque, size_t size, uint32_t type, v
 
         token = (SpiceMsgcMainAgentTokens *)message;
         reds->agent_state.send_tokens += token->num_tokens;
-        read_from_vdi_port();
+        while (read_from_vdi_port());
         break;
     }
     case SPICE_MSGC_MAIN_ATTACH_CHANNELS:
-- 
1.7.3.1