> -----Original Message-----
> From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> Sent: Friday, April 01, 2016 2:15 PM
> To: Friedrich, Eugen (ADITG/SW1)
> Cc: wayland mailing list; Natsume, Wataru (ADITJ/SWG); Ucan, Emre
> (ADITG/SW1)
> Subject: Re: [PATCH weston] systemd: take over a socket created by
> systemd in case of
>
> On Wed, 30 Mar 2016 09:31:04 +0000
> "Friedrich, Eugen (ADITG/SW1)" <efriedr...@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.htm
> l
> >
> > Signed-off-by: Eugen Friedrich <efriedr...@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.
[EF] should my automatic outlook settings ,will take care next time
>
> The subject line seems cut short, would be fine to just remove the "in case
> of" too.
[EF] agree
>
> 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,
> >
> > ¬ifier->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?
[EF] make sense, fs descriptors for child processes are not relevant, we should
unset it
>
> How about looping through all file descriptors received, instead of requiring
> exactly one to be ever defined?
[EF] did not saw the use-case before but basically it is a good idea, will add
this
>
> It would be good to use sd_is_socket(AF_UNIX, SOCK_STREAM,
> yes-listening) to check for a correct socket type.
[EF] good point, will add this
>
> > + 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.
[EF] agree, will add
>
>
> Thanks,
> pq
[EF] basically you can just ignore this patch , I will create new one which
will rework the findings
Best regards
Eugen Friedrich
Software Group I (ADITG/SW1)
Tel. +49 5121 49 6921
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
