[Spice-devel] [PATCH 5/5] client: don't crash when agent is missing WAN support

[Alon Levy]

On Fri, Jul 08, 2011 at 12:17:32PM +0200, Christophe Fergeau wrote:
> If you try to connect to a linux guest with WAN options, SPICE window opens up
> and is blank - it then fails with vdagent timeout message.  It should give a
> warning that this is only applicable for windows guest and still connect to
> guest.
> 
> It all starts in RedClient::handle_init
> This function checks whether we have an agent or not, because if we have an
> agent, there will be some kind of handshake to check both sides capabilities
> before all the spice channels are created.
> 
> When there is no agent running, the startup process goes on with
> SPICE_MSGC_MAIN_ATTACH_CHANNELS
> 
> When there is a windows agent running, VD_AGENT_ANNOUNCE_CAPABILITIES and
> VD_AGENT_DISPLAY_CONFIG messages are sent to the agent, and when processing the
> agent answer to the VD_AGENT_DISPLAY_CONFIG message,
> SPICE_MSGC_MAIN_ATTACH_CHANNELS will be sent and the startup process will go
> on.
> 
> However, when there is no agent running but --color-depth was used, handle_init
> won't send the SPICE_MSGC_MAIN_ATTACH_CHANNELS message but will wait for the
> agent handshake to proceed to its end, which won't happen, so it will timeout
> waiting for agent answers.
> 
> Similarly, the linux agent handles VD_AGENT_ANNOUNCE_CAPABILITIES messages, but
> it doesn't handle VD_AGENT_DISPLAY_CONFIG messages, so we'll never reach the
> point where a SPICE_MSGC_MAIN_ATTACH_CHANNELS will be sent.
> 
> This commit fixes this in 2 places:
> - unconditionnally send SPICE_MSGC_ATTACH_CHANNELS when no agent is running in
> handle_init
> - send SPICE_MSGC_MAIN_ATTACH_CHANNELS in
> RedClient::on_agent_announce_capabilities if the agent doesn't have the
> VD_AGENT_CAP_DISPLAY_CONFIG capability
> 
> This fixes RH bug #712938
> ---
>  client/red_client.cpp |   28 ++++++++++++++++++++--------
>  1 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/client/red_client.cpp b/client/red_client.cpp
> index 8918e4f..edcdb02 100644
> --- a/client/red_client.cpp
> +++ b/client/red_client.cpp
> @@ -966,19 +966,20 @@ void RedClient::handle_init(RedPeer::InMessage* message)
>          agent_start.num_tokens = ~0;
>          _marshallers->msgc_main_agent_start(msg->marshaller(), &agent_start);
>          post_message(msg);
> -    }
> -
> -    if (_agent_connected) {

Why do we want to send agent_announce_capabilities if !_agent_connected?

>          send_agent_announce_capabilities(true);
>          if (_auto_display_res) {
>             send_agent_monitors_config();
>          }
> -    }
> -
> -    if (!_auto_display_res && _display_setting.is_empty()) {
> -        post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +        if (_auto_display_res || !_display_setting.is_empty()) {
> +            _application.activate_interval_timer(*_agent_timer, AGENT_TIMEOUT);
> +        } else {
> +            post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +        }
>      } else {
> -        _application.activate_interval_timer(*_agent_timer, AGENT_TIMEOUT);
> +        if (_auto_display_res || !_display_setting.is_empty()) {
> +            LOG_WARN("no agent running, display options have been ignored");
> +        }
> +        post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
>      }
>  }
>  
> @@ -1046,6 +1047,17 @@ void RedClient::on_agent_announce_capabilities(
>          // not sending the color depth through send_agent_monitors_config, since
>          // it applies only for attached screens.
>          send_agent_display_config();
> +    } else if (!_auto_display_res) {

Who sets _auto_display_res? does this affect windows guest agents? the comment
below says linux but the test above seems to be not linux specific.

> +        /* linux agent doesn't support monitors/displays agent messages, so
> +         * we'll never reach on_agent_reply which sends this
> +         * ATTACH_CHANNELS message which is needed for client startup to go
> +         * on.
> +         */
> +        if (_auto_display_res || !_display_setting.is_empty()) {
> +            LOG_WARN("display options have been requested, but the agent doesn't support these options");
> +        }
> +        post_message(new Message(SPICE_MSGC_MAIN_ATTACH_CHANNELS));
> +        _application.deactivate_interval_timer(*_agent_timer);
>      }
>  }
>  
> -- 
> 1.7.6
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel