[PATCH weston v2] compositor: systemd notifications support

Bryce Harrington bryce at osg.samsung.com
Fri Sep 25 15:17:11 PDT 2015


On Fri, Sep 25, 2015 at 02:38:21PM -0500, Derek Foreman wrote:
> On 25/09/15 02:09 PM, Bryce Harrington wrote:
> > Hi Egor, welcome to the Wayland project, and thanks for this
> > contribution!
> > 
> > On Fri, Sep 25, 2015 at 06:00:27PM +0300, Egor Starkov wrote:
> >> Add systemd status and watchdog notification support.
> >> Feature is not compiled by default and can be enabled by
> >> "--enable-systemd-notify" configuration flag. It compiles
> >> into module "systemd-notify.so" and can be loaded by
> >> adding it in weston.ini like any other module, i.e.
> >> "modules=systemd-notify.so". Watchdog timeout equals to
> >> half of timeout defined by "WATCHDOG_USEC" environment
> >> variable, which is set by "WatchdogSec=" setting in
> >> service file.
> >>
> >> Signed-off-by: Egor Starkov <egor.starkov at ge.com>
> >> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > 
> > Ok, so if I understand this properly, this adds optional functionality
> > to make weston notify systemd when it's finished initializing, and also
> > optionally lets systemd set a watchdog to make weston ping back a
> > configurable number of msecs.  Presumably this'd allow triggering crash
> > and lockup handling functionalities to be hooked in?
> > 
> > LGTM,
> > 
> > Reviewed-by: Bryce Harrington <bryce at osg.samsung.com>
> 
> Looks fine to me as well,
> Reviewed-by: Derek Foreman <derekf at osg.samsung.com>

There was one small distcheck breakage (below), which I'll fix in a
followup patch.  

To ssh://git.freedesktop.org/git/wayland/weston
   892122e..7ce2e97  master -> master
 
> >> ---
> >>  Makefile.am          |  19 +++++++-
> >>  configure.ac         |  13 ++++++
> >>  src/main.c           |   1 +
> >>  src/systemd-notify.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  src/systemd-notify.h |  47 ++++++++++++++++++++
> >>  5 files changed, 202 insertions(+), 1 deletion(-)
> >>  create mode 100644 src/systemd-notify.c
> >>  create mode 100644 src/systemd-notify.h
> >>
> >> diff --git a/Makefile.am b/Makefile.am
> >> index 62719c9..5c953a8 100644
> >> --- a/Makefile.am
> >> +++ b/Makefile.am
> >> @@ -95,7 +95,24 @@ weston_SOURCES =					\
> >>  	shared/timespec-util.h				\
> >>  	shared/zalloc.h					\
> >>  	shared/platform.h				\
> >> -	src/weston-egl-ext.h
> >> +	src/weston-egl-ext.h				\
> >> +	src/systemd-notify.h
> >> +
> >> +if SYSTEMD_NOTIFY_SUPPORT
> >> +module_LTLIBRARIES += systemd-notify.la
> >> +systemd_notify_la_LDFLAGS = -module -avoid-version
> >> +systemd_notify_la_LIBADD = $(SYSTEMD_DAEMON_LIBS)
> >> +systemd_notify_la_CFLAGS =			\
> >> +	$(SYSTEMD_DAEMON_LIBS)			\
> >> +	$(PIXMAN_CFLAGS)			\
> >> +	$(AM_CFLAGS)
> >> +systemd_notify_la_SOURCES =			\
> >> +	src/systemd-notify.c			\
> >> +	src/systemd-notify.h			\
> >> +	shared/helpers.h			\
> >> +	shared/zalloc.h				\
> >> +	compositor.h

s/b src/compositor.h

> >> +endif
> >>  
> >>  nodist_weston_SOURCES =					\
> >>  	protocol/screenshooter-protocol.c		\
> >> diff --git a/configure.ac b/configure.ac
> >> index 1978705..b763515 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -565,6 +565,18 @@ if test x$wayland_scanner = x; then
> >>  	wayland_scanner=`$PKG_CONFIG --variable=wayland_scanner wayland-scanner`
> >>  fi
> >>  
> >> +AC_ARG_ENABLE(systemd_notify,
> >> +              AS_HELP_STRING([--enable-systemd-notify],
> >> +                             [Enables systemd notifications to
> >> +                              notify systemd about weston state
> >> +                              and update watchdog.]),,
> >> +              enable_systemd_notify=no)
> >> +AM_CONDITIONAL(SYSTEMD_NOTIFY_SUPPORT, test x$enable_systemd_notify = xyes)
> >> +if test "x$enable_systemd_notify" = "xyes"; then
> >> +  AC_DEFINE([SYSTEMD_NOTIFY_SUPPORT], [1], [Build the systemd sd_notify support])
> >> +  PKG_CHECK_MODULES(SYSTEMD_DAEMON, [libsystemd])
> >> +fi
> >> +
> >>  AC_CONFIG_FILES([Makefile src/version.h src/weston.pc])
> >>  
> >>  AM_CONDITIONAL([HAVE_GIT_REPO], [test -f $srcdir/.git/logs/HEAD])
> >> @@ -590,6 +602,7 @@ AC_MSG_RESULT([
> >>  
> >>  	weston-launch utility		${enable_weston_launch}
> >>  	systemd-login support		${have_systemd_login}
> >> +	systemd notify support		${enable_systemd_notify}
> >>  
> >>  	DRM Compositor			${enable_drm_compositor}
> >>  	X11 Compositor			${enable_x11_compositor}
> >> diff --git a/src/main.c b/src/main.c
> >> index a98570e..937bed2 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;
> >> diff --git a/src/systemd-notify.c b/src/systemd-notify.c
> >> new file mode 100644
> >> index 0000000..e61db0f
> >> --- /dev/null
> >> +++ b/src/systemd-notify.c
> >> @@ -0,0 +1,123 @@
> >> +/*
> >> + * Copyright (c) 2015 General Electric Company. All rights reserved.
> >> + *
> >> + * 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 "config.h"
> >> +
> >> +#include <errno.h>
> >> +#include <stdlib.h>
> >> +#include <systemd/sd-daemon.h>
> >> +#include <wayland-server.h>
> >> +#include "shared/helpers.h"
> >> +#include "shared/zalloc.h"
> >> +#include "compositor.h"
> >> +
> >> +struct systemd_notifier {
> >> +	int watchdog_time;
> >> +	struct wl_event_source *watchdog_source;
> >> +	struct wl_listener compositor_destroy_listener;
> >> +};
> >> +
> >> +static int
> >> +watchdog_handler(void *data)
> >> +{
> >> +	struct systemd_notifier *notifier = data;
> >> +
> >> +	wl_event_source_timer_update(notifier->watchdog_source,
> >> +				     notifier->watchdog_time);
> >> +
> >> +	sd_notify(0, "WATCHDOG=1");
> >> +
> >> +	return 1;
> >> +}
> >> +
> >> +static void
> >> +weston_compositor_destroy_listener(struct wl_listener *listener, void *data)
> >> +{
> >> +	struct systemd_notifier *notifier;
> >> +
> >> +	sd_notify(0, "STOPPING=1");
> >> +
> >> +	notifier = container_of(listener,
> >> +				struct systemd_notifier,
> >> +				compositor_destroy_listener);
> >> +
> >> +	if (notifier->watchdog_source)
> >> +		wl_event_source_remove(notifier->watchdog_source);
> >> +
> >> +	wl_list_remove(&notifier->compositor_destroy_listener.link);
> >> +	free(notifier);
> >> +}
> >> +
> >> +WL_EXPORT int
> >> +module_init(struct weston_compositor *compositor,
> >> +	    int *argc, char *argv[])
> >> +{
> >> +	char *tail;
> >> +	char *watchdog_time_env;
> >> +	struct wl_event_loop *loop;
> >> +	long watchdog_time_conv;
> >> +	struct systemd_notifier *notifier;
> >> +
> >> +	notifier = zalloc(sizeof *notifier);
> >> +	if (notifier == NULL)
> >> +		return -1;
> >> +
> >> +	notifier->compositor_destroy_listener.notify =
> >> +		weston_compositor_destroy_listener;
> >> +	wl_signal_add(&compositor->destroy_signal,
> >> +		      &notifier->compositor_destroy_listener);
> >> +
> >> +	sd_notify(0, "READY=1");
> >> +
> >> +	/* 'WATCHDOG_USEC' is environment variable that is set
> >> +	 * by systemd to transfer 'WatchdogSec' watchdog timeout
> >> +	 * setting from service file.*/
> >> +	watchdog_time_env = getenv("WATCHDOG_USEC");
> >> +
> >> +	if (!watchdog_time_env)
> >> +		return 0;
> >> +
> >> +	errno = 0;
> >> +	watchdog_time_conv = strtol(watchdog_time_env, &tail, 0);
> >> +	if ((errno != 0) || (*tail != '\0'))
> >> +		return 0;
> >> +
> >> +	/* Convert 'WATCHDOG_USEC' to milliseconds and notify
> >> +	 * systemd every half of that time.*/
> >> +	watchdog_time_conv /= 1000 * 2;
> >> +	if (watchdog_time_conv <= 0)
> >> +		return 0;
> >> +
> >> +	notifier->watchdog_time = watchdog_time_conv;
> >> +
> >> +	loop = wl_display_get_event_loop(compositor->wl_display);
> >> +	notifier->watchdog_source =
> >> +		wl_event_loop_add_timer(loop, watchdog_handler, notifier);
> >> +	wl_event_source_timer_update(notifier->watchdog_source,
> >> +				     notifier->watchdog_time);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> diff --git a/src/systemd-notify.h b/src/systemd-notify.h
> >> new file mode 100644
> >> index 0000000..947875e
> >> --- /dev/null
> >> +++ b/src/systemd-notify.h
> >> @@ -0,0 +1,47 @@
> >> +/*
> >> + * Copyright (c) 2015 General Electric Company. All rights reserved.
> >> + *
> >> + * 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 _SYSTEMD_NOTIFY_H_
> >> +#define _SYSTEMD_NOTIFY_H_
> >> +
> >> +#include "config.h"
> >> +
> >> +#ifdef SYSTEMD_NOTIFY_SUPPORT
> >> +
> >> +struct weston_compositor;
> >> +
> >> +void
> >> +weston_systemd_service_start(struct weston_compositor *compositor);
> >> +
> >> +#else
> >> +
> >> +static inline void
> >> +weston_systemd_service_start(struct weston_compositor *compositor)
> >> +{
> >> +}
> >> +
> >> +#endif //SYSTEMD_NOTIFY_SUPPORT
> >> +
> >> +#endif //_SYSTEMD_NOTIFY_H_
> >> -- 
> >> 1.8.3.1
> >>
> >> _______________________________________________
> >> wayland-devel mailing list
> >> wayland-devel at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> > 


More information about the wayland-devel mailing list