On Fri, 2017-03-24 at 11:49 +0100, Pavel Grunt wrote:
> Hi,
>
> looks good to me. How does it work on log out - it seems that in that
> case it kills the agent.
>
Yeah, for all other situations it works as it did before. So on logout
the agent will exit. This patch only changes the behavior when a client
disconnects.
Jonathon
> Pavel
>
> On Thu, 2017-03-16 at 10:08 -0500, Jonathon Jongsma wrote:
> > Previously, when a client disconnects, the vdagent executable
> > exited.
> > The agent was restarted immediately in order to be ready for the
> > next
> > client to reconnect.
> >
> > This is generally fine, but there are certain situations
> > where it causes issues. For example, if client A is connected to a
> > guest
> > and client B connects to the same guest, client A will be forcibly
> > disconnected and causes the the vdagent to restart. This scenario
> > is
> > racy because the agent can take some time to exit and restart.
> > Client B
> > may think that the agent is connected at startup and may send agent
> > messages to the guest. At some point the server will recieve
> > notification that the agent has exited and send an
> > AGENT_DISCONNECTED
> > message to client B. After the agent has been fully restarted, an
> > AGENT_CONNECTED message will be sent to the client, but any
> > messages
> > sent between client connection and the AGENT_DISCONNECTED message
> > will
> > be lost. This causes problems especially with fullscreen mode in
> > virt-viewer.
> >
> > The solution is to not exit and restart the agent when a client
> > disconnects. This is how the linux vdagent behaves. Instead, we
> > simply
> > cancel all ongoing file transfers and reset the clipboard when a
> > client
> > is disconnected.
> >
> > Fixes: rhbz#1064495
> > ---
> > vdagent/file_xfer.cpp | 20 +++++++++++++++-----
> > vdagent/file_xfer.h | 3 +++
> > vdagent/vdagent.cpp | 11 ++++++++---
> > 3 files changed, 26 insertions(+), 8 deletions(-)
> >
> > diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> > index 0e90ebe..f763ed3 100644
> > --- a/vdagent/file_xfer.cpp
> > +++ b/vdagent/file_xfer.cpp
> > @@ -31,17 +31,22 @@
> > #include "file_xfer.h"
> > #include "as_user.h"
> >
> > -FileXfer::~FileXfer()
> > +void FileXfer::reset()
> > {
> > FileXferTasks::iterator iter;
> > FileXferTask* task;
> >
> > for (iter = _tasks.begin(); iter != _tasks.end(); iter++) {
> > task = iter->second;
> > - CloseHandle(task->handle);
> > - DeleteFile(task->name);
> > + task->cancel();
> > delete task;
> > }
> > + _tasks.clear();
> > +}
> > +
> > +FileXfer::~FileXfer()
> > +{
> > + reset();
> > }
> >
> > void FileXfer::handle_start(VDAgentFileXferStartMessage* start,
> > @@ -152,6 +157,12 @@ fin:
> > return true;
> > }
> >
> > +void FileXferTask::cancel()
> > +{
> > + CloseHandle(handle);
> > + DeleteFile(name);
> > +}
> > +
> > void FileXfer::handle_status(VDAgentFileXferStatusMessage* status)
> > {
> > FileXferTasks::iterator iter;
> > @@ -168,8 +179,7 @@ void
> > FileXfer::handle_status(VDAgentFileXferStatusMessage* status)
> > return;
> > }
> > task = iter->second;
> > - CloseHandle(task->handle);
> > - DeleteFile(task->name);
> > + task->cancel();
> > _tasks.erase(iter);
> > delete task;
> > }
> > diff --git a/vdagent/file_xfer.h b/vdagent/file_xfer.h
> > index d9c65f3..25cd5c2 100644
> > --- a/vdagent/file_xfer.h
> > +++ b/vdagent/file_xfer.h
> > @@ -34,6 +34,8 @@ typedef struct ALIGN_VC FileXferTask {
> > uint64_t size;
> > uint64_t pos;
> > TCHAR name[MAX_PATH];
> > +
> > + void cancel();
> > } ALIGN_GCC FileXferTask;
> >
> > typedef std::map<uint32_t, FileXferTask*> FileXferTasks;
> > @@ -42,6 +44,7 @@ class FileXfer {
> > public:
> > ~FileXfer();
> > bool dispatch(VDAgentMessage* msg,
> > VDAgentFileXferStatusMessage* status);
> > + void reset();
> >
> > private:
> > void handle_start(VDAgentFileXferStartMessage* start,
> > VDAgentFileXferStatusMessage* status);
> > diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
> > index e3ba14b..c25a077 100644
> > --- a/vdagent/vdagent.cpp
> > +++ b/vdagent/vdagent.cpp
> > @@ -112,7 +112,7 @@ private:
> > DWORD get_cximage_format(uint32_t type) const;
> > enum { owner_none, owner_guest, owner_client };
> > void set_clipboard_owner(int new_owner);
> > - enum { CONTROL_STOP, CONTROL_DESKTOP_SWITCH, CONTROL_LOGON,
> > CONTROL_CLIPBOARD };
> > + enum { CONTROL_STOP, CONTROL_RESET, CONTROL_DESKTOP_SWITCH,
> > CONTROL_LOGON, CONTROL_CLIPBOARD };
> > void set_control_event(int control_command);
> > void handle_control_event();
> > VDIChunk* new_chunk(DWORD bytes = 0);
> > @@ -379,6 +379,11 @@ void VDAgent::handle_control_event()
> > _control_queue.pop();
> > vd_printf("Control command %d", control_command);
> > switch (control_command) {
> > + case CONTROL_RESET:
> > + _file_xfer.reset();
> > + set_control_event(CONTROL_CLIPBOARD);
> > + set_clipboard_owner(owner_none);
> > + break;
> > case CONTROL_STOP:
> > _running = false;
> > break;
> > @@ -1321,8 +1326,8 @@ void
> > VDAgent::dispatch_message(VDAgentMessage*
> > msg, uint32_t port)
> > break;
> > }
> > case VD_AGENT_CLIENT_DISCONNECTED:
> > - vd_printf("Client disconnected, agent to be restarted");
> > - set_control_event(CONTROL_STOP);
> > + vd_printf("Client disconnected, resetting agent state");
> > + set_control_event(CONTROL_RESET);
> > break;
> > case VD_AGENT_MAX_CLIPBOARD:
> > res = handle_max_clipboard((VDAgentMaxClipboard*)msg-
> > >data,
> > msg->size);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel