[Spice-devel] [PATCH 3/3] vdservice: stop service on virtio failure

Arnon Gilboa agilboa at redhat.com
Sun Sep 16 01:50:15 PDT 2012


read & write are async, and their completion is handled by handle_event(),
which returns status used by service execute loop.
previously an error in GetOverlappedResult caused vdservice hang.

rhbz#839564
---
 vdservice/pci_vdi_port.cpp    |    3 ++-
 vdservice/pci_vdi_port.h      |    2 +-
 vdservice/vdi_port.h          |    2 +-
 vdservice/vdservice.cpp       |   13 +++++--------
 vdservice/virtio_vdi_port.cpp |   24 +++++++++++++++---------
 vdservice/virtio_vdi_port.h   |    6 +++---
 6 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/vdservice/pci_vdi_port.cpp b/vdservice/pci_vdi_port.cpp
index 4deace1..7466fbc 100644
--- a/vdservice/pci_vdi_port.cpp
+++ b/vdservice/pci_vdi_port.cpp
@@ -124,8 +124,9 @@ int PCIVDIPort::read()
     return n;
 }
 
-void PCIVDIPort::handle_event(int event)
+bool PCIVDIPort::handle_event(int event)
 {
     // do nothing - the event merely serves to wake us up, then we call read/write
     // at VDService::execute start of while(_running) loop.
+    return true;
 }
diff --git a/vdservice/pci_vdi_port.h b/vdservice/pci_vdi_port.h
index caa990f..fcc76dc 100644
--- a/vdservice/pci_vdi_port.h
+++ b/vdservice/pci_vdi_port.h
@@ -41,7 +41,7 @@ public:
     virtual int read();
     virtual unsigned get_num_events() { return PCI_VDI_PORT_EVENT_COUNT; }
     virtual void fill_events(HANDLE* handles);
-    virtual void handle_event(int event);
+    virtual bool handle_event(int event);
 
 private:
     HANDLE _handle;
diff --git a/vdservice/vdi_port.h b/vdservice/vdi_port.h
index 50c4d29..a0fb20e 100644
--- a/vdservice/vdi_port.h
+++ b/vdservice/vdi_port.h
@@ -61,7 +61,7 @@ public:
     virtual bool init() = 0;
     virtual unsigned get_num_events() = 0;
     virtual void fill_events(HANDLE* handles) = 0;
-    virtual void handle_event(int event) = 0;
+    virtual bool handle_event(int event) = 0;
     virtual int write() = 0;
     virtual int read() = 0;
 
diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
index b48cbeb..2b925fd 100644
--- a/vdservice/vdservice.cpp
+++ b/vdservice/vdservice.cpp
@@ -130,7 +130,6 @@ private:
     bool _running;
     VDLog* _log;
     unsigned _events_count;
-    unsigned _events_vdi_port_base;
 };
 
 VDService* VDService::_singleton = NULL;
@@ -185,7 +184,6 @@ VDService::VDService()
     , _running (false)
     , _log (NULL)
     , _events_count(0)
-    , _events_vdi_port_base(0)
 {
     ZeroMemory(&_agent_proc_info, sizeof(_agent_proc_info));
     ZeroMemory(&_pipe_state, sizeof(_pipe_state));
@@ -536,13 +534,12 @@ bool VDService::execute()
     vd_printf("created %s", _vdi_port->name());
     _events_count = VD_STATIC_EVENTS_COUNT + _vdi_port->get_num_events() + 1 /*for agent*/;
     _events = new HANDLE[_events_count];
-    _events_vdi_port_base = VD_STATIC_EVENTS_COUNT;
     ZeroMemory(_events, _events_count);
     vd_printf("Connected to server");
     _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;
-    _vdi_port->fill_events(&_events[_events_vdi_port_base]);
+    _vdi_port->fill_events(&_events[VD_STATIC_EVENTS_COUNT]);
     _chunk_size = _chunk_port = 0;
     read_pipe();
     while (_running) {
@@ -602,12 +599,12 @@ bool VDService::execute()
                         }
                     }
                 } else {
-                    if (wait_ret >= WAIT_OBJECT_0 + _events_vdi_port_base &&
-                        wait_ret < WAIT_OBJECT_0 +
-                                   _events_vdi_port_base + _vdi_port->get_num_events()) {
-                        _vdi_port->handle_event(wait_ret - VD_STATIC_EVENTS_COUNT - WAIT_OBJECT_0);
+                    int vdi_event = wait_ret - VD_STATIC_EVENTS_COUNT - WAIT_OBJECT_0;
+                    if (vdi_event >= 0 && vdi_event < _vdi_port->get_num_events()) {
+                        _running = _vdi_port->handle_event(vdi_event);
                     } else {
                         vd_printf("WaitForMultipleObjects failed %lu", GetLastError());
+                        _running = false;
                     }
                 }
             }
diff --git a/vdservice/virtio_vdi_port.cpp b/vdservice/virtio_vdi_port.cpp
index 92eb129..be5568a 100644
--- a/vdservice/virtio_vdi_port.cpp
+++ b/vdservice/virtio_vdi_port.cpp
@@ -51,17 +51,21 @@ void VirtioVDIPort::fill_events(HANDLE* handles) {
     handles[VIRTIO_VDI_PORT_EVENT_READ] = _read.overlap.hEvent;
 }
 
-void VirtioVDIPort::handle_event(int event) {
+bool VirtioVDIPort::handle_event(int event) {
+    bool ret;
+
     switch (event) {
         case VIRTIO_VDI_PORT_EVENT_WRITE:
-            write_completion();
+            ret = write_completion();
             break;
         case VIRTIO_VDI_PORT_EVENT_READ:
-            read_completion();
+            ret = read_completion();
             break;
         default:
             vd_printf("ERROR: unexpected event %d", event);
+            ret = false;
     }
+    return ret;
 }
 
 bool VirtioVDIPort::init()
@@ -113,20 +117,21 @@ int VirtioVDIPort::write()
     return ret;
 }
 
-void VirtioVDIPort::write_completion()
+bool VirtioVDIPort::write_completion()
 {
     DWORD bytes;
 
     if (!_write.pending) {
-        return;
+        return true;
     }
     if (!GetOverlappedResult(_handle, &_write.overlap, &bytes, FALSE)) {
         vd_printf("GetOverlappedResult failed: %lu", GetLastError());
-        return;
+        return false;
     }
     _write.start = _write.ring + (_write.start - _write.ring + bytes) % BUF_SIZE;
     _write.bytes = bytes;
     _write.pending = false;
+    return true;
 }
 
 int VirtioVDIPort::read()
@@ -160,7 +165,7 @@ int VirtioVDIPort::read()
     return ret;
 }
 
-void VirtioVDIPort::read_completion()
+bool VirtioVDIPort::read_completion()
 {
     DWORD bytes;
 
@@ -169,13 +174,14 @@ void VirtioVDIPort::read_completion()
 
         if (err == ERROR_OPERATION_ABORTED || err == ERROR_NO_SYSTEM_RESOURCES) {
             _read.pending = false;
-            return;
+            return true;
         } else if (err != ERROR_MORE_DATA) {
             vd_printf("GetOverlappedResult failed: %lu", err);
-            return;
+            return false;
         }
     }
     _read.end = _read.ring + (_read.end - _read.ring + bytes) % BUF_SIZE;
     _read.bytes = bytes;
     _read.pending = false;
+    return true;
 }
diff --git a/vdservice/virtio_vdi_port.h b/vdservice/virtio_vdi_port.h
index 15b6811..d72edf4 100644
--- a/vdservice/virtio_vdi_port.h
+++ b/vdservice/virtio_vdi_port.h
@@ -17,13 +17,13 @@ public:
     virtual bool init();
     virtual unsigned get_num_events() { return VIRTIO_VDI_PORT_EVENT_COUNT; }
     virtual void fill_events(HANDLE* handles);
-    virtual void handle_event(int event);
+    virtual bool handle_event(int event);
     virtual int write();
     virtual int read();
 
 private:
-    void write_completion();
-    void read_completion();
+    bool write_completion();
+    bool read_completion();
 
 private:
     static VirtioVDIPort* _singleton;
-- 
1.7.4.1



More information about the Spice-devel mailing list