[Spice-devel] [RFC PATCH spice-common v2 01/20] Add a monitors config message from the client to the server

[Jonathon Jongsma]

So I sent my comments on patch 04/20 before this one, but this is
obviously very closely related. This spice protocol message type is
being added to support that new vdagent message. This is a pretty
significant change (from a protocol point of view), because in addition
to adding the new id data pair to the message, the message itself has
also been promoted from a vdagent message to a full-fledged main
channel message. I presume this was done because the main channel has
to do some processing of the data before passing it on to the
agent[*]. 

But this is slightly problematic because this message still requires
the vdagent to be connected in order for it to work properly. But if
you simply look at the protocol message definition, there's no
indication that this message requires a connected vdagent. 

Also, per the spice protocol, vdagent messages are rate-limited by
using a 'token' system. spice-gtk automatically handles that for us
(and I think it also has some handling for vdagent messages that are
attempted to be send when the vdagent is not yet connected). But by
promoting this message to a MainChannel message, we've sidestepped
these things.

All of those things make me think that this may not be the best
approach.


[*] of course, the server has had to do some processing of the monitors
config message for a while now (to decide whether to pass the message
to the agent or to the QXL device), but now the server will need to
actually inspect and modify the data that will be sent down to the
agent.


On Thu, 2018-08-16 at 18:26 +0200, Lukáš Hrázký wrote:
> The message is based on the VDAgentMonitorsConfig message already
> being
> sent from the client (but keeping the naming conventions of the
> SpiceMsgDisplayMonitorsConfig, that is being sent from the client to
> the
> server on the display channel), but adds the channel_id and
> monitor_id
> unique identification pair.
> 
> Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> ---
>  common/messages.h | 16 ++++++++++++++++
>  spice.proto       | 14 ++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/common/messages.h b/common/messages.h
> index 55de76e..942ba07 100644
> --- a/common/messages.h
> +++ b/common/messages.h
> @@ -112,6 +112,22 @@ typedef struct SpiceMsgMainMigrationSwitchHost {
>      uint8_t *cert_subject_data;
>  } SpiceMsgMainMigrationSwitchHost;
>  
> +typedef struct SpiceMsgcMainHead {
> +    uint32_t channel_id;
> +    uint32_t monitor_id;
> +    uint32_t width;
> +    uint32_t height;
> +    uint32_t depth;
> +    uint32_t x;
> +    uint32_t y;
> +} SpiceMainHead;
> +
> +typedef struct SpiceMsgcMainMonitorsConfig {
> +    uint16_t count;
> +    uint32_t flags;
> +    SpiceMainHead heads[0];
> +} SpiceMsgcMainMonitorsConfig;
> +
>  
>  typedef struct SpiceMsgMigrate {
>      uint32_t flags;
> diff --git a/spice.proto b/spice.proto
> index 07f11ec..80976d4 100644
> --- a/spice.proto
> +++ b/spice.proto
> @@ -340,6 +340,20 @@ channel MainChannel : BaseChannel {
>      } migrate_dst_do_seamless;
>  
>      Empty migrate_connected_seamless;
> +
> +    message {
> +        uint16 count;
> +        uint32 flags;
> +        struct MainHead {
> +            uint32 channel_id;
> +            uint32 monitor_id;
> +            uint32 width;
> +            uint32 height;
> +            uint32 depth;
> +            uint32 x;
> +            uint32 y;
> +        } heads[count] @end;
> +    } monitors_config;
>  };
>  
>  enum8 clip_type {