[PATCH weston 2/2] xwayland: make the plugin usable by libweston compositors
Pekka Paalanen
ppaalanen at gmail.com
Wed Jun 15 14:04:36 UTC 2016
Hi Giulio,
was the plugin registry easy to use?
On Sat, 4 Jun 2016 19:25:05 +0300
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
> This patch follows a similar approach to the taken to detach the backends
-to the
> from weston. But instead of passing a configuration struct when loading the
> plugin, we use the plugin API registry to register an API, and to get it
> in the compositor side.
> This API allows to spawn the Xwayland process in the compositor side, and
> to deal with signal handling.
> A new function is added in compositor.c to load and init the xwayland.so plugin.
>
> Signed-off-by: Giulio Camuffo <giuliocamuffo at gmail.com>
> ---
> Makefile.am | 14 ++-
> src/compositor.c | 15 +++
> src/compositor.h | 3 +
> src/main.c | 16 ++-
> src/weston.h | 3 +
> xwayland/launcher.c | 259 +++++++++++++++++++--------------------------
> xwayland/weston-xwayland.c | 207 ++++++++++++++++++++++++++++++++++++
> xwayland/xwayland-api.h | 134 +++++++++++++++++++++++
> xwayland/xwayland.h | 9 +-
> 9 files changed, 492 insertions(+), 168 deletions(-)
> create mode 100644 xwayland/weston-xwayland.c
> create mode 100644 xwayland/xwayland-api.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 7167b6e..2358000 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -143,7 +143,8 @@ bin_PROGRAMS += weston
>
> weston_LDFLAGS = -export-dynamic
> weston_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON \
> - -DMODULEDIR='"$(moduledir)"'
> + -DMODULEDIR='"$(moduledir)"' \
> + -DXSERVER_PATH='"@XSERVER_PATH@"'
> weston_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS) $(LIBUNWIND_CFLAGS)
> weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
> $(DLOPEN_LIBS) $(LIBINPUT_BACKEND_LIBS) \
> @@ -152,7 +153,8 @@ weston_LDADD = $(COMPOSITOR_LIBS) $(LIBUNWIND_LIBS) \
> weston_SOURCES = \
> src/main.c \
> src/weston-screenshooter.c \
> - src/text-backend.c
> + src/text-backend.c \
> + xwayland/weston-xwayland.c
>
> # Track this dependency explicitly instead of using BUILT_SOURCES. We
> # add BUILT_SOURCES to CLEANFILES, but we want to keep git-version.h
> @@ -958,7 +960,7 @@ endif
>
> if ENABLE_XWAYLAND
>
> -module_LTLIBRARIES += xwayland.la
> +libweston_module_LTLIBRARIES += xwayland.la
>
> xwayland_la_CPPFLAGS = \
> -I$(top_builddir)/protocol \
> @@ -968,8 +970,7 @@ xwayland_la_CPPFLAGS = \
> -I$(top_builddir)/xwayland \
> -DDATADIR='"$(datadir)"' \
> -DMODULEDIR='"$(moduledir)"' \
> - -DLIBEXECDIR='"$(libexecdir)"' \
> - -DXSERVER_PATH='"@XSERVER_PATH@"'
> + -DLIBEXECDIR='"$(libexecdir)"'
>
> xwayland_la_LDFLAGS = -module -avoid-version
> xwayland_la_LIBADD = \
> @@ -989,6 +990,9 @@ xwayland_la_SOURCES = \
> xwayland/hash.c \
> xwayland/hash.h \
> shared/helpers.h
> +
> +libwestoninclude_HEADERS += xwayland/xwayland-api.h
> +
> endif
>
>
> diff --git a/src/compositor.c b/src/compositor.c
> index 8464f88..bf8e5aa 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -4872,3 +4872,18 @@ weston_compositor_get_user_data(struct weston_compositor *compositor)
> {
> return compositor->user_data;
> }
> +
> +WL_EXPORT int
> +weston_compositor_load_xwayland(struct weston_compositor *compositor)
> +{
> + int (*module_init)(struct weston_compositor *ec,
> + int *argc, char *argv[]);
> + int argc = 0;
> +
> + module_init = weston_load_module("xwayland.so", "module_init");
> + if (!module_init)
> + return -1;
> + if (module_init(compositor, &argc, NULL) < 0)
> + return -1;
> + return 0;
> +}
> diff --git a/src/compositor.h b/src/compositor.h
> index 027b3cd..f934aab 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -1705,6 +1705,9 @@ weston_seat_get_pointer(struct weston_seat *seat);
> struct weston_touch *
> weston_seat_get_touch(struct weston_seat *seat);
>
> +int
> +weston_compositor_load_xwayland(struct weston_compositor *compositor);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/src/main.c b/src/main.c
> index 733bf09..d17d941 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -751,11 +751,17 @@ load_modules(struct weston_compositor *ec, const char *modules,
> while (*p) {
> end = strchrnul(p, ',');
> snprintf(buffer, sizeof buffer, "%.*s", (int) (end - p), p);
> - module_init = wet_load_module(buffer, "module_init");
> - if (!module_init)
> - return -1;
> - if (module_init(ec, argc, argv) < 0)
> - return -1;
> +
> + if (strcmp(buffer, "xwayland.so") == 0) {
I just noticed our tests load xwayland.so with an absolute path, so I
don't think this will match there.
Since both wet_load_module() and weston_load_module() use
$WESTON_BUILD_DIR, I think we could remove the absolute path from
weston-tests-env, but that's another story. Users might be using
absolute paths too.
This does work for the common case, though, so we can deal with this
later. It's the same issue with choosing the backends, where we might
want to use names that look less like file names.
> + if (wet_load_xwayland(ec) < 0)
> + return -1;
> + } else {
> + module_init = wet_load_module(buffer, "module_init");
> + if (!module_init)
> + return -1;
> + if (module_init(ec, argc, argv) < 0)
> + return -1;
> + }
> p = end;
> while (*p == ',')
> p++;
> diff --git a/src/weston.h b/src/weston.h
> index da7c7a9..bff5cc1 100644
> --- a/src/weston.h
> +++ b/src/weston.h
> @@ -63,6 +63,9 @@ wet_get_config(struct weston_compositor *compositor);
> void *
> wet_load_module(const char *name, const char *entrypoint);
>
> +int
> +wet_load_xwayland(struct weston_compositor *comp);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/xwayland/launcher.c b/xwayland/launcher.c
> index 4fd2553..39d520c 100644
> --- a/xwayland/launcher.c
> +++ b/xwayland/launcher.c
> @@ -36,119 +36,27 @@
> #include <signal.h>
>
> #include "xwayland.h"
> +#include "xwayland-api.h"
> #include "shared/helpers.h"
> #include "weston.h"
>
> static int
> -handle_sigusr1(int signal_number, void *data)
> -{
> - struct weston_xserver *wxs = data;
> -
> - /* We'd be safer if we actually had the struct
> - * signalfd_siginfo from the signalfd data and could verify
> - * this came from Xwayland.*/
> - wxs->wm = weston_wm_create(wxs, wxs->wm_fd);
> - wl_event_source_remove(wxs->sigusr1_source);
> -
> - return 1;
> -}
> -
> -static int
> weston_xserver_handle_event(int listen_fd, uint32_t mask, void *data)
> {
> struct weston_xserver *wxs = data;
> - char display[8], s[8], abstract_fd[8], unix_fd[8], wm_fd[8];
> - int sv[2], wm[2], fd;
> - char *xserver = NULL;
> - struct weston_config *config = wet_get_config(wxs->compositor);
> - struct weston_config_section *section;
> -
> - if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sv) < 0) {
> - weston_log("wl connection socketpair failed\n");
> - return 1;
> - }
> + char display[8];
> +
> + snprintf(display, sizeof display, ":%d", wxs->display);
>
> - if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, wm) < 0) {
> - weston_log("X wm connection socketpair failed\n");
> + wxs->pid = wxs->spawn_func(wxs->user_data, display, wxs->abstract_fd, wxs->unix_fd);
> + if (wxs->pid == -1) {
> + weston_log("Failed to spawn the Xwayland server\n");
> return 1;
> }
>
> - wxs->process.pid = fork();
> - switch (wxs->process.pid) {
> - case 0:
> - /* SOCK_CLOEXEC closes both ends, so we need to unset
> - * the flag on the client fd. */
> - fd = dup(sv[1]);
> - if (fd < 0)
> - goto fail;
> - snprintf(s, sizeof s, "%d", fd);
> - setenv("WAYLAND_SOCKET", s, 1);
> -
> - snprintf(display, sizeof display, ":%d", wxs->display);
> -
> - fd = dup(wxs->abstract_fd);
> - if (fd < 0)
> - goto fail;
> - snprintf(abstract_fd, sizeof abstract_fd, "%d", fd);
> - fd = dup(wxs->unix_fd);
> - if (fd < 0)
> - goto fail;
> - snprintf(unix_fd, sizeof unix_fd, "%d", fd);
> - fd = dup(wm[1]);
> - if (fd < 0)
> - goto fail;
> - snprintf(wm_fd, sizeof wm_fd, "%d", fd);
> -
> - section = weston_config_get_section(config,
> - "xwayland", NULL, NULL);
> - weston_config_section_get_string(section, "path",
> - &xserver, XSERVER_PATH);
> -
> - /* Ignore SIGUSR1 in the child, which will make the X
> - * server send SIGUSR1 to the parent (weston) when
> - * it's done with initialization. During
> - * initialization the X server will round trip and
> - * block on the wayland compositor, so avoid making
> - * blocking requests (like xcb_connect_to_fd) until
> - * it's done with that. */
> - signal(SIGUSR1, SIG_IGN);
> -
> - if (execl(xserver,
> - xserver,
> - display,
> - "-rootless",
> - "-listen", abstract_fd,
> - "-listen", unix_fd,
> - "-wm", wm_fd,
> - "-terminate",
> - NULL) < 0)
> - weston_log("exec of '%s %s -rootless "
> - "-listen %s -listen %s -wm %s "
> - "-terminate' failed: %m\n",
> - xserver, display,
> - abstract_fd, unix_fd, wm_fd);
Two spaces too much indent for the log command.
> - fail:
> - _exit(EXIT_FAILURE);
> -
> - default:
> - weston_log("forked X server, pid %d\n", wxs->process.pid);
> -
> - close(sv[1]);
> - wxs->client = wl_client_create(wxs->wl_display, sv[0]);
> -
> - close(wm[1]);
> - wxs->wm_fd = wm[0];
> -
> - weston_watch_process(&wxs->process);
> -
> - wl_event_source_remove(wxs->abstract_source);
> - wl_event_source_remove(wxs->unix_source);
> - break;
> -
> - case -1:
> - weston_log( "failed to fork\n");
> - break;
> - }
> + weston_log("Spawned Xwayland server, pid %d\n", wxs->pid);
> + wl_event_source_remove(wxs->abstract_source);
> + wl_event_source_remove(wxs->unix_source);
>
> return 1;
> }
> @@ -162,7 +70,7 @@ weston_xserver_shutdown(struct weston_xserver *wxs)
> unlink(path);
> snprintf(path, sizeof path, "/tmp/.X11-unix/X%d", wxs->display);
> unlink(path);
> - if (wxs->process.pid == 0) {
> + if (wxs->pid == 0) {
> wl_event_source_remove(wxs->abstract_source);
> wl_event_source_remove(wxs->unix_source);
> }
> @@ -175,39 +83,6 @@ weston_xserver_shutdown(struct weston_xserver *wxs)
> wxs->loop = NULL;
> }
>
> -static void
> -weston_xserver_cleanup(struct weston_process *process, int status)
> -{
> - struct weston_xserver *wxs =
> - container_of(process, struct weston_xserver, process);
> -
> - wxs->process.pid = 0;
> - wxs->client = NULL;
> - wxs->resource = NULL;
> -
> - wxs->abstract_source =
> - wl_event_loop_add_fd(wxs->loop, wxs->abstract_fd,
> - WL_EVENT_READABLE,
> - weston_xserver_handle_event, wxs);
> -
> - wxs->unix_source =
> - wl_event_loop_add_fd(wxs->loop, wxs->unix_fd,
> - WL_EVENT_READABLE,
> - weston_xserver_handle_event, wxs);
> -
> - if (wxs->wm) {
> - weston_log("xserver exited, code %d\n", status);
> - weston_wm_destroy(wxs->wm);
> - wxs->wm = NULL;
> - } else {
> - /* If the X server crashes before it binds to the
> - * xserver interface, shut down and don't try
> - * again. */
> - weston_log("xserver crashing too fast: %d\n", status);
> - weston_xserver_shutdown(wxs);
> - }
> -}
> -
> static int
> bind_to_abstract_socket(int display)
> {
> @@ -349,25 +224,32 @@ weston_xserver_destroy(struct wl_listener *l, void *data)
> free(wxs);
> }
>
> -WL_EXPORT int
> -module_init(struct weston_compositor *compositor,
> - int *argc, char *argv[])
> -
> +static struct weston_xwayland *
> +weston_xwayland_get(struct weston_compositor *compositor)
> {
> - struct wl_display *display = compositor->wl_display;
> + struct wl_listener *listener;
> struct weston_xserver *wxs;
> - char lockfile[256], display_name[8];
>
> - wxs = zalloc(sizeof *wxs);
> - if (wxs == NULL)
> - return -1;
> - wxs->process.cleanup = weston_xserver_cleanup;
> - wxs->wl_display = display;
> - wxs->compositor = compositor;
> + listener = wl_signal_get(&compositor->destroy_signal,
> + weston_xserver_destroy);
> + if (!listener)
> + return NULL;
>
> - wxs->display = 0;
> + wxs = wl_container_of(listener, wxs, destroy_listener);
> + return (struct weston_xwayland *)wxs;
> +}
>
> - retry:
> +static int
> +weston_xwayland_listen(struct weston_xwayland *xwayland, void *user_data,
> + weston_xwayland_spawn_xserver_func_t spawn_func)
> +{
> + struct weston_xserver *wxs = (struct weston_xserver *)xwayland;
> + char lockfile[256], display_name[8];
> +
> + wxs->user_data = user_data;
> + wxs->spawn_func = spawn_func;
> +
> +retry:
> if (create_lockfile(wxs->display, lockfile, sizeof lockfile) < 0) {
> if (errno == EAGAIN) {
> goto retry;
> @@ -399,7 +281,37 @@ module_init(struct weston_compositor *compositor,
> weston_log("xserver listening on display %s\n", display_name);
> setenv("DISPLAY", display_name, 1);
>
> - wxs->loop = wl_display_get_event_loop(display);
> + wxs->loop = wl_display_get_event_loop(wxs->wl_display);
> + wxs->abstract_source =
> + wl_event_loop_add_fd(wxs->loop, wxs->abstract_fd,
> + WL_EVENT_READABLE,
> + weston_xserver_handle_event, wxs);
> + wxs->unix_source =
> + wl_event_loop_add_fd(wxs->loop, wxs->unix_fd,
> + WL_EVENT_READABLE,
> + weston_xserver_handle_event, wxs);
> +
> + return 0;
> +}
> +
> +static void
> +weston_xwayland_xserver_loaded(struct weston_xwayland *xwayland,
> + struct wl_client *client, int wm_fd)
> +{
> + struct weston_xserver *wxs = (struct weston_xserver *)xwayland;
> + wxs->wm = weston_wm_create(wxs, wm_fd);
> + wxs->client = client;
> +}
> +
> +static void
> +weston_xwayland_xserver_exited(struct weston_xwayland *xwayland,
> + int exit_status)
> +{
> + struct weston_xserver *wxs = (struct weston_xserver *)xwayland;
> +
> + wxs->pid = 0;
> + wxs->client = NULL;
> +
> wxs->abstract_source =
> wl_event_loop_add_fd(wxs->loop, wxs->abstract_fd,
> WL_EVENT_READABLE,
> @@ -409,8 +321,49 @@ module_init(struct weston_compositor *compositor,
> WL_EVENT_READABLE,
> weston_xserver_handle_event, wxs);
>
> - wxs->sigusr1_source = wl_event_loop_add_signal(wxs->loop, SIGUSR1,
> - handle_sigusr1, wxs);
> + if (wxs->wm) {
> + weston_log("xserver exited, code %d\n", exit_status);
> + weston_wm_destroy(wxs->wm);
> + wxs->wm = NULL;
> + } else {
> + /* If the X server crashes before it binds to the
> + * xserver interface, shut down and don't try
> + * again. */
> + weston_log("xserver crashing too fast: %d\n", exit_status);
> + weston_xserver_shutdown(wxs);
> + }
> +}
> +
> +const struct weston_xwayland_api api = {
> + weston_xwayland_get,
> + weston_xwayland_listen,
> + weston_xwayland_xserver_loaded,
> + weston_xwayland_xserver_exited,
> +};
> +
> +WL_EXPORT int
> +module_init(struct weston_compositor *compositor,
> + int *argc, char *argv[])
> +
> +{
> + struct wl_display *display = compositor->wl_display;
> + struct weston_xserver *wxs;
> + int ret;
> +
> + wxs = zalloc(sizeof *wxs);
> + if (wxs == NULL)
> + return -1;
> + wxs->wl_display = display;
> + wxs->compositor = compositor;
> +
> + ret = weston_plugin_api_register(compositor, WESTON_XWAYLAND_API_NAME,
> + &api, sizeof(api));
> + if (ret < 0) {
> + weston_log("Failed to register the xwayland module API.\n");
> + free(wxs);
> + return -1;
> + }
> +
> wxs->destroy_listener.notify = weston_xserver_destroy;
> wl_signal_add(&compositor->destroy_signal, &wxs->destroy_listener);
>
> diff --git a/xwayland/weston-xwayland.c b/xwayland/weston-xwayland.c
> new file mode 100644
> index 0000000..137fe82
> --- /dev/null
> +++ b/xwayland/weston-xwayland.c
> @@ -0,0 +1,207 @@
> +/*
> + * Copyright © 2011 Intel Corporation
> + * Copyright © 2016 Giulio Camuffo
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include <signal.h>
> +#include <sys/socket.h>
> +
> +#include "compositor.h"
> +#include "weston.h"
> +#include "xwayland/xwayland-api.h"
> +#include "shared/helpers.h"
> +
> +struct wet_xwayland {
> + struct weston_compositor *compositor;
> + const struct weston_xwayland_api *api;
> + struct weston_xwayland *xwayland;
> + struct wl_event_source *sigusr1_source;
> + struct wl_client *client;
> + int wm_fd;
> + struct weston_process process;
> +};
> +
> +static int
> +handle_sigusr1(int signal_number, void *data)
> +{
> + struct wet_xwayland *wxw = data;
> +
> + /* We'd be safer if we actually had the struct
> + * signalfd_siginfo from the signalfd data and could verify
> + * this came from Xwayland.*/
> + wxw->api->xserver_loaded(wxw->xwayland, wxw->client, wxw->wm_fd);
> + wl_event_source_remove(wxw->sigusr1_source);
> +
> + return 1;
> +}
> +
> +static pid_t
> +spawn_xserver(void *user_data, const char *display, int abstract_fd, int unix_fd)
> +{
> + struct wet_xwayland *wxw = user_data;
> + pid_t pid;
> + char s[8], abstract_fd_str[8], unix_fd_str[8], wm_fd_str[8];
> + int sv[2], wm[2], fd;
> + char *xserver = NULL;
> + struct weston_config *config = wet_get_config(wxw->compositor);
> + struct weston_config_section *section;
> +
> + if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sv) < 0) {
> + weston_log("wl connection socketpair failed\n");
> + return 1;
> + }
> +
> + if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, wm) < 0) {
> + weston_log("X wm connection socketpair failed\n");
> + return 1;
> + }
> +
> + pid = fork();
> + switch (pid) {
> + case 0:
> + /* SOCK_CLOEXEC closes both ends, so we need to unset
> + * the flag on the client fd. */
> + fd = dup(sv[1]);
> + if (fd < 0)
> + goto fail;
> + snprintf(s, sizeof s, "%d", fd);
> + setenv("WAYLAND_SOCKET", s, 1);
> +
> + fd = dup(abstract_fd);
> + if (fd < 0)
> + goto fail;
> + snprintf(abstract_fd_str, sizeof abstract_fd_str, "%d", fd);
> + fd = dup(unix_fd);
> + if (fd < 0)
> + goto fail;
> + snprintf(unix_fd_str, sizeof unix_fd_str, "%d", fd);
> + fd = dup(wm[1]);
> + if (fd < 0)
> + goto fail;
> + snprintf(wm_fd_str, sizeof wm_fd_str, "%d", fd);
> +
> + section = weston_config_get_section(config,
> + "xwayland", NULL, NULL);
> + weston_config_section_get_string(section, "path",
> + &xserver, XSERVER_PATH);
> +
> + /* Ignore SIGUSR1 in the child, which will make the X
> + * server send SIGUSR1 to the parent (weston) when
> + * it's done with initialization. During
> + * initialization the X server will round trip and
> + * block on the wayland compositor, so avoid making
> + * blocking requests (like xcb_connect_to_fd) until
> + * it's done with that. */
> + signal(SIGUSR1, SIG_IGN);
> +
> + if (execl(xserver,
> + xserver,
> + display,
> + "-rootless",
> + "-listen", abstract_fd_str,
> + "-listen", unix_fd_str,
> + "-wm", wm_fd_str,
> + "-terminate",
> + NULL) < 0)
> + weston_log("exec of '%s %s -rootless "
> + "-listen %s -listen %s -wm %s "
> + "-terminate' failed: %m\n",
> + xserver, display,
> + abstract_fd_str, unix_fd_str, wm_fd_str);
> + fail:
> + _exit(EXIT_FAILURE);
> +
> + default:
> + close(sv[1]);
> + wxw->client = wl_client_create(wxw->compositor->wl_display, sv[0]);
> +
> + close(wm[1]);
> + wxw->wm_fd = wm[0];
> +
> + wxw->process.pid = pid;
> + weston_watch_process(&wxw->process);
> + break;
> +
> + case -1:
> + weston_log( "failed to fork\n");
> + break;
> + }
> +
> + return pid;
> +}
> +
> +static void
> +xserver_cleanup(struct weston_process *process, int status)
> +{
> + struct wet_xwayland *wxw =
> + container_of(process, struct wet_xwayland, process);
> + struct wl_event_loop *loop =
> + wl_display_get_event_loop(wxw->compositor->wl_display);
> +
> + wxw->api->xserver_exited(wxw->xwayland, status);
> + wxw->sigusr1_source = wl_event_loop_add_signal(loop, SIGUSR1,
> + handle_sigusr1, wxw);
This is a new addition, isn't it? Before, the SIGUSR1 handler was not
re-armed when Xwayland server exited, AFAICT. This should be mentioned
in the commit message the least.
Does this not mean that the old Xwayland re-starting code was broken?
It did resume listening on the two socket fds, but did not re-arm
SIGUSR1?
> + wxw->client = NULL;
> +}
> +
> +int
> +wet_load_xwayland(struct weston_compositor *comp)
> +{
> + const struct weston_xwayland_api *api;
> + struct weston_xwayland *xwayland;
> + struct wet_xwayland *wxw;
> + struct wl_event_loop *loop;
> +
> + if (weston_compositor_load_xwayland(comp) < 0)
> + return -1;
> +
> + api = weston_xwayland_get_api(comp);
> + if (!api) {
> + weston_log("Failed to get the xwayland module API.\n");
> + return -1;
> + }
> +
> + xwayland = api->get(comp);
> + if (!xwayland) {
> + weston_log("Failed to get the xwayland object.\n");
> + return -1;
> + }
> +
> + wxw = zalloc(sizeof *wxw);
> + if (!wxw)
> + return -1;
> +
> + wxw->compositor = comp;
> + wxw->api = api;
> + wxw->xwayland = xwayland;
> + wxw->process.cleanup = xserver_cleanup;
> + if (api->listen(xwayland, wxw, spawn_xserver) < 0)
> + return -1;
> +
> + loop = wl_display_get_event_loop(comp->wl_display);
> + wxw->sigusr1_source = wl_event_loop_add_signal(loop, SIGUSR1,
> + handle_sigusr1, wxw);
> +
> + return 0;
> +}
> diff --git a/xwayland/xwayland-api.h b/xwayland/xwayland-api.h
> new file mode 100644
> index 0000000..4be87e5
> --- /dev/null
> +++ b/xwayland/xwayland-api.h
> @@ -0,0 +1,134 @@
> +/*
> + * Copyright © 2016 Giulio Camuffo
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial
> + * portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef XWAYLAND_API_H
> +#define XWAYLAND_API_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <unistd.h>
> +
> +#include "plugin-registry.h"
> +
> +struct weston_compositor;
> +struct weston_xwayland;
> +
> +#define WESTON_XWAYLAND_API_NAME "weston_xwayland_v1"
> +
> +typedef pid_t
> +(*weston_xwayland_spawn_xserver_func_t)(
> + void *user_data, const char *display, int abstract_fd, int unix_fd);
> +
> +/** The libweston Xwayland API
> + *
> + * This API allows to control the Xwayland libweston module.
> + * The module must be loaded at runtime with \a weston_compositor_load_xwayland,
> + * after which the API can be retrieved by using \a weston_xwayland_get_api.
> + */
> +struct weston_xwayland_api {
> + /** Retrieve the Xwayland context object.
> + *
> + * Note that this function does not create a new object, but returns
> + * always the same object per compositor instance.
> + * This function cannot fail, if this API object is valid.
> + *
> + * \param compositor The compositor instance.
> + */
> + struct weston_xwayland *
> + (*get)(struct weston_compositor *compositor);
> +
> + /** Listen for X connections.
> + *
> + * This function tells the Xwayland module to start create a X socket
> + * and to listen for client connections. When one such connection is
> + * detected the given \a spawn_func callback will be called to start
> + * the Xwayland process.
> + *
> + * \param xwayland The Xwayland context object.
> + * \param user_data The user data pointer to be passed to \a spawn_func.
> + * \param spawn_func The callback function called to start the Xwayland
> + * server process.
> + *
> + * \return 0 on success, a negative number otherwise.
> + */
> + int
> + (*listen)(struct weston_xwayland *xwayland, void *user_data,
> + weston_xwayland_spawn_xserver_func_t spawn_func);
> +
> + /** Notify the Xwayland module the Xwayland server is loaded.
> + *
> + * After the Xwayland server process has been spawned it will notify
> + * the parent that is has finished the initialization by sending a
> + * SIGUSR1 signal.
> + * The caller should listen for that signal and call this function
> + * when it is received.
> + *
> + * \param xwayland The Xwayland context object.
> + * \param client The wl_client object representing the connection of
> + * the Xwayland server process.
> + * \param wm_fd The file descriptor for the wm.
> + */
> + void
> + (*xserver_loaded)(struct weston_xwayland *xwayland,
> + struct wl_client *client, int wm_fd);
> +
> + /** Notify the Xwayland module the Xwayland server has exited.
> + *
> + * Whenever the Xwayland server process quits this function should be
> + * called.
> + * The Xwayland module will keep listening for X connections on the
> + * socket, and may call the spawn function again.
> + *
> + * \param xwayland The Xwayland context object.
> + * \param exit_status The exit status of the Xwayland server process.
> + */
> + void
> + (*xserver_exited)(struct weston_xwayland *xwayland, int exit_status);
> +};
> +
> +/** Retrieve the API object for the libweston Xwayland module.
> + *
> + * The module must have been previously loaded by calling
> + * \a weston_compositor_load_xwayland.
> + *
> + * \param compositor The compositor instance.
> + */
> +static inline const struct weston_xwayland_api *
> +weston_xwayland_get_api(struct weston_compositor *compositor)
> +{
> + const void *api;
> + api = weston_plugin_api_get(compositor, WESTON_XWAYLAND_API_NAME,
> + sizeof(struct weston_xwayland_api));
> + /* The cast is necessary to use this function in C++ code */
> + return (const struct weston_xwayland_api *)api;
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/xwayland/xwayland.h b/xwayland/xwayland.h
> index e09c6f9..de4b6b6 100644
> --- a/xwayland/xwayland.h
> +++ b/xwayland/xwayland.h
> @@ -31,6 +31,7 @@
>
> #include "compositor.h"
> #include "weston.h"
> +#include "xwayland-api.h"
>
> #define SEND_EVENT_MASK (0x80)
> #define EVENT_TYPE(event) ((event)->response_type & ~SEND_EVENT_MASK)
> @@ -38,20 +39,18 @@
> struct weston_xserver {
> struct wl_display *wl_display;
> struct wl_event_loop *loop;
> - struct wl_event_source *sigchld_source;
> int abstract_fd;
> struct wl_event_source *abstract_source;
> int unix_fd;
> struct wl_event_source *unix_source;
> - int wm_fd;
> int display;
> - struct wl_event_source *sigusr1_source;
> - struct weston_process process;
> - struct wl_resource *resource;
> + pid_t pid;
> struct wl_client *client;
> struct weston_compositor *compositor;
> struct weston_wm *wm;
> struct wl_listener destroy_listener;
> + weston_xwayland_spawn_xserver_func_t spawn_func;
> + void *user_data;
> };
>
> struct weston_wm {
I quickly tested this, and it works. However, 'make check' fails on the
xwayland test. Otherwise I would have proposed to just fix the cosmetic
issues myself while merging.
Ah, I see. As I commented above, the tests use absolute path to the
xwayland.so, so it won't catch the special case, wet_load_xwayland() is
not called, and all the care-taking ends up missing that is now outside
of xwayland.so.
You could fix the special case to manage also absolute paths, or we
could see about dropping the absolute path from weston-tests-env.
Do you have a preference?
Otherwise very good work.
Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160615/88d6374c/attachment-0001.sig>
More information about the wayland-devel
mailing list