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

Wed Aug 10 05:47:13 UTC 2016

Hi Frediano,

Thanks for your review!
This path is based on assumption that mouse mode is always client for as long as vdagent running.
As we know now this assumption is wrong, so we are calling this patch back for rework. 

Regards,
Dmitry

> On 9 Aug 2016, at 19:42 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
> 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 <mailto:fziglio at redhat.com>> wrote:
> 
> On Mon, Aug 8, 2016 at 6:47 PM, Frediano Ziglio <fziglio at redhat.com <mailto: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 <mailto: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 <mailto:Spice-devel at lists.freedesktop.org>
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel <https://lists.freedesktop.org/mailman/listinfo/spice-devel>
> 
> 
> 
> -- 
> Respectfully,
> Sameeh Jubran
> Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>
> Junior Software Engineer @ Daynix <http://www.daynix.com/>.
> 
> 
> 
> 
> -- 
> Respectfully,
> Sameeh Jubran
> Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>
> Junior Software Engineer @ Daynix <http://www.daynix.com/>.
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160810/5a294e59/attachment-0001.html>