| Message ID | 1300290769-31155-5-git-send-email-alevy@redhat.com |
|---|---|
| State | New |
| Headers | show |
Looks good now, ack: Acked-by: Hans de Goede <hdegoede@redhat.com> On 03/16/2011 04:52 PM, Alon Levy wrote: > Switching locking protection of ds->cursor_set/cursor_move to moving > every call to these functions into the iothread and using the ssd->pipe > to transfer that, adding QXL_SERVER_CURSOR_SET, QXL_SERVER_CURSOR_MOVE. > > This is tested with both -vnc :0 -spice and -sdl -spice. > --- > hw/qxl-render.c | 25 +++++++++----- > hw/qxl.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/qxl.h | 4 ++ > ui/spice-display.h | 13 +++++++- > 4 files changed, 122 insertions(+), 10 deletions(-) > > diff --git a/hw/qxl-render.c b/hw/qxl-render.c > index 58965e0..6759edb 100644 > --- a/hw/qxl-render.c > +++ b/hw/qxl-render.c > @@ -178,7 +178,6 @@ fail: > return NULL; > } > > - > /* called from spice server thread context only */ > void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext) > { > @@ -209,18 +208,26 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext) > if (c == NULL) { > c = cursor_builtin_left_ptr(); > } > - qemu_mutex_lock_iothread(); > - qxl->ssd.ds->cursor_define(c); > - qxl->ssd.ds->mouse_set(x, y, 1); > - qemu_mutex_unlock_iothread(); > - cursor_put(c); > + qxl_server_request_cursor_set(qxl, c, x, y); > break; > case QXL_CURSOR_MOVE: > x = cmd->u.position.x; > y = cmd->u.position.y; > - qemu_mutex_lock_iothread(); > - qxl->ssd.ds->mouse_set(x, y, 1); > - qemu_mutex_unlock_iothread(); > + qxl_server_request_cursor_move(qxl, x, y); > break; > } > } > + > +/* called from iothread only (via qxl.c:pipe_read) */ > +void qxl_render_cursor_set(SimpleSpiceDisplay *ssd, QEMUCursor *c, int x, int y) > +{ > + ssd->ds->cursor_define(c); > + ssd->ds->mouse_set(x, y, 1); > + cursor_put(c); > +} > + > +/* called from iothread only (via qxl.c:pipe_read) */ > +void qxl_render_cursor_move(SimpleSpiceDisplay *ssd, int x, int y) > +{ > + ssd->ds->mouse_set(x, y, 1); > +} > diff --git a/hw/qxl.c b/hw/qxl.c > index cf3c938..4c27deb 100644 > --- a/hw/qxl.c > +++ b/hw/qxl.c > @@ -117,6 +117,27 @@ static QXLMode qxl_modes[] = { > #endif > }; > > +typedef struct __attribute__ ((__packed__)) { > + QEMUCursor *c; > + int x; > + int y; > +} QXLServerCursorSet; > + > +typedef struct __attribute__ ((__packed__)) { > + int x; > + int y; > +} QXLServerCursorMove; > + > +typedef struct __attribute__ ((__packed__)) { > + unsigned char req; > + QXLServerCursorMove data; > +} QXLServerCursorMoveRequest; > + > +typedef struct __attribute__ ((__packed__)) { > + unsigned char req; > + QXLServerCursorSet data; > +} QXLServerCursorSetRequest; > + > static PCIQXLDevice *qxl0; > > static void qxl_send_events(PCIQXLDevice *d, uint32_t events); > @@ -337,6 +358,33 @@ static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info) > } > > /* called from spice server thread context only */ > +void qxl_server_request_cursor_set(PCIQXLDevice *qxl, QEMUCursor *c, int x, int y) > +{ > + QXLServerCursorSetRequest req; > + int r; > + > + req.req = QXL_SERVER_CURSOR_SET; > + req.data.c = c; > + req.data.x = x; > + req.data.y = y; > + r = write(qxl->ssd.pipe[1],&req, sizeof(req)); > + assert(r == sizeof(req)); > +} > + > +/* called from spice server thread context only */ > +void qxl_server_request_cursor_move(PCIQXLDevice *qxl, int x, int y) > +{ > + QXLServerCursorMoveRequest req; > + int r; > + > + req.req = QXL_SERVER_CURSOR_MOVE; > + req.data.x = x; > + req.data.y = y; > + r = write(qxl->ssd.pipe[1],&req, sizeof(req)); > + assert(r == sizeof(req)); > +} > + > +/* called from spice server thread context only */ > static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) > { > PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl); > @@ -1055,12 +1103,37 @@ static void qxl_map(PCIDevice *pci, int region_num, > } > } > > +static void read_bytes(int fd, void *buf, int len_requested) > +{ > + int len; > + int total_len = 0; > + > + do { > + len = read(fd, buf, len_requested - total_len); > + if (len< 0) { > + if (errno == EINTR || errno == EAGAIN) { > + continue; > + } > + perror("qxl: pipe_read: read failed"); > + /* will abort once it's out of the while loop */ > + break; > + } > + total_len += len; > + buf = (uint8_t *)buf + len; > + } while (total_len< len_requested); > + assert(total_len == len_requested); > +} > + > static void pipe_read(void *opaque) > { > SimpleSpiceDisplay *ssd = opaque; > unsigned char cmd; > int len, set_irq = 0; > int create_update = 0; > + int cursor_set = 0; > + int cursor_move = 0; > + QXLServerCursorSet cursor_set_data; > + QXLServerCursorMove cursor_move_data; > > while (1) { > cmd = 0; > @@ -1082,6 +1155,17 @@ static void pipe_read(void *opaque) > case QXL_SERVER_CREATE_UPDATE: > create_update = 1; > break; > + case QXL_SERVER_CURSOR_SET: > + if (cursor_set == 1) { > + cursor_put(cursor_set_data.c); > + } > + cursor_set = 1; > + read_bytes(ssd->pipe[0],&cursor_set_data, sizeof(cursor_set_data)); > + break; > + case QXL_SERVER_CURSOR_MOVE: > + cursor_move = 1; > + read_bytes(ssd->pipe[0],&cursor_move_data, sizeof(cursor_move_data)); > + break; > default: > fprintf(stderr, "%s: unknown cmd %u\n", __FUNCTION__, cmd); > abort(); > @@ -1099,6 +1183,12 @@ static void pipe_read(void *opaque) > if (set_irq) { > qxl_set_irq(container_of(ssd, PCIQXLDevice, ssd)); > } > + if (cursor_move) { > + qxl_render_cursor_move(ssd, cursor_move_data.x, cursor_move_data.y); > + } > + if (cursor_set) { > + qxl_render_cursor_set(ssd, cursor_set_data.c, cursor_set_data.x, cursor_set_data.y); > + } > } > > static void qxl_send_events(PCIQXLDevice *d, uint32_t events) > diff --git a/hw/qxl.h b/hw/qxl.h > index 701245f..f4f99ec 100644 > --- a/hw/qxl.h > +++ b/hw/qxl.h > @@ -93,6 +93,8 @@ typedef struct PCIQXLDevice { > > /* qxl.c */ > void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id); > +void qxl_server_request_cursor_set(PCIQXLDevice *qxl, QEMUCursor *c, int x, int y); > +void qxl_server_request_cursor_move(PCIQXLDevice *qxl, int x, int y); > > /* qxl-logger.c */ > void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id); > @@ -102,3 +104,5 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext); > void qxl_render_resize(PCIQXLDevice *qxl); > void qxl_render_update(PCIQXLDevice *qxl); > void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext); > +void qxl_render_cursor_set(SimpleSpiceDisplay *ssd, QEMUCursor *c, int x, int y); > +void qxl_render_cursor_move(SimpleSpiceDisplay *ssd, int x, int y); > diff --git a/ui/spice-display.h b/ui/spice-display.h > index 2be6a34..bbfd689 100644 > --- a/ui/spice-display.h > +++ b/ui/spice-display.h > @@ -31,11 +31,22 @@ > > #define NUM_SURFACES 1024 > > -/* For commands/requests from server thread to iothread */ > +/* > + * Commands/requests from server thread to iothread. > + * Note that CREATE_UPDATE is used both with qxl and without it (spice-display) > + * the others are only used with the qxl device. > + * > + * SET_IRQ - just the request is sent (1 byte) > + * CREATE_UPDATE - jus the request is sent (1 byte) > + * CURSOR_SET - send QXLServerRequestCursorSet > + * CURSOR_MOVE - send QXLServerRequestCursorMove > + */ > #define QXL_EMPTY_UPDATE ((void *)-1) > enum { > QXL_SERVER_SET_IRQ = 1, > QXL_SERVER_CREATE_UPDATE, > + QXL_SERVER_CURSOR_SET, > + QXL_SERVER_CURSOR_MOVE > }; > > struct SimpleSpiceUpdate;
On 03/16/11 16:52, Alon Levy wrote: > +void qxl_server_request_cursor_set(PCIQXLDevice *qxl, QEMUCursor *c, int x, int y) > +{ > + QXLServerCursorSetRequest req; > + int r; > + > + req.req = QXL_SERVER_CURSOR_SET; > + req.data.c = c; > + req.data.x = x; > + req.data.y = y; > + r = write(qxl->ssd.pipe[1], &req, sizeof(req)); > + assert(r == sizeof(req)); > +} There's a number of asserts here, which I am not sure is a good thing. I don't understand how far down the code this is, and if it is really fatal if this write fails? > +/* called from spice server thread context only */ > +void qxl_server_request_cursor_move(PCIQXLDevice *qxl, int x, int y) > +{ > + QXLServerCursorMoveRequest req; > + int r; > + > + req.req = QXL_SERVER_CURSOR_MOVE; > + req.data.x = x; > + req.data.y = y; > + r = write(qxl->ssd.pipe[1], &req, sizeof(req)); > + assert(r == sizeof(req)); ditto > +static void read_bytes(int fd, void *buf, int len_requested) > +{ > + int len; > + int total_len = 0; > + > + do { > + len = read(fd, buf, len_requested - total_len); > + if (len < 0) { > + if (errno == EINTR || errno == EAGAIN) { > + continue; > + } > + perror("qxl: pipe_read: read failed"); > + /* will abort once it's out of the while loop */ > + break; > + } > + total_len += len; > + buf = (uint8_t *)buf + len; > + } while (total_len < len_requested); > + assert(total_len == len_requested); and here? Cheers, Jes
On Wed, Mar 16, 2011 at 05:48:41PM +0100, Jes Sorensen wrote: > On 03/16/11 16:52, Alon Levy wrote: > > +void qxl_server_request_cursor_set(PCIQXLDevice *qxl, QEMUCursor *c, int x, int y) > > +{ > > + QXLServerCursorSetRequest req; > > + int r; > > + > > + req.req = QXL_SERVER_CURSOR_SET; > > + req.data.c = c; > > + req.data.x = x; > > + req.data.y = y; > > + r = write(qxl->ssd.pipe[1], &req, sizeof(req)); > > + assert(r == sizeof(req)); > > +} > > There's a number of asserts here, which I am not sure is a good thing. I > don't understand how far down the code this is, and if it is really > fatal if this write fails? A failure there means we can't write to a pipe between the server thread and the iothread (main thread). That is not supposed to happen - and if it does it means some operation by the spice server will never complete. Same for the asserts below, writes are from spice server thread, reads are in iothread. > > > +/* called from spice server thread context only */ > > +void qxl_server_request_cursor_move(PCIQXLDevice *qxl, int x, int y) > > +{ > > + QXLServerCursorMoveRequest req; > > + int r; > > + > > + req.req = QXL_SERVER_CURSOR_MOVE; > > + req.data.x = x; > > + req.data.y = y; > > + r = write(qxl->ssd.pipe[1], &req, sizeof(req)); > > + assert(r == sizeof(req)); > > ditto > > > +static void read_bytes(int fd, void *buf, int len_requested) > > +{ > > + int len; > > + int total_len = 0; > > + > > + do { > > + len = read(fd, buf, len_requested - total_len); > > + if (len < 0) { > > + if (errno == EINTR || errno == EAGAIN) { > > + continue; > > + } > > + perror("qxl: pipe_read: read failed"); > > + /* will abort once it's out of the while loop */ > > + break; > > + } > > + total_len += len; > > + buf = (uint8_t *)buf + len; > > + } while (total_len < len_requested); > > + assert(total_len == len_requested); > > and here? > > Cheers, > Jes > > >
On 03/17/11 10:32, Alon Levy wrote: > On Wed, Mar 16, 2011 at 05:48:41PM +0100, Jes Sorensen wrote: >> > On 03/16/11 16:52, Alon Levy wrote: >>> > > +void qxl_server_request_cursor_set(PCIQXLDevice *qxl, QEMUCursor *c, int x, int y) >>> > > +{ >>> > > + QXLServerCursorSetRequest req; >>> > > + int r; >>> > > + >>> > > + req.req = QXL_SERVER_CURSOR_SET; >>> > > + req.data.c = c; >>> > > + req.data.x = x; >>> > > + req.data.y = y; >>> > > + r = write(qxl->ssd.pipe[1], &req, sizeof(req)); >>> > > + assert(r == sizeof(req)); >>> > > +} >> > >> > There's a number of asserts here, which I am not sure is a good thing. I >> > don't understand how far down the code this is, and if it is really >> > fatal if this write fails? > A failure there means we can't write to a pipe between the server thread > and the iothread (main thread). That is not supposed to happen - and if > it does it means some operation by the spice server will never complete. > > Same for the asserts below, writes are from spice server thread, reads > are in iothread. But shouldn't this make it try to reconnect? Even if the reconnect fails, it shouldn't kill the guest IMHO. Cheers, Jes
On Thu, Mar 17, 2011 at 10:48:43AM +0100, Jes Sorensen wrote: > On 03/17/11 10:32, Alon Levy wrote: > > On Wed, Mar 16, 2011 at 05:48:41PM +0100, Jes Sorensen wrote: > >> > On 03/16/11 16:52, Alon Levy wrote: > >>> > > +void qxl_server_request_cursor_set(PCIQXLDevice *qxl, QEMUCursor *c, int x, int y) > >>> > > +{ > >>> > > + QXLServerCursorSetRequest req; > >>> > > + int r; > >>> > > + > >>> > > + req.req = QXL_SERVER_CURSOR_SET; > >>> > > + req.data.c = c; > >>> > > + req.data.x = x; > >>> > > + req.data.y = y; > >>> > > + r = write(qxl->ssd.pipe[1], &req, sizeof(req)); > >>> > > + assert(r == sizeof(req)); > >>> > > +} > >> > > >> > There's a number of asserts here, which I am not sure is a good thing. I > >> > don't understand how far down the code this is, and if it is really > >> > fatal if this write fails? > > A failure there means we can't write to a pipe between the server thread > > and the iothread (main thread). That is not supposed to happen - and if > > it does it means some operation by the spice server will never complete. > > > > Same for the asserts below, writes are from spice server thread, reads > > are in iothread. > > But shouldn't this make it try to reconnect? Even if the reconnect > fails, it shouldn't kill the guest IMHO. reconnect? between two threads in the qemu process? why would the write fail to begin with? this is like saying if I'm failing a kvm ioctl I should just retry. > > Cheers, > Jes > > >
On 03/17/11 11:27, Alon Levy wrote: > On Thu, Mar 17, 2011 at 10:48:43AM +0100, Jes Sorensen wrote: >>> Same for the asserts below, writes are from spice server thread, reads >>> are in iothread. >> >> But shouldn't this make it try to reconnect? Even if the reconnect >> fails, it shouldn't kill the guest IMHO. > > reconnect? between two threads in the qemu process? why would the write > fail to begin with? this is like saying if I'm failing a kvm ioctl I should > just retry. Ah ok, I missed that part, somehow I had in my mind it was two different processes, despite you mentioning threads. Still if gfx handling fails, it shouldn't nuke the guest. Cheers, Jes
On Thu, Mar 17, 2011 at 11:29:03AM +0100, Jes Sorensen wrote: > On 03/17/11 11:27, Alon Levy wrote: > > On Thu, Mar 17, 2011 at 10:48:43AM +0100, Jes Sorensen wrote: > >>> Same for the asserts below, writes are from spice server thread, reads > >>> are in iothread. > >> > >> But shouldn't this make it try to reconnect? Even if the reconnect > >> fails, it shouldn't kill the guest IMHO. > > > > reconnect? between two threads in the qemu process? why would the write > > fail to begin with? this is like saying if I'm failing a kvm ioctl I should > > just retry. > > Ah ok, I missed that part, somehow I had in my mind it was two different > processes, despite you mentioning threads. > > Still if gfx handling fails, it shouldn't nuke the guest. ok, try to apply that logic to any other device - network, usb, etc., I don't think it holds. > > Cheers, > Jes >
On 03/17/11 11:45, Alon Levy wrote: > On Thu, Mar 17, 2011 at 11:29:03AM +0100, Jes Sorensen wrote: >> On 03/17/11 11:27, Alon Levy wrote: >>> On Thu, Mar 17, 2011 at 10:48:43AM +0100, Jes Sorensen wrote: >>>>> Same for the asserts below, writes are from spice server thread, reads >>>>> are in iothread. >>>> >>>> But shouldn't this make it try to reconnect? Even if the reconnect >>>> fails, it shouldn't kill the guest IMHO. >>> >>> reconnect? between two threads in the qemu process? why would the write >>> fail to begin with? this is like saying if I'm failing a kvm ioctl I should >>> just retry. >> >> Ah ok, I missed that part, somehow I had in my mind it was two different >> processes, despite you mentioning threads. >> >> Still if gfx handling fails, it shouldn't nuke the guest. > > ok, try to apply that logic to any other device - network, usb, etc., I don't > think it holds. Maybe I am looking at the wrong angle - I would think that is network or usb breaks, we would still keep running, and for gfx the guest should be able to keep running even if the monitor is disconnected. It's not a big issue so if you feel it is fine as is, I won't object. Cheers, Jes
On Thu, Mar 17, 2011 at 03:19:28PM +0100, Jes Sorensen wrote: > On 03/17/11 11:45, Alon Levy wrote: > > On Thu, Mar 17, 2011 at 11:29:03AM +0100, Jes Sorensen wrote: > >> On 03/17/11 11:27, Alon Levy wrote: > >>> On Thu, Mar 17, 2011 at 10:48:43AM +0100, Jes Sorensen wrote: > >>>>> Same for the asserts below, writes are from spice server thread, reads > >>>>> are in iothread. > >>>> > >>>> But shouldn't this make it try to reconnect? Even if the reconnect > >>>> fails, it shouldn't kill the guest IMHO. > >>> > >>> reconnect? between two threads in the qemu process? why would the write > >>> fail to begin with? this is like saying if I'm failing a kvm ioctl I should > >>> just retry. > >> > >> Ah ok, I missed that part, somehow I had in my mind it was two different > >> processes, despite you mentioning threads. > >> > >> Still if gfx handling fails, it shouldn't nuke the guest. > > > > ok, try to apply that logic to any other device - network, usb, etc., I don't > > think it holds. > > Maybe I am looking at the wrong angle - I would think that is network or > usb breaks, we would still keep running, and for gfx the guest should be > able to keep running even if the monitor is disconnected. > > It's not a big issue so if you feel it is fine as is, I won't object. I think it could be marginally useful - I mean if someone is running a vm with spice it's for desktop use, and that means graphics is important. of course they could be running a server vm and have it for debugging, but that would be silly. This could hide an actual bug, by not aborting here. Not a major point. I'm also not sure what's the right thing to do - turn on a flag for stopping handling requests? it would just open up a whole set of states we don't handle currently, half-working states. > > Cheers, > Jes >
diff --git a/hw/qxl-render.c b/hw/qxl-render.c index 58965e0..6759edb 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -178,7 +178,6 @@ fail: return NULL; } - /* called from spice server thread context only */ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext) { @@ -209,18 +208,26 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext) if (c == NULL) { c = cursor_builtin_left_ptr(); } - qemu_mutex_lock_iothread(); - qxl->ssd.ds->cursor_define(c); - qxl->ssd.ds->mouse_set(x, y, 1); - qemu_mutex_unlock_iothread(); - cursor_put(c); + qxl_server_request_cursor_set(qxl, c, x, y); break; case QXL_CURSOR_MOVE: x = cmd->u.position.x; y = cmd->u.position.y; - qemu_mutex_lock_iothread(); - qxl->ssd.ds->mouse_set(x, y, 1); - qemu_mutex_unlock_iothread(); + qxl_server_request_cursor_move(qxl, x, y); break; } } + +/* called from iothread only (via qxl.c:pipe_read) */ +void qxl_render_cursor_set(SimpleSpiceDisplay *ssd, QEMUCursor *c, int x, int y) +{ + ssd->ds->cursor_define(c); + ssd->ds->mouse_set(x, y, 1); + cursor_put(c); +} + +/* called from iothread only (via qxl.c:pipe_read) */ +void qxl_render_cursor_move(SimpleSpiceDisplay *ssd, int x, int y) +{ + ssd->ds->mouse_set(x, y, 1); +} diff --git a/hw/qxl.c b/hw/qxl.c index cf3c938..4c27deb 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -117,6 +117,27 @@ static QXLMode qxl_modes[] = { #endif }; +typedef struct __attribute__ ((__packed__)) { + QEMUCursor *c; + int x; + int y; +} QXLServerCursorSet; + +typedef struct __attribute__ ((__packed__)) { + int x; + int y; +} QXLServerCursorMove; + +typedef struct __attribute__ ((__packed__)) { + unsigned char req; + QXLServerCursorMove data; +} QXLServerCursorMoveRequest; + +typedef struct __attribute__ ((__packed__)) { + unsigned char req; + QXLServerCursorSet data; +} QXLServerCursorSetRequest; + static PCIQXLDevice *qxl0; static void qxl_send_events(PCIQXLDevice *d, uint32_t events); @@ -337,6 +358,33 @@ static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info) } /* called from spice server thread context only */ +void qxl_server_request_cursor_set(PCIQXLDevice *qxl, QEMUCursor *c, int x, int y) +{ + QXLServerCursorSetRequest req; + int r; + + req.req = QXL_SERVER_CURSOR_SET; + req.data.c = c; + req.data.x = x; + req.data.y = y; + r = write(qxl->ssd.pipe[1], &req, sizeof(req)); + assert(r == sizeof(req)); +} + +/* called from spice server thread context only */ +void qxl_server_request_cursor_move(PCIQXLDevice *qxl, int x, int y) +{ + QXLServerCursorMoveRequest req; + int r; + + req.req = QXL_SERVER_CURSOR_MOVE; + req.data.x = x; + req.data.y = y; + r = write(qxl->ssd.pipe[1], &req, sizeof(req)); + assert(r == sizeof(req)); +} + +/* called from spice server thread context only */ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) { PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl); @@ -1055,12 +1103,37 @@ static void qxl_map(PCIDevice *pci, int region_num, } } +static void read_bytes(int fd, void *buf, int len_requested) +{ + int len; + int total_len = 0; + + do { + len = read(fd, buf, len_requested - total_len); + if (len < 0) { + if (errno == EINTR || errno == EAGAIN) { + continue; + } + perror("qxl: pipe_read: read failed"); + /* will abort once it's out of the while loop */ + break; + } + total_len += len; + buf = (uint8_t *)buf + len; + } while (total_len < len_requested); + assert(total_len == len_requested); +} + static void pipe_read(void *opaque) { SimpleSpiceDisplay *ssd = opaque; unsigned char cmd; int len, set_irq = 0; int create_update = 0; + int cursor_set = 0; + int cursor_move = 0; + QXLServerCursorSet cursor_set_data; + QXLServerCursorMove cursor_move_data; while (1) { cmd = 0; @@ -1082,6 +1155,17 @@ static void pipe_read(void *opaque) case QXL_SERVER_CREATE_UPDATE: create_update = 1; break; + case QXL_SERVER_CURSOR_SET: + if (cursor_set == 1) { + cursor_put(cursor_set_data.c); + } + cursor_set = 1; + read_bytes(ssd->pipe[0], &cursor_set_data, sizeof(cursor_set_data)); + break; + case QXL_SERVER_CURSOR_MOVE: + cursor_move = 1; + read_bytes(ssd->pipe[0], &cursor_move_data, sizeof(cursor_move_data)); + break; default: fprintf(stderr, "%s: unknown cmd %u\n", __FUNCTION__, cmd); abort(); @@ -1099,6 +1183,12 @@ static void pipe_read(void *opaque) if (set_irq) { qxl_set_irq(container_of(ssd, PCIQXLDevice, ssd)); } + if (cursor_move) { + qxl_render_cursor_move(ssd, cursor_move_data.x, cursor_move_data.y); + } + if (cursor_set) { + qxl_render_cursor_set(ssd, cursor_set_data.c, cursor_set_data.x, cursor_set_data.y); + } } static void qxl_send_events(PCIQXLDevice *d, uint32_t events) diff --git a/hw/qxl.h b/hw/qxl.h index 701245f..f4f99ec 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -93,6 +93,8 @@ typedef struct PCIQXLDevice { /* qxl.c */ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id); +void qxl_server_request_cursor_set(PCIQXLDevice *qxl, QEMUCursor *c, int x, int y); +void qxl_server_request_cursor_move(PCIQXLDevice *qxl, int x, int y); /* qxl-logger.c */ void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id); @@ -102,3 +104,5 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext); void qxl_render_resize(PCIQXLDevice *qxl); void qxl_render_update(PCIQXLDevice *qxl); void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext); +void qxl_render_cursor_set(SimpleSpiceDisplay *ssd, QEMUCursor *c, int x, int y); +void qxl_render_cursor_move(SimpleSpiceDisplay *ssd, int x, int y); diff --git a/ui/spice-display.h b/ui/spice-display.h index 2be6a34..bbfd689 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -31,11 +31,22 @@ #define NUM_SURFACES 1024 -/* For commands/requests from server thread to iothread */ +/* + * Commands/requests from server thread to iothread. + * Note that CREATE_UPDATE is used both with qxl and without it (spice-display) + * the others are only used with the qxl device. + * + * SET_IRQ - just the request is sent (1 byte) + * CREATE_UPDATE - jus the request is sent (1 byte) + * CURSOR_SET - send QXLServerRequestCursorSet + * CURSOR_MOVE - send QXLServerRequestCursorMove + */ #define QXL_EMPTY_UPDATE ((void *)-1) enum { QXL_SERVER_SET_IRQ = 1, QXL_SERVER_CREATE_UPDATE, + QXL_SERVER_CURSOR_SET, + QXL_SERVER_CURSOR_MOVE }; struct SimpleSpiceUpdate;