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

[Alon Levy]

On Fri, Apr 01, 2011 at 05:13:01PM +0200, Hans de Goede wrote:
> We were calling reds_reset_vdp on client disconnect, which is not a good
> idea. reds_reset_vdp does 3 things:

ACK.

> 
> 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
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel