[PATCH weston] systemd: take over a socket created by systemd in case of

[Pekka Paalanen]

On Wed, 30 Mar 2016 09:31:04 +0000
"Friedrich, Eugen (ADITG/SW1)" <efriedrich at de.adit-jv.com> wrote:

> Systemd provides a feature of socket-based activation, details in [1]
> This commit adds an implementation to check if socket was provided by systemd
> and adds this as an additional socket to wayland display.
> This is usefull for early rendering use-cases where weston and early-rendering-application
> can be started parallel.
> 
> [1] https://www.freedesktop.org/software/systemd/man/systemd.socket.html
> 
> Signed-off-by: Eugen Friedrich <efriedrich at de.adit-jv.com>
> ---
> src/systemd-notify.c |   12 ++++++++++++
> 1 file changed, 12 insertions(+)
> 

Hi Eugen,

sorry, but it seems something has butchered the patch. All tabs have
converted to spaces, there is an extra space in front of every context
line, and there is some HTML garbage too.

The subject line seems cut short, would be fine to just remove the "in
case of" too.

The feature itself seems fine to me.

> diff --git a/src/systemd-notify.c b/src/systemd-notify.c
> index e61db0f..a921241 100644
> --- a/src/systemd-notify.c
> +++ b/src/systemd-notify.c
> @@ -79,6 +79,7 @@ module_init(struct weston_compositor *compositor,
>                struct wl_event_loop *loop;
>                long watchdog_time_conv;
>                struct systemd_notifier *notifier;
> +             int fd;
>                 notifier = zalloc(sizeof *notifier);
>                if (notifier == NULL)
> @@ -89,6 +90,17 @@ module_init(struct weston_compositor *compositor,
>                wl_signal_add(&compositor->destroy_signal,
>                                      &notifier->compositor_destroy_listener);
> +             /*take additional display socket if provided by systemd*/
> +             if (1 == sd_listen_fds(0)) {

Shouldn't the argument be non-zero to clean up the environment, as
Weston will launch child processes?

How about looping through all file descriptors received, instead of
requiring exactly one to be ever defined?

It would be good to use sd_is_socket(AF_UNIX, SOCK_STREAM,
yes-listening) to check for a correct socket type.

> +                             fd = SD_LISTEN_FDS_START + 0;
> +                             weston_log("info:add socket for weston created by systemd\n");
> +
> +                             if (wl_display_add_socket_fd(compositor->wl_display, fd)) {
> +                                             weston_log("wl_display_add_socket_fd failed\n");
> +                                             return -1;
> +                             }
> +             }
> +
>                sd_notify(0, "READY=1");
>                 /* 'WATCHDOG_USEC' is environment variable that is set
> --
> 1.7.9.5
> 

I also seem to miss a bit of documentation. The systemd integration
features were not really documented before either, but at least
the ./configure --help talks only about notifications. Some words
somewhere that this plugin supports systemd socket activation would be
nice to add.


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/20160401/d944a313/attachment.sig>