[Spice-devel] [qxl] xspice: Adjust to X.org 1.19 changes
Hans de Goede
hdegoede at redhat.com
Wed Dec 14 11:07:55 UTC 2016
Hi,
On 14-12-16 11:51, 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().
Thank you for doing this, one small comment inline, otherwise looks
good:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
(with the comment fixed).
> ---
> 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(-)
>
> diff --git a/configure.ac b/configure.ac
> index e865e75..b737d0d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -68,8 +68,6 @@ PKG_CHECK_EXISTS(xfont2,
>
> # Obtain compiler/linker options for the driver dependencies
> PKG_CHECK_MODULES(XORG, [xorg-server >= 1.0.99.901] xproto fontsproto $xfont_pc $REQUIRED_MODULES)
> -# Check for xorg 1.19 as XSpice is currently not working with it
> -PKG_CHECK_EXISTS([xorg-server >= 1.18.99], [has_xorg119=yes])
>
> save_CFLAGS="$CFLAGS"
> CFLAGS="$XORG_CFLAGS"
> @@ -141,9 +139,6 @@ if test "x$enable_xspice" = "xyes"; then
> AC_SUBST(SPICE_LIBS)
> ],
> )
> - if test x"${enable_xspice}" = "xyes" && test x"${has_xorg119}" = "xyes"; then
> - AC_MSG_ERROR("XSpice cannot currently work against X.Org 1.19")
> - fi
> else
> enable_xspice=no
> fi
> 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
> @@ -193,6 +193,7 @@ static void timer_remove(SpiceTimer *timer)
> free(timer);
> }
>
> +#if GET_ABI_MAJOR(ABI_VIDEODRV_VERSION) < 23
> struct SpiceWatch {
> RingItem link;
> int fd;
> @@ -236,11 +237,6 @@ static void watch_remove(SpiceWatch *watch)
> watch_count--;
> }
>
> -static void channel_event(int event, SpiceChannelEventInfo *info)
> -{
> - NOT_IMPLEMENTED
> -}
> -
> static int set_watch_fds(fd_set *rfds, fd_set *wfds)
> {
> SpiceWatch *watch;
> @@ -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) {
> + 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));
This malloc may fail, please replace malloc with
xnfalloc which stands for X no fail alloc, it works like
the glib malloc functions.
> +
> + 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
> }
>
Regards,
Hans
More information about the Spice-devel
mailing list