[PATCH weston] compositor: systemd notifications support

[Pekka Paalanen]

On Thu, 24 Sep 2015 15:53:53 +0300
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> 2015-09-24 14:06 GMT+03:00 Egor Starkov <egor.starkov at ge.com>:
> > Add systemd status and watchdog notification support.
> > Feature is not compiled by default and can be enabled by
> > "--enable-systemd-notify" configuration flag. Watchdog
> > timeout equals to half of timeout defined by "WATCHDOG_USEC"
> > environment variable, which is set by "WatchdogSec="
> > setting in service file.
> 
> Hi,
> 
> I don't quite like that we're calling a systemd specific thing in the
> main(), can't this be called by the logind launcher? Alternatively i
> guess we could abstract this a way with a new weston_launcher_start()
> or something like it.

Hi,

the launcher optionally uses logind, not systemd. Logind can exist
without systemd, so conflating this patch with that doesn't seem
appropriate either.

We can't call it at the backend init time, because the listening
sockets are created only afterwards. I suppose we could do it from the
idle handler? But then we need to set up the idle handler somewhere.

Or would it help if we named it weston_service_start()?

> Besides currently if the notify support is built it will be called
> even when running nested in X or another wayland compositor, right?
> Using the launcher would avoid that.

Yes, it will be called, and it's probably unneeded for the nested
backends. It will also be called when starting weston manually instead
from a service file, but sd_notify() should handle that just fine.

There is an interesting scenario there. If host Weston is started from
systemd service, I don't think it removes the systemd variables from
the environment. If you then start a nested Weston in the same process
hierarchy, it would think it's running on systemd.

Hmm, should this be turned into a plugin instead?

Making it a plugin seems like the most flexible choice for a user. And
for packaging.

> >
> > Signed-off-by: Egor Starkov <egor.starkov at ge.com>
> > Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > ---
> >  Makefile.am          |  10 ++++-
> >  configure.ac         |  13 ++++++
> >  src/main.c           |   3 ++
> >  src/systemd-notify.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/systemd-notify.h |  47 ++++++++++++++++++++
> >  5 files changed, 192 insertions(+), 1 deletion(-)
> >  create mode 100644 src/systemd-notify.c
> >  create mode 100644 src/systemd-notify.h

> > diff --git a/src/main.c b/src/main.c
> > index a98570e..fd32751 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -46,6 +46,7 @@
> >  #include "../shared/helpers.h"
> >  #include "git-version.h"
> >  #include "version.h"
> > +#include "systemd-notify.h"
> >
> >  static struct wl_list child_process_list;
> >  static struct weston_compositor *segv_compositor;
> > @@ -815,6 +816,8 @@ int main(int argc, char *argv[])
> >
> >         weston_compositor_wake(ec);
> >
> > +       weston_systemd_service_start(ec);
> > +
> >         wl_display_run(display);
> >
> >         /* Allow for setting return exit code after


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: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150924/a5c91df5/attachment.sig>