[Spice-devel] [PATCH 3/3] vdservice: stop service on virtio failure
Uri Lublin
uril at redhat.com
Sun Sep 23 08:21:39 PDT 2012
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;
More information about the Spice-devel
mailing list