[Spice-devel] [vdagent-windows v1] vdagent-win: check for locked session

Victor Toso lists at victortoso.com
Mon May 9 09:14:23 UTC 2016


Hi,

On Sat, May 07, 2016 at 09:59:09AM -0400, Frediano Ziglio wrote:
> A bit OT but on the same page.
>
> I used the drag&drop feature and it's quite useful.
> It's not however perfectly integrated. Would be really good if user could for instance
> drop from the client to a specific explorer window in the guest.
> I understand that this would be integrate in the shell in a quite more complicated way
> but for instance would possibly avoid also the lock problem as there will be no possible
> interaction with the shell.
>
> Question: is somebody "expert" on Windows shell extensions? Would be complicated to
> implement drag&drop in that way?
>
> Frediano

That's something we should have indeed. I also would like to have
something like the rich text copy & paste [0]. I think the best approach
is having the agent integrated with gtk so we can benefit from its API
[1] which AFAIK, works on windows.

[0] https://bugs.freedesktop.org/show_bug.cgi?id=91754
[1] https://developer.gnome.org/gtk3/stable/gtk3-Clipboards.html

I would like to know your opinion (and others as well) about integrating
the linux and windows agents in one code base, I'm not sure how
troublesome this could become.

For instance, both agents work differently. The linux system agent reads
the messages from virtio and redirects them to the session agent while
on windows, the session agent is the one that reads from virtio and the
system agent is more about start/stop/restart the session agent.. I'm
not sure if there is a reason for that.

In this patch, I realized that the windows API could be different from
session programs and for services. I'm newbie on windows but I'd really
like to have both agents in one codebase in the future, if that makes
sense.

  toso

>
> 
> > 
> > Hi,
> > 
> > On Fri, May 06, 2016 at 05:49:03PM +0200, Fabiano FidĂȘncio wrote:
> > > On Fri, May 6, 2016 at 5:35 PM, Victor Toso <victortoso at redhat.com> wrote:
> > > > Hi,
> > > >
> > > > On Fri, May 06, 2016 at 05:21:40PM +0200, Pavel Grunt wrote:
> > > >> Hey, it looks good. But according to docs, it will not work on WinXP
> > > >>
> > > >> https://msdn.microsoft.com/en-us/library/windows/desktop/aa383828%28v=vs.85%29.a
> > > >> spx
> > > >>
> > >
> > > Okay. OTOH you can find some documents mentioning the very same API
> > > when dealing specifically with Windows XP. Here is one example:
> > > https://support.microsoft.com/en-us/kb/310153 .. but you can find a
> > > few more if you google for it.
> > >
> > > >> Pavel
> > > >
> > > > Interesting, I did not see that. Thanks!
> > > >
> > > > I would still keep the patch as it is fairly simple and then make a
> > > > patch that could fix WinXP later on.
> > >
> > > Totally agreed. Actually, talking about _upstream_ _only_, makes no
> > > sense to try to find a solution for a dead OS.
> > 
> > Indeed.
> > 
> > > What I'd like to suggest is:
> > > - Test with Windows XP
> > > - Let us know about the result of your tests
> > > - In case there is an issue with Windows XP, just mention it in the commit
> > > log.
> > 
> > Sounds good, I'll do that.
> > Cheers,
> >   toso
> > >
> > > > The fix on WinXP could be done in
> > > > the vdservice, by parsing WTS_SESSION_LOCK/UNLOCK from
> > > > SERVICE_CONTROL_SESSIONCHANGE and then trying to avoid the file-xfer to
> > > > start in the vdagent.
> > > > What I'm missing is the communication between vdservice and vdagent
> > > > which is nothing like we have in the linux vdagent.
> > > >
> > > > Thanks for taking a look at this,
> > > >   toso
> > > >
> > > >>
> > > >> On Fri, 2016-05-06 at 13:50 +0200, Victor Toso wrote:
> > > >> > From WTSRegisterSessionNotification() we are aware of session changes
> > > >> > (WM_WTSSESSION_CHANGE) such as Lock/Unlock events.
> > > >> >
> > > >> > We can use that to disable features such as drag-and-drop.
> > > >> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1323628
> > > >> > ---
> > > >> >  vdagent/vdagent.cpp | 20 +++++++++++++++++++-
> > > >> >  1 file changed, 19 insertions(+), 1 deletion(-)
> > > >> >
> > > >> > diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
> > > >> > index c976665..a01bc70 100644
> > > >> > --- a/vdagent/vdagent.cpp
> > > >> > +++ b/vdagent/vdagent.cpp
> > > >> > @@ -146,6 +146,7 @@ private:
> > > >> >      uint32_t _in_msg_pos;
> > > >> >      bool _pending_input;
> > > >> >      bool _running;
> > > >> > +    bool _session_is_locked;
> > > >> >      bool _desktop_switch;
> > > >> >      DesktopLayout* _desktop_layout;
> > > >> >      bool _updating_display_config;
> > > >> > @@ -205,6 +206,7 @@ VDAgent::VDAgent()
> > > >> >      , _in_msg_pos (0)
> > > >> >      , _pending_input (false)
> > > >> >      , _running (false)
> > > >> > +    , _session_is_locked (false)
> > > >> >      , _desktop_switch (false)
> > > >> >      , _desktop_layout (NULL)
> > > >> >      , _display_setting (VD_AGENT_REGISTRY_KEY)
> > > >> > @@ -1282,7 +1284,19 @@ void VDAgent::dispatch_message(VDAgentMessage*
> > > >> > msg,
> > > >> > uint32_t port)
> > > >> >      case VD_AGENT_ANNOUNCE_CAPABILITIES:
> > > >> >          res =
> > > >> >          handle_announce_capabilities((VDAgentAnnounceCapabilities*)msg-
> > > >> > >data, msg->size);
> > > >> >          break;
> > > >> > -    case VD_AGENT_FILE_XFER_START:
> > > >> > +    case VD_AGENT_FILE_XFER_START: {
> > > >> > +        VDAgentFileXferStatusMessage status;
> > > >> > +        if (_session_is_locked) {
> > > >> > +            VDAgentFileXferStartMessage *s =
> > > >> > (VDAgentFileXferStartMessage
> > > >> > *)msg->data;
> > > >> > +            status.id = s->id;
> > > >> > +            status.result = VD_AGENT_FILE_XFER_STATUS_ERROR;
> > > >> > +            vd_printf("Fail to start file-xfer %u due: Locked
> > > >> > session",
> > > >> > status.id);
> > > >> > +            write_message(VD_AGENT_FILE_XFER_STATUS, sizeof(status),
> > > >> > &status);
> > > >> > +        } else if (_file_xfer.dispatch(msg, &status)) {
> > > >> > +            write_message(VD_AGENT_FILE_XFER_STATUS, sizeof(status),
> > > >> > &status);
> > > >> > +        }
> > > >> > +        break;
> > > >> > +    }
> > > >> >      case VD_AGENT_FILE_XFER_STATUS:
> > > >> >      case VD_AGENT_FILE_XFER_DATA: {
> > > >> >          VDAgentFileXferStatusMessage status;
> > > >> > @@ -1489,6 +1503,10 @@ LRESULT CALLBACK VDAgent::wnd_proc(HWND hwnd,
> > > >> > UINT
> > > >> > message, WPARAM wparam, LPARA
> > > >> >      case WM_WTSSESSION_CHANGE:
> > > >> >          if (wparam == WTS_SESSION_LOGON) {
> > > >> >              a->set_control_event(CONTROL_LOGON);
> > > >> > +        } else if (wparam == WTS_SESSION_LOCK) {
> > > >> > +            a->_session_is_locked = true;
> > > >> > +        } else if (wparam == WTS_SESSION_UNLOCK) {
> > > >> > +            a->_session_is_locked = false;
> > > >> >          }
> > > >> >          break;
> > > >> >      default:
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
> > > Best Regards,
> > > --
> > > Fabiano FidĂȘncio
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 


More information about the Spice-devel mailing list