[Spice-devel] [PATCH] Adding new ioctl for updating Vdagent state

[Frediano Ziglio]

The code is fine but the rationale lacks a bit. 
On Linux there is no such feature but not the bug so if your rationale is this is needed to fix a bug I don't understand why. 
Also the rationale assume the server or client pointer is all dependent on vdagent existence which seems not true. 
I was looking at Sandy (should download last WDDM) code and seems that EnablePointer is used in 3 cases 
- to reply cursor support (which is wrong, driver support cursor either way); 
- to avoid sending cursor shape changes (which seems not that fine); 
- to avoid sending cursor movements (which reduces guest -> server dialog for sure and perhaps server -> client if server propagate always). 

Frediano 

> Hi,

> What do you think that needs to be done?
> The patches should be applied?
> The interface of the device should be extended to support broadcasting the
> mouse mode to the driver? ( I think would require to modify the spice
> protocol )
> The interface of spice-gtk should be extended to support broadcasting the
> mouse mode to vdagent?

> On Mon, Aug 8, 2016 at 8:08 PM, Frediano Ziglio < fziglio at redhat.com > wrote:

> > > On Mon, Aug 8, 2016 at 6:47 PM, Frediano Ziglio < fziglio at redhat.com >
> > > wrote:
> > 
> 

> > > > > An optimal solution to this would be that the driver knows somehow
> > > > > when
> > > 
> > 
> 
> > > > > we are in a server mode and when we are in a client mode. However
> > > > > this
> > > 
> > 
> 
> > > > > information isn't available in the driver nor in the vdagent.
> > > 
> > 
> 

> > > > Could you explain why an optimal solution require this knowledge?
> > > 
> > 
> 
> > > > Also "However this information isn't available in the driver nor in the
> > > > vdagent."
> > > 
> > 
> 
> > > > does not make sense to me. The vdagent is sending this information to
> > > > the
> > > 
> > 
> 
> > > > driver so this information is available in vdagent.
> > > 
> > 
> 

> > > The current information the driver is getting is about Vdagent state
> > > (running/off), When
> > 
> 
> > > Vdagent is running then the client mouse should be enabled and when
> > > Vdagent
> > > is off
> > 
> 
> > > server mouse should be enabled. This is true for the typical use case of
> > > Vdagent.
> > 
> 
> > > But this isn't exactly true all of the time, for example when running
> > > Spicy
> > > along with Vdagent on
> > 
> 
> > > and we send a command to change the mouse mode to server mode we get that
> > > Vdagent state is on
> > 
> 
> > > and the mouse in server mode.
> > 
> 

> > Now I got it.
> 

> > So currently the driver bases it's behavior on vdagent existence but this
> > is
> > not correct
> 
> > as there are cases where agent is present but you want to used server
> > mouse.
> 

> > Looking at the rationale:
> 

> > "A new ioctl for updating the driver with vdagent running state.
> 
> > This patch adds new ioctl operation to Vdagent in order to update
> 
> > the driver on Vdagent state. This allows the driver to know
> 
> > when Vdagent is running and when it is off.
> 

> > Spice supports two mouse modes: server and client. The server mouse
> 
> > mode pointer should be enabled when vdagent is off and the client
> 
> > mouse mode should be enabled when it is on. The mouse mode
> 
> > is updated by the driver and thus this patch is needed."
> 

> > I still have a doubt. Is not the service (vdagentd in Linux) that speak
> 
> > to the driver? Potentially there are multiple agents, one for login.
> 

> > Also if you can tell client to use server mode in this case you
> 
> > end up having the agent running but server mode which
> 
> > seems wrong from your rationale.
> 

> > Also "adds new ioctl operation to Vdagent" the ioctl is implemented
> 
> > in the driver so I would say "adds new ioctl operation to the driver in
> 
> > order to allow Vdagent to communicate its state to the driver".
> 

> > Frediano
> 

> > > > Frediano
> > > 
> > 
> 

> > > > > On Mon, Aug 8, 2016 at 6:33 PM, Christophe Fergeau <
> > > > > cfergeau at redhat.com
> > > > > >
> > > 
> > 
> 
> > > > > wrote:
> > > 
> > 
> 

> > > > > > On Mon, Aug 08, 2016 at 06:19:41PM +0300, Sameeh Jubran wrote:
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 
> > > > > > > >
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 
> > > > > > > This patch enables the driver to send move commands to the QXL
> > > > > > > device
> > > 
> > 
> 
> > > > > > > when
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 
> > > > > > > vdagent is off.
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 
> > > > > > > This maybe the reason you are not getting any move commands.
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 

> > > > > > But don't we have exactly the same issue with a fedora guest or a
> > > > > > win7-
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 
> > > > > > guest? I've observed the same with a Fedora guest at least.
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 
> > > > > > Also I don't think there is a strict equivalence between vdagent is
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 
> > > > > > running and mouse is in server mode, so if we were to change this
> > > > > > at
> > > > > > the
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 
> > > > > > agent level, are we going to fix all cases?
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 

> > > > > > Christophe
> > > 
> > 
> 
> > > > >
> > > 
> > 
> 

> > > > > --
> > > 
> > 
> 
> > > > > Respectfully,
> > > 
> > 
> 
> > > > > Sameeh Jubran
> > > 
> > 
> 
> > > > > Linkedin
> > > 
> > 
> 
> > > > > Junior Software Engineer @ Daynix .
> > > 
> > 
> 

> > > > > _______________________________________________
> > > 
> > 
> 
> > > > > Spice-devel mailing list
> > > 
> > 
> 
> > > > > Spice-devel at lists.freedesktop.org
> > > 
> > 
> 
> > > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
> > 
> 

> > > --
> > 
> 
> > > Respectfully,
> > 
> 
> > > Sameeh Jubran
> > 
> 
> > > Linkedin
> > 
> 
> > > Junior Software Engineer @ Daynix .
> > 
> 

> --
> Respectfully,
> Sameeh Jubran
> Linkedin
> Junior Software Engineer @ Daynix .
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160809/dfc1169e/attachment.html>