[Spice-devel] [PATCH] Wait to send monitor config until agent caps are received

[Marc-André Lureau]

----- Original Message -----
> When the first display is disabled and the vdagent is restarted in the guest,
> it sometimes becomes enabled. This appears to be caused by a race condition
> when an agent becomes connected. When the agent becomes connected,
> virt-viewer
> triggers a display update (spice_main_send_monitor_config()). This display
> update happens in a timeout handler, but the timeout interval is set to 0 (so
> it behaves basically like an idle handler).
> 
> The race happens because spice_main_send_monitor_config() behaves slightly
> differently depending on the agent's capabilities. And sometimes the idle
> handler runs before the client and server have negotiated capabilities. In
> this
> case, we have to assume that the server does not support sparse monitor
> configurations. So instead of sending down an update where display #0 is off
> and display #1 is WxH, we send down an update that only a single display:
> display #0 is WxH. This results in the first display becoming enabled.
> 
> To solve the issue, we trigger a display update when the agent becomes
> connected, but give it a long timeout (e.g. 5 seconds). This gives some time

To avoid the timeout, we could rely on the agent caps reply to happen right after sending the client caps->request, no?

> for the agent capabilities negotiation to occur. When we receive the agent
> capabilities from the guest, we trigger an immediate display update
> (cancelling
> the previously-scheduled update). If we receive no agent capabilities
> (perhaps
> because we're connected to a very old server that doesn't send capabilities),
> the display update will occur after the initial timeout.
> 
> Resolves: rhbz#1043782, rhbz#1032923
> ---
> 
> NOTE: the 5-second timeout I've used here is rather arbitrary. I'm open to
> modifying this value if somebody has a good argument for a shorter or longer
> timeout.
> 
>  gtk/channel-main.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/gtk/channel-main.c b/gtk/channel-main.c
> index 7a299a4..cb873b7 100644
> --- a/gtk/channel-main.c
> +++ b/gtk/channel-main.c
> @@ -1356,6 +1356,12 @@ static void agent_start(SpiceMainChannel *channel)
>          agent_announce_caps(channel);
>          agent_send_msg_queue(channel);
>      }
> +
> +    // trigger a display update, but give it a long timeout to allow agent
> +    // capabilities to be negotiated. When we receive the capabilities
> message,
> +    // the display update will happen immediately. This long timeout is to
> +    // accomodate older servers that may not send capabilities.
> +    update_display_timer(SPICE_MAIN_CHANNEL(channel), 5);
>  }
>  
>  /* coroutine context  */
> @@ -1508,7 +1514,6 @@ static void main_handle_mouse_mode(SpiceChannel
> *channel, SpiceMsgIn *in)
>  static void main_handle_agent_connected(SpiceChannel *channel, SpiceMsgIn
>  *in)
>  {
>      agent_start(SPICE_MAIN_CHANNEL(channel));
> -    update_display_timer(SPICE_MAIN_CHANNEL(channel), 0);
>  }
>  
>  /* coroutine context */
> @@ -1519,7 +1524,6 @@ static void
> main_handle_agent_connected_tokens(SpiceChannel *channel, SpiceMsgIn
>  
>      c->agent_tokens = msg->num_tokens;
>      agent_start(SPICE_MAIN_CHANNEL(channel));
> -    update_display_timer(SPICE_MAIN_CHANNEL(channel), 0);
>  }
>  
>  /* coroutine context */
> @@ -1790,6 +1794,7 @@ static void main_agent_handle_msg(SpiceChannel
> *channel,
>          }
>          c->agent_caps_received = true;
>          g_coroutine_signal_emit(self, signals[SPICE_MAIN_AGENT_UPDATE], 0);
> +        update_display_timer(SPICE_MAIN_CHANNEL(channel), 0);
>  
>          if (caps->request)
>              agent_announce_caps(self);
> --
> 1.9.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>