[Spice-commits] 6 commits - vdagent/desktop_layout.cpp vdagent/vdagent.cpp vdservice/vdservice.cpp

Arnon Gilboa agilboa at kemper.freedesktop.org
Sun Jul 24 08:22:47 PDT 2011


 vdagent/desktop_layout.cpp |    2 
 vdagent/vdagent.cpp        |    8 +-
 vdservice/vdservice.cpp    |  148 +++++++++++++++++++++++++++++----------------
 3 files changed, 102 insertions(+), 56 deletions(-)

New commits:
commit c28d7f02744d834812bac000715975d9416c2da2
Author: Arnon Gilboa <agilboa at redhat.com>
Date:   Sun Jul 24 13:24:28 2011 +0300

    vdagent: add casting

diff --git a/vdagent/desktop_layout.cpp b/vdagent/desktop_layout.cpp
index 9bd1852..0eada52 100644
--- a/vdagent/desktop_layout.cpp
+++ b/vdagent/desktop_layout.cpp
@@ -67,7 +67,7 @@ void DesktopLayout::get_displays()
         }
         size_t size = _displays.size();
         if (!wcsstr(dev_info.DeviceString, L"QXL")) {
-            display_id = size;
+            display_id = (DWORD)size;
         } else if (!get_qxl_device_id(dev_info.DeviceKey, &display_id)) {
             vd_printf("get_qxl_device_id failed %S", dev_info.DeviceKey);
             break;
commit d896c004e58590698a61e6fda57cbe225262ca50
Author: Arnon Gilboa <agilboa at redhat.com>
Date:   Sun Jul 24 13:24:15 2011 +0300

    vdagent: remove whitespaces

diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
index 1ebf1e4..646afed 100644
--- a/vdagent/vdagent.cpp
+++ b/vdagent/vdagent.cpp
@@ -565,11 +565,11 @@ bool VDAgent::handle_clipboard(VDAgentClipboard* clipboard, uint32_t size)
 
 HGLOBAL VDAgent::utf8_alloc(LPCSTR data, int size)
 {
-    HGLOBAL handle; 
+    HGLOBAL handle;
     LPVOID buf;
     int len;
 
-    // Received utf8 string is not null-terminated   
+    // Received utf8 string is not null-terminated
     if (!(len = MultiByteToWideChar(CP_UTF8, 0, data, size, NULL, 0))) {
         return NULL;
     }
@@ -581,7 +581,7 @@ HGLOBAL VDAgent::utf8_alloc(LPCSTR data, int size)
     if (!(buf = GlobalLock(handle))) {
         GlobalFree(handle);
         return NULL;
-    }  
+    }
     // Translate data and set clipboard content
     if (!(MultiByteToWideChar(CP_UTF8, 0, data, size, (LPWSTR)buf, len))) {
         GlobalUnlock(handle);
@@ -1027,7 +1027,7 @@ uint32_t VDAgent::get_clipboard_format(uint32_t type)
     for (int i = 0; i < clipboard_formats_count; i++) {
         for (uint32_t* ptype = clipboard_formats[i].types; *ptype; ptype++) {
             if (*ptype == type) {
-                return clipboard_formats[i].format; 
+                return clipboard_formats[i].format;
             }
         }
     }
commit 5edb588d6feec57c281ad8b652d9fae1a9fdedb8
Author: Arnon Gilboa <agilboa at redhat.com>
Date:   Sun Jul 24 13:23:27 2011 +0300

    vdservice: extract init_vdi_port()

diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
index 94fe361..b382a6e 100644
--- a/vdservice/vdservice.cpp
+++ b/vdservice/vdservice.cpp
@@ -80,6 +80,7 @@ private:
     static DWORD WINAPI control_handler(DWORD control, DWORD event_type,
                                         LPVOID event_data, LPVOID context);
     static VOID WINAPI main(DWORD argc, TCHAR * argv[]);
+    bool init_vdi_port();
     void set_control_event(int control_command);
     void handle_control_event();
     void pipe_write_completion();
@@ -454,6 +455,20 @@ VDIPort *create_pci_vdi_port()
     return new PCIVDIPort();
 }
 
+bool VDService::init_vdi_port()
+{
+    VDIPort* (*creators[])(void) = { create_virtio_vdi_port, create_pci_vdi_port };
+
+    for (int i = 0 ; i < sizeof(creators)/sizeof(creators[0]); ++i) {
+        _vdi_port = creators[i]();
+        if (_vdi_port->init()) {
+            return true;
+        }
+        delete _vdi_port;
+    }
+    return false;
+}
+
 bool VDService::execute()
 {
     SECURITY_ATTRIBUTES sec_attr;
@@ -488,19 +503,7 @@ bool VDService::execute()
         return false;
     }
 
-    bool init = false;
-    {
-        VDIPort* (*creators[])(void) = { create_virtio_vdi_port, create_pci_vdi_port };
-        for (int i = 0 ; i < sizeof(creators)/sizeof(creators[0]); ++i) {
-            _vdi_port = creators[i]();
-            init = _vdi_port->init();
-            if (init) {
-                break;
-            }
-            delete _vdi_port;
-        }
-    }
-    if (!init) {
+    if (!init_vdi_port()) {
         vd_printf("Failed to create VDIPort instance");
         CloseHandle(pipe);
         return false;
commit 407acb2288e48281cf1c60c70608f5838ac97c91
Author: Arnon Gilboa <agilboa at redhat.com>
Date:   Sun Jul 24 13:20:50 2011 +0300

    vdservice: clean agent proc handle in kill_agent()
    
    fill_agent_events() will not add it to the events used by WaitFor(),
    preventing usage of a dead process handle.

diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
index b6c9234..94fe361 100644
--- a/vdservice/vdservice.cpp
+++ b/vdservice/vdservice.cpp
@@ -904,21 +904,24 @@ bool VDService::kill_agent()
 {
     DWORD exit_code = 0;
     DWORD wait_ret;
+    HANDLE proc_handle;
     bool ret = true;
 
     if (!_agent_alive) {
         return true;
     }
     _agent_alive = false;
+    proc_handle = _agent_proc_info.hProcess;
+    _agent_proc_info.hProcess = 0;
     if (_pipe_connected) {
         _pipe_connected = false;
         DisconnectNamedPipe(_pipe_state.pipe);
     }
-    if (GetProcessId(_agent_proc_info.hProcess)) {
-        wait_ret = WaitForSingleObject(_agent_proc_info.hProcess, 3000);
+    if (GetProcessId(proc_handle)) {
+        wait_ret = WaitForSingleObject(proc_handle, 3000);
         switch (wait_ret) {
         case WAIT_OBJECT_0:
-            if (GetExitCodeProcess(_agent_proc_info.hProcess, &exit_code)) {
+            if (GetExitCodeProcess(proc_handle, &exit_code)) {
                 vd_printf("vdagent exit code %u", exit_code);
             } else if (exit_code == STILL_ACTIVE) {
                 vd_printf("Failed killing vdagent");
@@ -937,7 +940,7 @@ bool VDService::kill_agent()
             break;
         }
     }
-    CloseHandle(_agent_proc_info.hProcess);
+    CloseHandle(proc_handle);
     CloseHandle(_agent_proc_info.hThread);
     ZeroMemory(&_agent_proc_info, sizeof(_agent_proc_info));
     return ret;
commit e0a75c9042eca4aa6e013aa05c2603f5cf156f57
Author: Arnon Gilboa <agilboa at redhat.com>
Date:   Sun Jul 24 13:18:33 2011 +0300

    vdservice: use overlap ConnectNamedPipe() in launch_agent()
    
    passing NULL is buggy when pipe opened with FILE_FLAG_OVERLAPPED

diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
index c270ca8..b6c9234 100644
--- a/vdservice/vdservice.cpp
+++ b/vdservice/vdservice.cpp
@@ -836,6 +836,7 @@ BOOL create_process_as_user(IN DWORD session_id, IN LPCWSTR application_name,
 bool VDService::launch_agent()
 {
     STARTUPINFO startup_info;
+    OVERLAPPED overlap;
     BOOL ret = FALSE;
 
     ZeroMemory(&startup_info, sizeof(startup_info));
@@ -876,15 +877,27 @@ bool VDService::launch_agent()
         return false;
     }
     vd_printf("Wait for vdagent to connect");
-    if (ConnectNamedPipe(_pipe_state.pipe, NULL) || GetLastError() == ERROR_PIPE_CONNECTED) {
+    ZeroMemory(&overlap, sizeof(overlap));
+    overlap.hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
+    DWORD err = (ConnectNamedPipe(_pipe_state.pipe, &overlap) ? 0 : GetLastError());
+    if (err = ERROR_IO_PENDING) {
+        DWORD wait_ret = WaitForSingleObject(overlap.hEvent, 3000);
+        if (wait_ret != WAIT_OBJECT_0) {
+            vd_printf("WaitForSingleObject() failed: %u error: %u", wait_ret,
+                wait_ret == WAIT_FAILED ? GetLastError() : 0);
+            ret = FALSE;
+        }
+    } else if (err != 0 || err != ERROR_PIPE_CONNECTED) {
+        vd_printf("ConnectNamedPipe() failed: %u", err);
+        ret = FALSE;
+    }
+    if (ret) {
+        vd_printf("Pipe connected by vdagent");
         _pipe_connected = true;
         _pending_reset = false;
-        vd_printf("Pipe connected by vdagent");
-    } else {
-        vd_printf("ConnectNamedPipe() failed: %u", GetLastError());
-        return false;
     }
-    return true;
+    CloseHandle(overlap.hEvent);
+    return !!ret;
 }
 
 bool VDService::kill_agent()
commit f1bc45e53b62a6307267fc016a6e291fe30900ea
Author: Arnon Gilboa <agilboa at redhat.com>
Date:   Sun Jul 24 12:44:00 2011 +0300

    vdservice: add control events RHBZ #719140 #722980
    
    -prevent race between service control manager (SCM) & the service main thread.
    -use events for stop, logon, agent restart.
    -thread-safe control command queue

diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
index 32c0ca5..c270ca8 100644
--- a/vdservice/vdservice.cpp
+++ b/vdservice/vdservice.cpp
@@ -21,6 +21,7 @@
 #include <userenv.h>
 #include <stdio.h>
 #include <tlhelp32.h>
+#include <queue>
 #include "vdcommon.h"
 #include "virtio_vdi_port.h"
 #include "pci_vdi_port.h"
@@ -52,10 +53,18 @@ enum {
     VD_EVENT_PIPE_READ = 0,
     VD_EVENT_PIPE_WRITE,
     VD_EVENT_CONTROL,
-    VD_EVENT_LOGON,
     VD_STATIC_EVENTS_COUNT // Must be last
 };
 
+enum {
+    VD_CONTROL_IDLE = 0,
+    VD_CONTROL_STOP,
+    VD_CONTROL_LOGON,
+    VD_CONTROL_RESTART_AGENT,
+};
+
+typedef std::queue<int> VDControlQueue;
+
 class VDService {
 public:
     static VDService* get();
@@ -71,6 +80,8 @@ private:
     static DWORD WINAPI control_handler(DWORD control, DWORD event_type,
                                         LPVOID event_data, LPVOID context);
     static VOID WINAPI main(DWORD argc, TCHAR * argv[]);
+    void set_control_event(int control_command);
+    void handle_control_event();
     void pipe_write_completion();
     void write_agent_control(uint32_t type, uint32_t opaque);
     void read_pipe();
@@ -79,7 +90,6 @@ private:
     bool handle_agent_control(VDPipeMessage* msg);
     bool restart_agent(bool normal_restart);
     bool launch_agent();
-    void send_logon();
     bool kill_agent();
     unsigned fill_agent_event() {
         ASSERT(_events);
@@ -96,13 +106,14 @@ private:
     SERVICE_STATUS_HANDLE _status_handle;
     PROCESS_INFORMATION _agent_proc_info;
     HANDLE _control_event;
-    HANDLE _logon_event;
     HANDLE* _events;
     TCHAR _agent_path[MAX_PATH];
     VDIPort* _vdi_port;
     VDPipeState _pipe_state;
-    uint32_t _connection_id;
+    VDControlQueue _control_queue;
+    mutex_t _control_mutex;
     mutex_t _agent_mutex;
+    uint32_t _connection_id;
     DWORD _session_id;
     DWORD _chunk_port;
     DWORD _chunk_size;
@@ -180,15 +191,14 @@ VDService::VDService()
     _control_event = CreateEvent(NULL, FALSE, FALSE, NULL);
     _pipe_state.write.overlap.hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
     _pipe_state.read.overlap.hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
-    _logon_event = CreateEvent(NULL, FALSE, FALSE, NULL);
     _agent_path[0] = wchar_t('\0');
     MUTEX_INIT(_agent_mutex);
+    MUTEX_INIT(_control_mutex);
     _singleton = this;
 }
 
 VDService::~VDService()
 {
-    CloseHandle(_logon_event);
     CloseHandle(_pipe_state.read.overlap.hEvent);
     CloseHandle(_pipe_state.write.overlap.hEvent);
     CloseHandle(_control_event);
@@ -293,6 +303,41 @@ const char* session_events[] = {
     "LOCK", "UNLOCK", "REMOTE_CONTROL"
 };
 
+void VDService::set_control_event(int control_command)
+{
+    MUTEX_LOCK(_control_mutex);
+    _control_queue.push(control_command);
+    if (_control_event && !SetEvent(_control_event)) {
+        vd_printf("SetEvent() failed: %u", GetLastError());
+    }
+    MUTEX_UNLOCK(_control_mutex);
+}
+
+void VDService::handle_control_event()
+{
+    MUTEX_LOCK(_control_mutex);
+    while (_control_queue.size()) {
+        int control_command = _control_queue.front();
+        _control_queue.pop();
+        vd_printf("Control command %d", control_command);
+        switch (control_command) {
+        case VD_CONTROL_STOP:
+            _running = false;
+            break;
+        case VD_CONTROL_LOGON:
+            write_agent_control(VD_AGENT_SESSION_LOGON, 0);
+            break;
+        case VD_CONTROL_RESTART_AGENT:
+            _running = restart_agent(true);
+            break;
+        default:
+            vd_printf("Unsupported control command %u", control_command);
+        }
+    }
+    MUTEX_UNLOCK(_control_mutex);
+
+}
+
 DWORD WINAPI VDService::control_handler(DWORD control, DWORD event_type, LPVOID event_data,
                                         LPVOID context)
 {
@@ -319,11 +364,9 @@ DWORD WINAPI VDService::control_handler(DWORD control, DWORD event_type, LPVOID
         if (s->_system_version != SYS_VER_UNSUPPORTED) {
             if (event_type == WTS_CONSOLE_CONNECT) {
                 s->_session_id = session_id;
-                if (!s->restart_agent(true)) {
-                    s->stop();
-               }
+                s->set_control_event(VD_CONTROL_RESTART_AGENT);
             } else if (event_type == WTS_SESSION_LOGON) {
-                s->send_logon();
+                s->set_control_event(VD_CONTROL_LOGON);
             }
         }
         break;
@@ -471,7 +514,6 @@ bool VDService::execute()
     _events[VD_EVENT_PIPE_READ] = _pipe_state.read.overlap.hEvent;
     _events[VD_EVENT_PIPE_WRITE] = _pipe_state.write.overlap.hEvent;
     _events[VD_EVENT_CONTROL] = _control_event;
-    _events[VD_EVENT_LOGON] = _logon_event;
     _agent_proc_info.hProcess;
     _vdi_port->fill_events(&_events[_events_vdi_port_base]);
     _chunk_size = _chunk_port = 0;
@@ -523,11 +565,7 @@ bool VDService::execute()
                 pipe_write_completion();
                 break;
             case WAIT_OBJECT_0 + VD_EVENT_CONTROL:
-                vd_printf("Control event");
-                break;
-            case WAIT_OBJECT_0 + VD_EVENT_LOGON:
-                vd_printf("logon event");
-                write_agent_control(VD_AGENT_SESSION_LOGON, 0);
+                handle_control_event();
                 break;
             case WAIT_TIMEOUT:
                 break;
@@ -918,17 +956,7 @@ bool VDService::restart_agent(bool normal_restart)
 void VDService::stop()
 {
     vd_printf("Service stopped");
-    _running = false;
-    if (_control_event && !SetEvent(_control_event)) {
-        vd_printf("SetEvent() failed: %u", GetLastError());
-    }
-}
-
-void VDService::send_logon()
-{
-    if (_logon_event && !SetEvent(_logon_event)) {
-        vd_printf("SetEvent() failed: %u", GetLastError());
-    }
+    set_control_event(VD_CONTROL_STOP);
 }
 
 void VDService::pipe_write_completion()
@@ -1154,4 +1182,3 @@ int _tmain(int argc, TCHAR* argv[], TCHAR* envp[])
     delete vdservice;
     return (success ? 0 : -1);
 }
-


More information about the Spice-commits mailing list