Hi Christophe,
Please see some comments below
On 12/14/2016 12:51 PM, Christophe Fergeau wrote:
In newer X.org versions, it's no longer supported to modify the set of
FDs passed to a BlockHandler method to get notified when the FD has data
to be read. This was limited anyway as we could only get read events
this way, and had to do our own polling to get notified about socket
writeability.
Starting from xserver 1.19, the supported way of doing this is to use
the SetNotifyFd/RemoveNotifyFd API, which is actually a much better way
as it matches very well the 'watch' API spice-server expects Xspice to
implement.
This commit switches to that new API, which removes the need for
RegisterBlockAndWakeupHandlers().
---
I've lightly (xeyes/rxvt) tested this on f25, and tested this still builds on
f24.
This should fix the issues Hans mentioned in
https://lists.freedesktop.org/archives/spice-devel/2016-October/032501.html
configure.ac | 5 ---
src/spiceqxl_main_loop.c | 90 ++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 84 insertions(+), 11 deletions(-)
[skip]
diff --git a/src/spiceqxl_main_loop.c b/src/spiceqxl_main_loop.c
index 49b715a..81bc418 100644
--- a/src/spiceqxl_main_loop.c
+++ b/src/spiceqxl_main_loop.c
[skip]
@@ -276,7 +272,7 @@ static void xspice_block_handler(pointer data, OSTimePtr timeout, pointer readma
/*
* xserver only calls wakeup_handler with the read fd_set, so we
* must either patch it or do a polling select ourselves, this is the
- * later approach. Since we are already doing a polling select, we
+ * latter approach. Since we are already doing a polling select, we
* already select on all (we could avoid selecting on the read since
* that *is* actually taken care of by the wakeup handler).
*/
@@ -338,9 +334,88 @@ static void xspice_wakeup_handler(pointer data, int nfds, pointer readmask)
select_and_check_watches();
}
+#else /* GET_ABI_MAJOR(ABI_VIDEODRV_VERSION) < 23 */
+
+struct SpiceWatch {
+ int fd;
+ int event_mask;
+ SpiceWatchFunc func;
+ void *opaque;
+};
+
+static void watch_fd_notified(int fd, int xevents, void *data)
+{
+ SpiceWatch *watch = (SpiceWatch *)data;
+
+ if ((watch->event_mask & SPICE_WATCH_EVENT_READ) && (xevents & X_NOTIFY_READ)) {
+ watch->func(watch->fd, SPICE_WATCH_EVENT_READ, watch->opaque);
+ }
+
+ if ((watch->event_mask & SPICE_WATCH_EVENT_WRITE) && (xevents & X_NOTIFY_WRITE)) {
+ watch->func(watch->fd, SPICE_WATCH_EVENT_WRITE, watch->opaque);
+ }
+}
+
+static int watch_update_mask2(SpiceWatch *watch, int event_mask)
+{
+ SetNotifyFd(watch->fd, NULL, X_NOTIFY_NONE, NULL);
+ watch->event_mask = 0;
+
+ if (event_mask & SPICE_WATCH_EVENT_READ) {
+ SetNotifyFd(watch->fd, watch_fd_notified, X_NOTIFY_READ, watch);
+ } else if (event_mask & SPICE_WATCH_EVENT_READ) {
1. This should be (event_mask & SPICE_WATCH_EVENT_WRITE)
2. The "else if" fails to support event_mask which is READ | WRITE.
Can it not watch both events ?
Regards,
Uri.
+ SetNotifyFd(watch->fd, watch_fd_notified, X_NOTIFY_WRITE, watch);
+ } else {
+ DPRINTF(0, "Unexpected watch event_mask: %i", event_mask);
+ return -1;
+ }
+ watch->event_mask = event_mask;
+
+ return 0;
+}
+
+static SpiceWatch *watch_add(int fd, int event_mask, SpiceWatchFunc func, void *opaque)
+{
+ SpiceWatch *watch = malloc(sizeof(SpiceWatch));
+
+ DPRINTF(0, "adding %p, fd=%d", watch, fd);
+
+ watch->fd = fd;
+ watch->func = func;
+ watch->opaque = opaque;
+ if (watch_update_mask2(watch, event_mask) != 0) {
+ free(watch);
+ return NULL;
+ }
+
+ return watch;
+}
+
+static void watch_update_mask(SpiceWatch *watch, int event_mask)
+{
+ DPRINTF(0, "fd %d to %d", watch->fd, event_mask);
+ watch_update_mask2(watch, event_mask);
+}
+
+static void watch_remove(SpiceWatch *watch)
+{
+ DPRINTF(0, "remove %p (fd %d)", watch, watch->fd);
+ RemoveNotifyFd(watch->fd);
+ free(watch);
+}
+#endif
+
+static void channel_event(int event, SpiceChannelEventInfo *info)
+{
+ NOT_IMPLEMENTED
+}
+
+
SpiceCoreInterface *basic_event_loop_init(void)
{
+#if GET_ABI_MAJOR(ABI_VIDEODRV_VERSION) < 23
ring_init(&watches);
+#endif
bzero(&core, sizeof(core));
core.base.major_version = SPICE_INTERFACE_CORE_MAJOR;
core.base.minor_version = SPICE_INTERFACE_CORE_MINOR; // anything less then 3 and channel_event isn't called
@@ -355,7 +430,10 @@ SpiceCoreInterface *basic_event_loop_init(void)
return &core;
}
+
void xspice_register_handlers(void)
{
+#if GET_ABI_MAJOR(ABI_VIDEODRV_VERSION) < 23
RegisterBlockAndWakeupHandlers(xspice_block_handler, xspice_wakeup_handler, 0);
+#endif
}
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel