[Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL

[Frediano Ziglio]

> 
> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> 
> When this capability is negoticated by both the client & the agent,

negotiated

> the clipboard grab messages have an associated serial counter.
> 
> The serial is reset to 0 upon client connection.
> 
> The counter is increment by 1 on each grab message, by both sides.
> 

What would happen in case of restart of the guest? How the guest is
supposed to keep the old serial?
I think you can achieve sending the serial with an additional separate
message at the beginning.
I don't think this new protocol is designed to support multiple
clients.

> The sender of the message with the highest serial should be the
> clipboard grab owner, and the current session serial should be
> updated.
> 
> If a lower serial than the current session serial is received, the
> grab should be discarded.
> 
> Whenever two grabs share the same serial, the one coming from the
> client should have a higher priority and the client should gain the
> clipboard ownership.
> 
> No special treatement is done for the unlikely case of overflowing the

treatment

> counter. It may temporarily inverse the priority, until both side have
> overflown and/or synchronized.
> 

synchronized? So there's a single counter for both guest and client?
I though were two counters, one for each side.

> Note: this mechanism isn't aiming at making "the most recent" (as in
> time) side gaining the ownership. One side sending subsequent grab
> messages earlier will likely take the ownership over a side sending a
> single message simultaneously the other way. It only clears the
> situation where both side believe that the other is the current
> clipboard owner, by having a global ordering and priority in case of
> serial conflict.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> ---
>  spice/vd_agent.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/spice/vd_agent.h b/spice/vd_agent.h
> index 862cb5c..9ae3cc7 100644
> --- a/spice/vd_agent.h
> +++ b/spice/vd_agent.h
> @@ -218,6 +218,9 @@ typedef struct SPICE_ATTR_PACKED VDAgentClipboardGrab {
>  #if 0 /* VD_AGENT_CAP_CLIPBOARD_SELECTION */
>      uint8_t selection;
>      uint8_t __reserved[sizeof(uint32_t) - 1 * sizeof(uint8_t)];
> +#endif
> +#if 0 /* VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL */
> +    uint32_t serial;
>  #endif

I would prefer a new message instead of partial structures.
Why not reusing part of __reserved?

>      uint32_t types[0];
>  } VDAgentClipboardGrab;
> @@ -288,6 +291,7 @@ enum {
>      VD_AGENT_CAP_FILE_XFER_DETAILED_ERRORS,
>      VD_AGENT_CAP_GRAPHICS_DEVICE_INFO,
>      VD_AGENT_CAP_CLIPBOARD_NO_RELEASE_ON_REGRAB,
> +    VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL,
>      VD_AGENT_END_CAP,
>  };
>  

I lost the original issue. Won't be easier to just define a static precedence,
like "in case of conflict the client win"? It would avoid to have 2 cases to test,
each potentially with problems to solve.

Frediano