Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
Hi
On Fri, Mar 29, 2019 at 10:11 AM Frediano Ziglio wrote:
>
> >
> > Hi
> >
> > On Thu, Mar 28, 2019 at 6:05 PM Frediano Ziglio wrote:
> > >
> > > >
> > > > ..Hi
> > > >
> > > > On Thu, Mar 28, 2019 at 4:14 PM Frediano Ziglio
> > > > wrote:
> > > > > > The role of the grab message is to take ownership of the clipboard
> > > > > > (to
> > > > > > advertize clipboard data available). It may come at any time from
> > > > > > both
> > > > > > side, and override the current grab owner. It may come from the
> > > > > > guest
> > > > > > (after C-c in guest app, for ex), so the client grabs the clipboard.
> > > > > > Or it may come from the client (C-c in client side app), and the
> > > > > > agent
> > > > > > will grab the guest clipboard.
> > > > > >
> > > > >
> > > > > Yes, I realized this morning thinking about how clipboards works
> > > > > (very rusty in my mind).
> > > > > Instead of setting it you get the ownership that is you are willing
> > > > > to set a value on the clipboard but you defer setting it to avoid
> > > > > the expense of data copy.
> > > > > So, the old (original?) protocol was simply to sending entire data
> > > > > on every change, this avoided to loose the clipboard data entirely.
> > > >
> > > > I don't even remember that version. It looks like the original version
> > > > was already "on-demand"
> > > >
> > > >
> > > > commit 5fdd0ba6650919dcfd7740c041ad7d2b9c4560ab
> > > > Author: Arnon Gilboa
> > > > Date: Mon Oct 4 16:45:15 2010 +0200
> > > >
> > > > vd_agent: add protocol messages for clipboard/selection-owner model
> > > >
> > > > -VD_AGENT_CLIPBOARD_GRAB(type) - tell the other side that an
> > > > application in our side ("we") got ownership of the clipboard.
> > > > -VD_AGENT_CLIPBOARD_REQUEST(type) - after we know the other side
> > > > owns the clipboard (GRAB received), we notify the os we are the owner.
> > > > when we are asked by the os/app for the clipboard data, we send this
> > > > RE
> > > > QUEST msg to the other side.
> > > > -VD_AGENT_CLIPBOARD(type, data) - the existing message for sending
> > > > the clipboard, is now sent only in response to REQUEST.
> > > > -VD_AGENT_CLIPBOARD_RELEASE - tell the other side that we are no
> > > > longer the owner of the clipboard (e.g. the owner app was closed).
> > > >
> > > > this patch will be followed by agent & client patches handling the
> > > > above messages.
> > > >
> > > >
> > > >
> > > > So VD_AGENT_CAP_CLIPBOARD_BY_DEMAND simply means clipboard support to
> > > > me.
> > > >
> > >
> > > I suppose the "now" in "the existing message for sending the clipboard, is
> > > now sent only in response to REQUEST" means that was changed.
> > >
> > > I personally think the code should be readable from a tarball/snapshot not
> > > pretending people to dig into 10 years of history. I'll try to find some
> > > time
> > > to put these lines into vd_agent.h.
> > >
> > > > > The current is: if we get new local clipboard data send the "grab"
> > > > > so remote knows that will have to read our data.
> > > >
> > > > yes
> > > >
> > > > > So either we have ownership/grab, meaning the data are remote
> > > > > waiting to get fetched or we don't have ownership and the remote
> > > > > should be grabbing as we have data to send (at least in a stable
> > > > > state).
> > > >
> > > >
> > > > That last sentence is confusing. There are 2 states the client can
> > > > "have the grab".
> > > >
> > > > 1. the client took the grab for the remote data: we are talking about
> > > > the client app in the client desktop environment
> > > > 2. the client took the grab, at the protocol level: it sent a grab
> > > > over the protocol to announce it can provide clipboard data to the
> > > > remote. In this case, the client app doesn't have the grab in the
> > > > client desktop environment, but another client application.
> > > >
> > > > In general, when talking about the protocol, "client has the grab"
> > > > means 2) to me, iow it can provide data to the remote.
> > > >
> > >
> > > I think I mean the opposite. The reason is that some application
> > > have the ownership on either side, when you send a VD_AGENT_CLIPBOARD_GRAB
> > > the current local application (so spice-gtk/vd_agent) does NOT have the
> > > ownership and it's asking the remote to take to the application (so will
> > > remove the ownership from another remote application).
> > > From how I read the message (VD_AGENT_CLIPBOARD_GRAB) is telling the
> > > other side to grab (take ownership) of the clipboard so will be the
> > > remote to have the "grab", not the local.
> >
> >
> > I find it hard to understand you. The sequence of events is as such.
> >
> > A & B are client or remote exchangeably:
> > - an application on A takes the clipboard grab (== announce clipboard
> > data is available)
> > - A get notified by the desktop and sends a GRAB message to B side
> > - B receives GRAB, and takes the clipboard grab on B desktop side
> >
> > With this
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
>
> Hi
>
> On Thu, Mar 28, 2019 at 6:05 PM Frediano Ziglio wrote:
> >
> > >
> > > ..Hi
> > >
> > > On Thu, Mar 28, 2019 at 4:14 PM Frediano Ziglio
> > > wrote:
> > > > > The role of the grab message is to take ownership of the clipboard
> > > > > (to
> > > > > advertize clipboard data available). It may come at any time from
> > > > > both
> > > > > side, and override the current grab owner. It may come from the guest
> > > > > (after C-c in guest app, for ex), so the client grabs the clipboard.
> > > > > Or it may come from the client (C-c in client side app), and the
> > > > > agent
> > > > > will grab the guest clipboard.
> > > > >
> > > >
> > > > Yes, I realized this morning thinking about how clipboards works
> > > > (very rusty in my mind).
> > > > Instead of setting it you get the ownership that is you are willing
> > > > to set a value on the clipboard but you defer setting it to avoid
> > > > the expense of data copy.
> > > > So, the old (original?) protocol was simply to sending entire data
> > > > on every change, this avoided to loose the clipboard data entirely.
> > >
> > > I don't even remember that version. It looks like the original version
> > > was already "on-demand"
> > >
> > >
> > > commit 5fdd0ba6650919dcfd7740c041ad7d2b9c4560ab
> > > Author: Arnon Gilboa
> > > Date: Mon Oct 4 16:45:15 2010 +0200
> > >
> > > vd_agent: add protocol messages for clipboard/selection-owner model
> > >
> > > -VD_AGENT_CLIPBOARD_GRAB(type) - tell the other side that an
> > > application in our side ("we") got ownership of the clipboard.
> > > -VD_AGENT_CLIPBOARD_REQUEST(type) - after we know the other side
> > > owns the clipboard (GRAB received), we notify the os we are the owner.
> > > when we are asked by the os/app for the clipboard data, we send this
> > > RE
> > > QUEST msg to the other side.
> > > -VD_AGENT_CLIPBOARD(type, data) - the existing message for sending
> > > the clipboard, is now sent only in response to REQUEST.
> > > -VD_AGENT_CLIPBOARD_RELEASE - tell the other side that we are no
> > > longer the owner of the clipboard (e.g. the owner app was closed).
> > >
> > > this patch will be followed by agent & client patches handling the
> > > above messages.
> > >
> > >
> > >
> > > So VD_AGENT_CAP_CLIPBOARD_BY_DEMAND simply means clipboard support to me.
> > >
> >
> > I suppose the "now" in "the existing message for sending the clipboard, is
> > now sent only in response to REQUEST" means that was changed.
> >
> > I personally think the code should be readable from a tarball/snapshot not
> > pretending people to dig into 10 years of history. I'll try to find some
> > time
> > to put these lines into vd_agent.h.
> >
> > > > The current is: if we get new local clipboard data send the "grab"
> > > > so remote knows that will have to read our data.
> > >
> > > yes
> > >
> > > > So either we have ownership/grab, meaning the data are remote
> > > > waiting to get fetched or we don't have ownership and the remote
> > > > should be grabbing as we have data to send (at least in a stable
> > > > state).
> > >
> > >
> > > That last sentence is confusing. There are 2 states the client can
> > > "have the grab".
> > >
> > > 1. the client took the grab for the remote data: we are talking about
> > > the client app in the client desktop environment
> > > 2. the client took the grab, at the protocol level: it sent a grab
> > > over the protocol to announce it can provide clipboard data to the
> > > remote. In this case, the client app doesn't have the grab in the
> > > client desktop environment, but another client application.
> > >
> > > In general, when talking about the protocol, "client has the grab"
> > > means 2) to me, iow it can provide data to the remote.
> > >
> >
> > I think I mean the opposite. The reason is that some application
> > have the ownership on either side, when you send a VD_AGENT_CLIPBOARD_GRAB
> > the current local application (so spice-gtk/vd_agent) does NOT have the
> > ownership and it's asking the remote to take to the application (so will
> > remove the ownership from another remote application).
> > From how I read the message (VD_AGENT_CLIPBOARD_GRAB) is telling the
> > other side to grab (take ownership) of the clipboard so will be the
> > remote to have the "grab", not the local.
>
>
> I find it hard to understand you. The sequence of events is as such.
>
> A & B are client or remote exchangeably:
> - an application on A takes the clipboard grab (== announce clipboard
> data is available)
> - A get notified by the desktop and sends a GRAB message to B side
> - B receives GRAB, and takes the clipboard grab on B desktop side
>
> With this sequence, "A" has the clipboard grab at the Spice protocol
> level, so to say. It means there is clipboard data on A side.
>
Not clear either. If what you wrote it's correct, what I read:
You wrote "B receives GRAB, and takes the clipboard grab" so I suppose
that B now have the "clipboard grab", however
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
Hi
On Thu, Mar 28, 2019 at 6:05 PM Frediano Ziglio wrote:
>
> >
> > ..Hi
> >
> > On Thu, Mar 28, 2019 at 4:14 PM Frediano Ziglio wrote:
> > > > The role of the grab message is to take ownership of the clipboard (to
> > > > advertize clipboard data available). It may come at any time from both
> > > > side, and override the current grab owner. It may come from the guest
> > > > (after C-c in guest app, for ex), so the client grabs the clipboard.
> > > > Or it may come from the client (C-c in client side app), and the agent
> > > > will grab the guest clipboard.
> > > >
> > >
> > > Yes, I realized this morning thinking about how clipboards works
> > > (very rusty in my mind).
> > > Instead of setting it you get the ownership that is you are willing
> > > to set a value on the clipboard but you defer setting it to avoid
> > > the expense of data copy.
> > > So, the old (original?) protocol was simply to sending entire data
> > > on every change, this avoided to loose the clipboard data entirely.
> >
> > I don't even remember that version. It looks like the original version
> > was already "on-demand"
> >
> >
> > commit 5fdd0ba6650919dcfd7740c041ad7d2b9c4560ab
> > Author: Arnon Gilboa
> > Date: Mon Oct 4 16:45:15 2010 +0200
> >
> > vd_agent: add protocol messages for clipboard/selection-owner model
> >
> > -VD_AGENT_CLIPBOARD_GRAB(type) - tell the other side that an
> > application in our side ("we") got ownership of the clipboard.
> > -VD_AGENT_CLIPBOARD_REQUEST(type) - after we know the other side
> > owns the clipboard (GRAB received), we notify the os we are the owner.
> > when we are asked by the os/app for the clipboard data, we send this
> > RE
> > QUEST msg to the other side.
> > -VD_AGENT_CLIPBOARD(type, data) - the existing message for sending
> > the clipboard, is now sent only in response to REQUEST.
> > -VD_AGENT_CLIPBOARD_RELEASE - tell the other side that we are no
> > longer the owner of the clipboard (e.g. the owner app was closed).
> >
> > this patch will be followed by agent & client patches handling the
> > above messages.
> >
> >
> >
> > So VD_AGENT_CAP_CLIPBOARD_BY_DEMAND simply means clipboard support to me.
> >
>
> I suppose the "now" in "the existing message for sending the clipboard, is
> now sent only in response to REQUEST" means that was changed.
>
> I personally think the code should be readable from a tarball/snapshot not
> pretending people to dig into 10 years of history. I'll try to find some time
> to put these lines into vd_agent.h.
>
> > > The current is: if we get new local clipboard data send the "grab"
> > > so remote knows that will have to read our data.
> >
> > yes
> >
> > > So either we have ownership/grab, meaning the data are remote
> > > waiting to get fetched or we don't have ownership and the remote
> > > should be grabbing as we have data to send (at least in a stable
> > > state).
> >
> >
> > That last sentence is confusing. There are 2 states the client can
> > "have the grab".
> >
> > 1. the client took the grab for the remote data: we are talking about
> > the client app in the client desktop environment
> > 2. the client took the grab, at the protocol level: it sent a grab
> > over the protocol to announce it can provide clipboard data to the
> > remote. In this case, the client app doesn't have the grab in the
> > client desktop environment, but another client application.
> >
> > In general, when talking about the protocol, "client has the grab"
> > means 2) to me, iow it can provide data to the remote.
> >
>
> I think I mean the opposite. The reason is that some application
> have the ownership on either side, when you send a VD_AGENT_CLIPBOARD_GRAB
> the current local application (so spice-gtk/vd_agent) does NOT have the
> ownership and it's asking the remote to take to the application (so will
> remove the ownership from another remote application).
> From how I read the message (VD_AGENT_CLIPBOARD_GRAB) is telling the
> other side to grab (take ownership) of the clipboard so will be the
> remote to have the "grab", not the local.
I find it hard to understand you. The sequence of events is as such.
A & B are client or remote exchangeably:
- an application on A takes the clipboard grab (== announce clipboard
data is available)
- A get notified by the desktop and sends a GRAB message to B side
- B receives GRAB, and takes the clipboard grab on B desktop side
With this sequence, "A" has the clipboard grab at the Spice protocol
level, so to say. It means there is clipboard data on A side.
> >
> > > Not sure what is the initial state, when you connect.
> >
> > Initial state is undefined, and currently whatever the guest or client
> > side state is. Iow, Spice client/agent doen't do anything afaik. They
> > will only react to further grab events.
> >
>
> I suppose the agent will keep its state while the client if was not
> connected get some initial state (boolean variables have to be true or
> false at the end).
>
> > We
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
>
> ..Hi
>
> On Thu, Mar 28, 2019 at 4:14 PM Frediano Ziglio wrote:
> > > The role of the grab message is to take ownership of the clipboard (to
> > > advertize clipboard data available). It may come at any time from both
> > > side, and override the current grab owner. It may come from the guest
> > > (after C-c in guest app, for ex), so the client grabs the clipboard.
> > > Or it may come from the client (C-c in client side app), and the agent
> > > will grab the guest clipboard.
> > >
> >
> > Yes, I realized this morning thinking about how clipboards works
> > (very rusty in my mind).
> > Instead of setting it you get the ownership that is you are willing
> > to set a value on the clipboard but you defer setting it to avoid
> > the expense of data copy.
> > So, the old (original?) protocol was simply to sending entire data
> > on every change, this avoided to loose the clipboard data entirely.
>
> I don't even remember that version. It looks like the original version
> was already "on-demand"
>
>
> commit 5fdd0ba6650919dcfd7740c041ad7d2b9c4560ab
> Author: Arnon Gilboa
> Date: Mon Oct 4 16:45:15 2010 +0200
>
> vd_agent: add protocol messages for clipboard/selection-owner model
>
> -VD_AGENT_CLIPBOARD_GRAB(type) - tell the other side that an
> application in our side ("we") got ownership of the clipboard.
> -VD_AGENT_CLIPBOARD_REQUEST(type) - after we know the other side
> owns the clipboard (GRAB received), we notify the os we are the owner.
> when we are asked by the os/app for the clipboard data, we send this
> RE
> QUEST msg to the other side.
> -VD_AGENT_CLIPBOARD(type, data) - the existing message for sending
> the clipboard, is now sent only in response to REQUEST.
> -VD_AGENT_CLIPBOARD_RELEASE - tell the other side that we are no
> longer the owner of the clipboard (e.g. the owner app was closed).
>
> this patch will be followed by agent & client patches handling the
> above messages.
>
>
>
> So VD_AGENT_CAP_CLIPBOARD_BY_DEMAND simply means clipboard support to me.
>
I suppose the "now" in "the existing message for sending the clipboard, is
now sent only in response to REQUEST" means that was changed.
I personally think the code should be readable from a tarball/snapshot not
pretending people to dig into 10 years of history. I'll try to find some time
to put these lines into vd_agent.h.
> > The current is: if we get new local clipboard data send the "grab"
> > so remote knows that will have to read our data.
>
> yes
>
> > So either we have ownership/grab, meaning the data are remote
> > waiting to get fetched or we don't have ownership and the remote
> > should be grabbing as we have data to send (at least in a stable
> > state).
>
>
> That last sentence is confusing. There are 2 states the client can
> "have the grab".
>
> 1. the client took the grab for the remote data: we are talking about
> the client app in the client desktop environment
> 2. the client took the grab, at the protocol level: it sent a grab
> over the protocol to announce it can provide clipboard data to the
> remote. In this case, the client app doesn't have the grab in the
> client desktop environment, but another client application.
>
> In general, when talking about the protocol, "client has the grab"
> means 2) to me, iow it can provide data to the remote.
>
I think I mean the opposite. The reason is that some application
have the ownership on either side, when you send a VD_AGENT_CLIPBOARD_GRAB
the current local application (so spice-gtk/vd_agent) does NOT have the
ownership and it's asking the remote to take to the application (so will
remove the ownership from another remote application).
From how I read the message (VD_AGENT_CLIPBOARD_GRAB) is telling the
other side to grab (take ownership) of the clipboard so will be the
remote to have the "grab", not the local.
>
> > Not sure what is the initial state, when you connect.
>
> Initial state is undefined, and currently whatever the guest or client
> side state is. Iow, Spice client/agent doen't do anything afaik. They
> will only react to further grab events.
>
I suppose the agent will keep its state while the client if was not
connected get some initial state (boolean variables have to be true or
false at the end).
> We could change the client or the guest to take the grab on connect,
> if clipboard data is available. That doesn't require protocol change.
> Although in case of conflict, we would probably end in the same
> situation that this protocol change is aiming to solve.
>
When there's a disconnection we should drop the ownership as we
cannot anymore get the data from the remote in case other application
are asking so the "disconnected" state should be no ownership of
the clipboard on both ends (although I suppose the client will be
closed at that time).
Once a connection happens we could however have both ends with
data on the clipboard (the total states are 3 (a) client/agent have
ownership (b) other
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
..Hi
On Thu, Mar 28, 2019 at 4:14 PM Frediano Ziglio wrote:
> > The role of the grab message is to take ownership of the clipboard (to
> > advertize clipboard data available). It may come at any time from both
> > side, and override the current grab owner. It may come from the guest
> > (after C-c in guest app, for ex), so the client grabs the clipboard.
> > Or it may come from the client (C-c in client side app), and the agent
> > will grab the guest clipboard.
> >
>
> Yes, I realized this morning thinking about how clipboards works
> (very rusty in my mind).
> Instead of setting it you get the ownership that is you are willing
> to set a value on the clipboard but you defer setting it to avoid
> the expense of data copy.
> So, the old (original?) protocol was simply to sending entire data
> on every change, this avoided to loose the clipboard data entirely.
I don't even remember that version. It looks like the original version
was already "on-demand"
commit 5fdd0ba6650919dcfd7740c041ad7d2b9c4560ab
Author: Arnon Gilboa
Date: Mon Oct 4 16:45:15 2010 +0200
vd_agent: add protocol messages for clipboard/selection-owner model
-VD_AGENT_CLIPBOARD_GRAB(type) - tell the other side that an
application in our side ("we") got ownership of the clipboard.
-VD_AGENT_CLIPBOARD_REQUEST(type) - after we know the other side
owns the clipboard (GRAB received), we notify the os we are the owner.
when we are asked by the os/app for the clipboard data, we send this
RE
QUEST msg to the other side.
-VD_AGENT_CLIPBOARD(type, data) - the existing message for sending
the clipboard, is now sent only in response to REQUEST.
-VD_AGENT_CLIPBOARD_RELEASE - tell the other side that we are no
longer the owner of the clipboard (e.g. the owner app was closed).
this patch will be followed by agent & client patches handling the
above messages.
So VD_AGENT_CAP_CLIPBOARD_BY_DEMAND simply means clipboard support to me.
> The current is: if we get new local clipboard data send the "grab"
> so remote knows that will have to read our data.
yes
> So either we have ownership/grab, meaning the data are remote
> waiting to get fetched or we don't have ownership and the remote
> should be grabbing as we have data to send (at least in a stable
> state).
That last sentence is confusing. There are 2 states the client can
"have the grab".
1. the client took the grab for the remote data: we are talking about
the client app in the client desktop environment
2. the client took the grab, at the protocol level: it sent a grab
over the protocol to announce it can provide clipboard data to the
remote. In this case, the client app doesn't have the grab in the
client desktop environment, but another client application.
In general, when talking about the protocol, "client has the grab"
means 2) to me, iow it can provide data to the remote.
> Not sure what is the initial state, when you connect.
Initial state is undefined, and currently whatever the guest or client
side state is. Iow, Spice client/agent doen't do anything afaik. They
will only react to further grab events.
We could change the client or the guest to take the grab on connect,
if clipboard data is available. That doesn't require protocol change.
Although in case of conflict, we would probably end in the same
situation that this protocol change is aiming to solve.
> But from the stable state (one and only one has the ownership)
> is not clear how you get both sending the grab message at the same
> time (the one without ownership should not send the grab).
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
> Hi
>
> On Wed, Mar 27, 2019 at 4:50 PM Frediano Ziglio wrote:
> >
> > > Hi
> > >
> > > On Wed, Mar 27, 2019 at 8:23 AM Frediano Ziglio
> > > wrote:
> > > >
> > > > >
> > > > > From: Marc-André Lureau
> > > > >
> > > > > 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?
> > >
> > > This is like a new client-agent connection in this case, it starts again
> > > from
> > > 0.
> > >
> > > > I think you can achieve sending the serial with an additional separate
> > > > message at the beginning.
> > >
> > > I don't think it's necessary, but I am may be missing something.
> > >
> >
> > Well, adding a field or a message are both changes, just different.
> >
> > > > I don't think this new protocol is designed to support multiple
> > > > clients.
> > >
> > > clipboard sharing isn't designed for multiple client either.
> > >
> > > >
> > > > > 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
> > >
> > > ok
> > >
> > > >
> > > > > 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.
> > >
> > > Conceptually, it's the "same" counter.
> > >
> >
> > Okay, it was not clear.
> >
> > > >
> > > > > 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
> > > > > ---
> > > > > 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.
> > >
> > > VDAgentClipboardGrabSelectionAndSerialAnd... ?
> > >
> >
> > Usually a version schema is used like VDAgentClipboardGrabSelection_v2
> >
> > > yeah, it's not ideal. I wish we would use a better protocol, be it
> > > json or protobuf etc..
> > >
> >
> > This is not a justification to produce even worse code.
> > A plain C structure would be fine, many binary protocols (like IP)
> > uses this method.
> > A semi-defined C structure with missing fields is the worst I
> > ever seen, continuing to use is not so sensible.
>
> Different trade-offs, version vs capabilities.
>
> Since we don't have missing fields in the protocol, this means
> VDAgentClipboardGrabSelection_v2 should have selection capability
> included.
>
I don't think this is an issue
> This may not be a problem for vdagent, but for many cases,
> capabilities offer more flexibility than versioning.
>
Sure. What I don't like, if is not really clear is having a
semi-defined C structure. I would prefer to not having the structure
instead. There's no C rule to define "this field is present if you
have this capability", VDAgentClipboardGrabSelection does not
represent a message (unless there's no selection) but currently
the end of a message so it's misleading.
> > It would be better to not define the C structure at all.
> > OT: I think it was discussed to use an external library/tool, somebody
> > suggested to implement new messages with
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
Hi
On Wed, Mar 27, 2019 at 4:50 PM Frediano Ziglio wrote:
>
> > Hi
> >
> > On Wed, Mar 27, 2019 at 8:23 AM Frediano Ziglio wrote:
> > >
> > > >
> > > > From: Marc-André Lureau
> > > >
> > > > 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?
> >
> > This is like a new client-agent connection in this case, it starts again
> > from
> > 0.
> >
> > > I think you can achieve sending the serial with an additional separate
> > > message at the beginning.
> >
> > I don't think it's necessary, but I am may be missing something.
> >
>
> Well, adding a field or a message are both changes, just different.
>
> > > I don't think this new protocol is designed to support multiple
> > > clients.
> >
> > clipboard sharing isn't designed for multiple client either.
> >
> > >
> > > > 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
> >
> > ok
> >
> > >
> > > > 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.
> >
> > Conceptually, it's the "same" counter.
> >
>
> Okay, it was not clear.
>
> > >
> > > > 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
> > > > ---
> > > > 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.
> >
> > VDAgentClipboardGrabSelectionAndSerialAnd... ?
> >
>
> Usually a version schema is used like VDAgentClipboardGrabSelection_v2
>
> > yeah, it's not ideal. I wish we would use a better protocol, be it
> > json or protobuf etc..
> >
>
> This is not a justification to produce even worse code.
> A plain C structure would be fine, many binary protocols (like IP)
> uses this method.
> A semi-defined C structure with missing fields is the worst I
> ever seen, continuing to use is not so sensible.
Different trade-offs, version vs capabilities.
Since we don't have missing fields in the protocol, this means
VDAgentClipboardGrabSelection_v2 should have selection capability
included.
This may not be a problem for vdagent, but for many cases,
capabilities offer more flexibility than versioning.
> It would be better to not define the C structure at all.
> OT: I think it was discussed to use an external library/tool, somebody
> suggested to implement new messages with different serialization
> but nobody came with a RFC/proposal.
I guess we could start by having a gitlab issue to discuss it.
> >
> >
> > > Why not reusing part of __reserved?
> >
> > Because it's only 24 bits there, and if we make it too small, we would
> > have to deal explicitly with rounding issues.
> >
>
> I don't think a "(char)(remote_serial - local_serial) > 0" is so
> complicated to read.
I am not good at wrapped arithmetic. How would that work? for ex
(char)(255 - 100) == -101, that doesn't look right.
> But this is second to use a semi-defined structure, just a
> workaround using the already present ugly hack.
>
> > >
> > > > uint32_t types[0];
> > > > }
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
> Hi
>
> On Wed, Mar 27, 2019 at 8:23 AM Frediano Ziglio wrote:
> >
> > >
> > > From: Marc-André Lureau
> > >
> > > 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?
>
> This is like a new client-agent connection in this case, it starts again from
> 0.
>
> > I think you can achieve sending the serial with an additional separate
> > message at the beginning.
>
> I don't think it's necessary, but I am may be missing something.
>
Well, adding a field or a message are both changes, just different.
> > I don't think this new protocol is designed to support multiple
> > clients.
>
> clipboard sharing isn't designed for multiple client either.
>
> >
> > > 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
>
> ok
>
> >
> > > 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.
>
> Conceptually, it's the "same" counter.
>
Okay, it was not clear.
> >
> > > 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
> > > ---
> > > 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.
>
> VDAgentClipboardGrabSelectionAndSerialAnd... ?
>
Usually a version schema is used like VDAgentClipboardGrabSelection_v2
> yeah, it's not ideal. I wish we would use a better protocol, be it
> json or protobuf etc..
>
This is not a justification to produce even worse code.
A plain C structure would be fine, many binary protocols (like IP)
uses this method.
A semi-defined C structure with missing fields is the worst I
ever seen, continuing to use is not so sensible.
It would be better to not define the C structure at all.
OT: I think it was discussed to use an external library/tool, somebody
suggested to implement new messages with different serialization
but nobody came with a RFC/proposal.
>
>
> > Why not reusing part of __reserved?
>
> Because it's only 24 bits there, and if we make it too small, we would
> have to deal explicitly with rounding issues.
>
I don't think a "(char)(remote_serial - local_serial) > 0" is so
complicated to read.
But this is second to use a semi-defined structure, just a
workaround using the already present ugly hack.
> >
> > > 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.
>
> You mean without this request serial?
>
> How would you "legitimately steal" the client grab then?
>
>
> thanks
>
I think I lost a bit all the cases which I suppose are not much
documented.
Frediano
___
Spice-devel
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
Hi
On Wed, Mar 27, 2019 at 8:23 AM Frediano Ziglio wrote:
>
> >
> > From: Marc-André Lureau
> >
> > 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?
This is like a new client-agent connection in this case, it starts again from 0.
> I think you can achieve sending the serial with an additional separate
> message at the beginning.
I don't think it's necessary, but I am may be missing something.
> I don't think this new protocol is designed to support multiple
> clients.
clipboard sharing isn't designed for multiple client either.
>
> > 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
ok
>
> > 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.
Conceptually, it's the "same" counter.
>
> > 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
> > ---
> > 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.
VDAgentClipboardGrabSelectionAndSerialAnd... ?
yeah, it's not ideal. I wish we would use a better protocol, be it
json or protobuf etc..
> Why not reusing part of __reserved?
Because it's only 24 bits there, and if we make it too small, we would
have to deal explicitly with rounding issues.
>
> > 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.
You mean without this request serial?
How would you "legitimately steal" the client grab then?
thanks
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
>
> From: Marc-André Lureau
>
> 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
> ---
> 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
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
Hi,
On Sun, Mar 24, 2019 at 7:40 PM Marc-André Lureau
wrote:
>
> Hi
>
> On Sun, Mar 24, 2019 at 6:49 PM Jakub Janku wrote:
> >
> > On Fri, Mar 22, 2019 at 2:57 PM wrote:
> > >
> > > From: Marc-André Lureau
> > >
> > > When this capability is negoticated by both the client & the agent,
> > > 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.
> > >
> > > 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.
> >
> > Why should the client be the one with a higher priority?
>
> No strong reason, there has to be a winner in this case: both side are
> taking the grab simultaneously, we have to decide which one actually
> holds it in the end. Other ideas?
As I said - depending on the keyboard focus. However, this would
probably further complicate the protocol and so I have doubts whether
it's worth it, since this already handles a rather uncommon scenario.
Would love to hear opinions on who should be the "winner" in this case
from others too.
Apart from that, would be great if James tested it to see how his
environment behaves with these patches.
>
> >
> > If you look at James' case again:
> > with you proposed change, the clipboard manager would take over if a
> > race occurs, but the clipboard manager usually supports only a limited
> > number of targets.
> > For example, you copy a graph in Excel, (regrab and race occurs),
> > clipboard manager from the client wins and sets the clipboard, now
> > you're able to paste the graph only as a non-editable image.
>
> If there is a clipboard manager on the client side, it's the client
> desire to do such transformation, isn't it?
That's hard to say. I can imagine it could confuse some users while
others would appreciate it (e.g. in the case when they'd run some
automation systems in the client).
>
> >
> > So to provide an "uninterrupted" experience, I think that the
> > component with keyboard focus should actually get the priority.
>
> You are working against the clipboard manager in this case, which may
> do as much harm as good.
Correct, but you can still run another clipboard manager in the guest system.
>
> > >
> > > No special treatement is done for the unlikely case of overflowing the
> > > counter. It may temporarily inverse the priority, until both side have
> > > overflown and/or synchronized.
> > >
> > > 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
> > > ---
> > > 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
> > > 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,
> > > };
> > >
> > > --
> > > 2.21.0.4.g36eb1cb9cf
> > >
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>
> --
> Marc-André Lureau
Overall, I'm not against this implementation, but testing from someone
who's experienced the issues is a must, imho.
Cheers,
Jakub
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
Hi
On Sun, Mar 24, 2019 at 6:49 PM Jakub Janku wrote:
>
> On Fri, Mar 22, 2019 at 2:57 PM wrote:
> >
> > From: Marc-André Lureau
> >
> > When this capability is negoticated by both the client & the agent,
> > 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.
> >
> > 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.
>
> Why should the client be the one with a higher priority?
No strong reason, there has to be a winner in this case: both side are
taking the grab simultaneously, we have to decide which one actually
holds it in the end. Other ideas?
>
> If you look at James' case again:
> with you proposed change, the clipboard manager would take over if a
> race occurs, but the clipboard manager usually supports only a limited
> number of targets.
> For example, you copy a graph in Excel, (regrab and race occurs),
> clipboard manager from the client wins and sets the clipboard, now
> you're able to paste the graph only as a non-editable image.
If there is a clipboard manager on the client side, it's the client
desire to do such transformation, isn't it?
>
> So to provide an "uninterrupted" experience, I think that the
> component with keyboard focus should actually get the priority.
You are working against the clipboard manager in this case, which may
do as much harm as good.
> >
> > No special treatement is done for the unlikely case of overflowing the
> > counter. It may temporarily inverse the priority, until both side have
> > overflown and/or synchronized.
> >
> > 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
> > ---
> > 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
> > 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,
> > };
> >
> > --
> > 2.21.0.4.g36eb1cb9cf
> >
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice-protocol 3/3] vdagent: introduce VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL
On Fri, Mar 22, 2019 at 2:57 PM wrote:
>
> From: Marc-André Lureau
>
> When this capability is negoticated by both the client & the agent,
> 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.
>
> 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.
Why should the client be the one with a higher priority?
If you look at James' case again:
with you proposed change, the clipboard manager would take over if a
race occurs, but the clipboard manager usually supports only a limited
number of targets.
For example, you copy a graph in Excel, (regrab and race occurs),
clipboard manager from the client wins and sets the clipboard, now
you're able to paste the graph only as a non-editable image.
So to provide an "uninterrupted" experience, I think that the
component with keyboard focus should actually get the priority.
>
> No special treatement is done for the unlikely case of overflowing the
> counter. It may temporarily inverse the priority, until both side have
> overflown and/or synchronized.
>
> 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
> ---
> 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
> 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,
> };
>
> --
> 2.21.0.4.g36eb1cb9cf
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
