[Spice-devel] [RFC/POC PATCH spice-protocol 02/16] Create a version 2 of the VDAgentMonitorsConfig message

Lukáš Hrázký lhrazky at redhat.com
Thu Jun 14 16:46:21 UTC 2018


On Thu, 2018-06-14 at 08:37 -0500, Jonathon Jongsma wrote:
> On Tue, 2018-06-05 at 17:30 +0200, Lukáš Hrázký wrote:
> > To keep compatibility with old endpoints (any of client, server,
> > vd_agent), we need to copy the message to add the output_id field.
> > 
> > The output_id is the guest-side id of the xrandr output (to be
> > precise,
> > it is the index in the list of xrandr outputs) that is set in the
> > monitors config messages by the streaming agent. It is later used in
> > the
> > guest by vd_agent for mouse input and possibly monitors_config
> > (enabling/disabling monitors and setting the resolution/position of
> > monitors).
> > 
> > Signed-off-by: Lukáš Hrázký <lhrazky at redhat.com>
> > ---
> >  spice/vd_agent.h | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/spice/vd_agent.h b/spice/vd_agent.h
> > index dda7044..05c9c40 100644
> > --- a/spice/vd_agent.h
> > +++ b/spice/vd_agent.h
> > @@ -154,6 +154,33 @@ typedef struct SPICE_ATTR_PACKED
> > VDAgentMonitorsConfig {
> >      VDAgentMonConfig monitors[0];
> >  } VDAgentMonitorsConfig;
> >  
> > +typedef struct SPICE_ATTR_PACKED VDAgentMonConfigV2 {
> > +    /* The output_id is the guest-side id of the xrandr output (to
> > be precise,
> > +     * it is the index in the list of xrandr outputs) that is set in
> > the
> > +     * monitors config messages by the streaming agent. It is later
> > used in the
> > +     * guest by vd_agent for mouse input and possibly
> > monitors_config
> > +     * (enabling/disabling monitors and setting the
> > resolution/position of
> > +     * monitors).
> > +    */
> > +    uint32_t output_id;
> > +    /*
> > +     * Note a width and height of 0 can be used to indicate a
> > disabled
> > +     * monitor, this may only be used with agents with the
> > +     * VD_AGENT_CAP_SPARSE_MONITORS_CONFIG capability.
> > +     */
> > +    uint32_t height;
> > +    uint32_t width;
> > +    uint32_t depth;
> > +    int32_t x;
> > +    int32_t y;
> > +} VDAgentMonConfigV2;
> 
> You might put the output_id at the end of the struct instead of the
> beginning so that the beginning of the struct is identical to the old
> version?

Does it have any actual benefit? I put it first because conventionally
IDs are put first in structs, at least from my experience. Since I
don't feel to be limited by compatibility anymore, I thought I'd make
it nicer while I can.

> Or maybe just extend it rather than copying the old struct?
> 
> typedef struct SPICE_ATTR_PACKED VDAgentMonConfigV2 {
>   VDAgentMonConfig base;
>   uin32_t output_id;
> } ....
> 
> Maybe that makes it a too awkward to use though...

Yeah, saves a bit of duplicity, but makes it more awkward, so I don't
feel it's better?

> > +
> > +typedef struct SPICE_ATTR_PACKED VDAgentMonitorsConfigV2 {
> > +    uint32_t num_of_monitors;
> > +    uint32_t flags;
> > +    VDAgentMonConfigV2 monitors[0];
> > +} VDAgentMonitorsConfigV2;
> > +
> >  enum {
> >      VD_AGENT_DISPLAY_CONFIG_FLAG_DISABLE_WALLPAPER = (1 << 0),
> >      VD_AGENT_DISPLAY_CONFIG_FLAG_DISABLE_FONT_SMOOTH = (1 << 1),


More information about the Spice-devel mailing list