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.star...@ge.com>
>> Reviewed-by: Pekka Paalanen <pekka.paala...@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 <br...@osg.samsung.com>
Looks fine to me as well,
Reviewed-by: Derek Foreman <der...@osg.samsung.com>
>> ---
>> 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
>> +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(¬ifier->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,
>> + ¬ifier->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@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
