[Spice-devel] [PATCH 0.8 1/9] server: Don't reset agent state when the client disconnects

[Hans de Goede]

We were calling reds_reset_vdp on client disconnect, which is not a good
idea. reds_reset_vdp does 3 things:

1) It resets the state related to reading chunks from the spicevmc virtio
   port. If the client disconnects while the guest agent is in the middle
   of sending a chunk, this will lead to an inconsistent state, and lots
   of printing of "dispatch_vdi_port_data: invalid port" messages caused
   by this inconsistent state sometimes followed by a segfault.

   This can be triggered by copy and pasting something large (say
   a screenshot) from the guest to the spice-gtk client, as the spice-gtk
   client currently has a bug causing it to crash when receiving a multi
   chunk vdagent messages. Without this patch (and with the spice-gtk bug
   present) I can consistently reproduce this.

2) It clears any buffered writes from the client to the guest still pending
   because the virtio port cannot consume data fast enough. Since the agent
   itself is still running fine, throwing away writes for it because the
   client has disconnected makes no sense. Esp, since on clean exit the
   client may very well send a clipboard release message directly
   before closing the connection, and this may get lost this way.

3) It sets client_agent_started to false, this is the only thing which
   actually makes sense to do on client disconnect.

Note that since we no longer reset the vdp state on client disconnect, we
must now reset it on agent disconnect even if we don't have a client. So
the reds_reset_vdp call in reds_agent_remove() gets moved to the top,
above both the agent_state.connected and reds->peer checks which will
both fail in the no client case.
---
 server/reds.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index a808390..1521bc5 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -751,7 +751,7 @@ static void reds_disconnect()
         if (sif->state) {
             sif->state(vdagent, reds->agent_state.connected);
         }
-        reds_reset_vdp();
+        reds->agent_state.client_agent_started = FALSE;
     }
 
     reds_shatdown_channels();
@@ -1099,13 +1099,16 @@ static void reds_agent_remove()
     SpiceCharDeviceInstance *sin = vdagent;
     SpiceCharDeviceInterface *sif;
 
+    if (!reds->mig_target) {
+        reds_reset_vdp();
+    }
+
     if (!reds->agent_state.connected) {
         return;
     }
     reds->agent_state.connected = 0;
     vdagent = NULL;
     reds_update_mouse_mode();
-
     if (!reds->peer || !sin) {
         return;
     }
@@ -1117,7 +1120,6 @@ static void reds_agent_remove()
     if (reds->mig_target) {
         return;
     }
-    reds_reset_vdp();
     reds_send_agent_disconnected();
 }
 
-- 
1.7.4.2