On 09/16/2012 11:50 AM, Arnon Gilboa wrote:
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.
also removed _events_vdi_port_base field as part of a clean-up. Could
have been a separate patch.
Other than that, looks good.
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;
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel