[Spice-devel] [PATCH spice-server 2/2] Only send device display info to supported agents

[Frediano Ziglio]

> 
> Only send the graphics device display info to agents that advertise the
> VD_AGENT_CAP_GRAPHICS_DEVICE_INFO capability
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
>  server/reds-private.h | 1 +
>  server/reds.c         | 9 +++++++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/server/reds-private.h b/server/reds-private.h
> index 9dbc7fa94..b2b99b50e 100644
> --- a/server/reds-private.h
> +++ b/server/reds-private.h
> @@ -134,6 +134,7 @@ struct RedsState {
>      GList *qxl_instances;
>      MainDispatcher *main_dispatcher;
>      RedRecord *record;
> +    gboolean agent_graphics_device_info;

Style: bool
I would prefer a agent_supports_graphics_device_info (or a comment).
But more than this I think it's on the wrong structure, it should be
in RedCharDeviceVDIPortPrivate where the state of the agent is stored.

>  };
>  
>  #define FOREACH_QXL_INSTANCE(_reds, _qxl) \
> diff --git a/server/reds.c b/server/reds.c
> index 63bfadb22..4228ab6ad 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -813,6 +813,10 @@ static void reds_adjust_agent_capabilities(RedsState
> *reds, VDAgentMessage *mess
>      if (!reds->config->agent_file_xfer) {
>          VD_AGENT_SET_CAPABILITY(capabilities->caps,
>          VD_AGENT_CAP_FILE_XFER_DISABLED);
>      }
> +
> +    size_t caps_size = VD_AGENT_CAPS_SIZE_FROM_MSG_SIZE(message->size);
> +    reds->agent_graphics_device_info =
> VD_AGENT_HAS_CAPABILITY(capabilities->caps, caps_size,
> +
> VD_AGENT_CAP_GRAPHICS_DEVICE_INFO);

The "adjust_agent_capabilities" is no more appropriate now, I would
change in "filter_agent_capabilities". Even better to have another
function to handle capabilities (see below).
OT: Note that VD_AGENT_SET_CAPABILITY above could overflow the message,
in this case not much of an issue, the buffer is statically allocated
in a structure and the change will stay in that buffer.

>  }
>  
>  /* reads from the device till completes reading a message that is addressed
>  to the client,
> @@ -965,6 +969,11 @@ void reds_send_device_display_info(RedsState *reds)
>      if (!reds->agent_dev->priv->agent_attached) {
>          return;
>      }
> +    if (!reds->agent_graphics_device_info) {
> +        g_debug("Not sending device display info to the agent because the
> agent does not support it");
> +        return;
> +    }
> +
>      g_debug("Sending device display info to the agent:");
>  
>      SpiceMarshaller *m = spice_marshaller_new();

I think this change will cause the message to not be sent in case the agent connect
as the variable will be false at the beginning, when the capabilities are received
should be sent.
The variable should be reset also that the agent disconnect (reds_reset_vdp)

Frediano