[PATCH weston 2/2] xwayland: make the plugin usable by libweston compositors
Giulio Camuffo
giuliocamuffo at gmail.com
Wed Jun 29 09:11:29 UTC 2016
2016-06-15 16:04 GMT+02:00 Pekka Paalanen <ppaalanen at gmail.com>:
> Hi Giulio,
>
> was the plugin registry easy to use?
Yeah, quite straightforward. Good work on it.
<snip>
>> +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?
Weird, i was sure the old code used to do it, but apparently you're
right. So yes, i think not having it is broken.
>
>> + 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?
I will change the strcmp to a strstr, i prefer not to touch the tests
code until we decide if and how we want to change the user-facing
interface for plugins.
Cheers,
Giulio
>
> Otherwise very good work.
>
>
> Thanks,
> pq
More information about the wayland-devel
mailing list