James Hilliard: 1 seatd: Add systemd notify support 4 files changed, 99 insertions(+), 2 deletions(-)
Two updates: First, I have made this "proper" sd_notify adapter: https://git.sr.ht/~kennylevinsen/readyfd2sd It uses libsystemd for notification and maintains that the direct child of systemd is the started service with the monitoring process being forked off before exec. As such it has the right systemd main pid without giving off a systemd alien child warnings, looks clean in the process tree, and supports all the sd_notify socket types. Second, I am reconsidering whether we should allow this in a minimal (only readiness), and strictly optional form. While systemd will not be seen in a positive light around here - projects like seatd and elogind would not really need to exist if systemd accepted cooperation with other communities - I do not want our policy to be hostility to systemd users. I need to find a suitable balance here.
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~kennylevinsen/seatd-devel/patches/43688/mbox | git am -3Learn more about email & git
This lets systemd know when the service is ready and informs systemd of any errors that occur. Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- contrib/systemd/seatd.service | 2 +- meson.build | 12 ++++- meson_options.txt | 1 + seatd/seatd.c | 86 +++++++++++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 2 deletions(-) diff --git a/contrib/systemd/seatd.service b/contrib/systemd/seatd.service index bbbaf23..e7f9ca3 100644 --- a/contrib/systemd/seatd.service +++ b/contrib/systemd/seatd.service @@ -3,7 +3,7 @@ Description=Seat management daemon Documentation=man:seatd(1) [Service] -Type=simple +Type=notify # Specify the group you'd like to grant access to seatd ExecStart=seatd -g seat Restart=always diff --git a/meson.build b/meson.build index 516d7d2..001cbd3 100644 --- a/meson.build +++ b/meson.build @@ -124,6 +124,7 @@ server_files = [ with_seatd = get_option('libseat-seatd') == 'enabled' with_builtin = get_option('libseat-builtin') == 'enabled' with_server = get_option('server') == 'enabled' +with_server_systemd = get_option('server-systemd') == 'enabled' if with_seatd or with_builtin private_files += 'libseat/backend/seatd.c' @@ -215,12 +216,20 @@ libseat = declare_dependency( meson.override_dependency('libseat', libseat) if with_server + seatd_deps = [realtime] + seatd_c_args = [] + if with_server_systemd + libsystemd = dependency('libsystemd', required: true) + seatd_deps += libsystemd + seatd_c_args += '-DSEATD_SERVER_SYSTEMD=1' + endif executable( 'seatd', [ server_files, 'seatd/seatd.c' ], include_directories: [include_directories('.', 'include')], install: true, - dependencies: [realtime], + dependencies: seatd_deps, + c_args: seatd_c_args, ) executable( 'seatd-launch', @@ -287,4 +296,5 @@ summary({ 'libseat-systemd': logind.found() and logind.name() == 'libsystemd', 'libseat-elogind': logind.found() and logind.name() == 'libelogind', 'server': with_server, + 'server-systemd': with_server_systemd, }, bool_yn: true) diff --git a/meson_options.txt b/meson_options.txt index 0731ac4..a1f639d 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -2,6 +2,7 @@ option('libseat-logind', type: 'combo', choices: ['auto', 'disabled', 'elogind', option('libseat-seatd', type: 'combo', choices: ['enabled', 'disabled'], value: 'enabled', description: 'seatd support for libseat') option('libseat-builtin', type: 'combo', choices: ['enabled', 'disabled'], value: 'disabled', description: 'built-in seatd server for libseat') option('server', type: 'combo', choices: ['enabled', 'disabled'], value: 'enabled', description: 'seatd server') +option('server-systemd', type: 'combo', choices: ['enabled', 'disabled'], value: 'disabled', description: 'seatd server systemd notify support') option('examples', type: 'combo', choices: ['enabled', 'disabled'], value: 'disabled', description: 'libseat example programs') option('man-pages', type: 'feature', value: 'auto', description: 'Generate and install man pages') option('defaultpath', type: 'string', value: '', description: 'Default location for seatd socket (empty for default)') diff --git a/seatd/seatd.c b/seatd/seatd.c index f88e6c9..65c801a 100644 --- a/seatd/seatd.c +++ b/seatd/seatd.c @@ -10,6 +10,10 @@ #include <sys/stat.h> #include <sys/un.h> +#if defined(SEATD_SERVER_SYSTEMD) +#include <systemd/sd-daemon.h> +#endif + #include "client.h" #include "log.h" #include "poller.h" @@ -25,6 +29,10 @@ static int open_socket(const char *path, int uid, int gid) { int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0); if (fd == -1) { log_errorf("Could not create socket: %s", strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not create socket: %s\n" + "ERRNO=%i", strerror(errno), errno); +#endif return -1; } @@ -33,20 +41,36 @@ static int open_socket(const char *path, int uid, int gid) { socklen_t size = offsetof(struct sockaddr_un, sun_path) + strlen(addr.unix.sun_path); if (bind(fd, &addr.generic, size) == -1) { log_errorf("Could not bind socket: %s", strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not bind socket: %s\n" + "ERRNO=%i", strerror(errno), errno); +#endif goto error; } if (listen(fd, LISTEN_BACKLOG) == -1) { log_errorf("Could not listen on socket: %s", strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not listen on socket: %s\n" + "ERRNO=%i", strerror(errno), errno); +#endif goto error; } if (uid != -1 || gid != -1) { if (chmod(path, 0770) == -1) { log_errorf("Could not chmod socket: %s", strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not chmod socket: %s\n" + "ERRNO=%i", strerror(errno), errno); +#endif goto error; } if (chown(path, uid, gid) == -1) { log_errorf("Could not chown socket to uid %d, gid %d: %s", uid, gid, strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not chown socket to uid %d, gid %d: %s\n" + "ERRNO=%i", uid, gid, strerror(errno), errno); +#endif goto error; } } @@ -79,17 +103,29 @@ int main(int argc, char *argv[]) { readiness = atoi(optarg); if (readiness < 0) { fprintf(stderr, "Invalid readiness fd: %s\n", optarg); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Invalid readiness fd: %s\n" + "ERRNO=%i", optarg, EINVAL); +#endif return 1; } break; case 'u': { if (!chown_socket) { fprintf(stderr, "-u/-g and -z are mutually exclusive\n"); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=-u/-g and -z are mutually exclusive\n" + "ERRNO=%i", EINVAL); +#endif return 1; } struct passwd *pw = getpwnam(optarg); if (pw == NULL) { fprintf(stderr, "Could not find user by name '%s'.\n", optarg); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not find user by name '%s'.\n" + "ERRNO=%i", optarg, ENOENT); +#endif return 1; } else { uid = pw->pw_uid; @@ -99,11 +135,19 @@ int main(int argc, char *argv[]) { case 'g': { if (!chown_socket) { fprintf(stderr, "-u/-g and -z are mutually exclusive\n"); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=-u/-g and -z are mutually exclusive\n" + "ERRNO=%i", EINVAL); +#endif return 1; } struct group *gr = getgrnam(optarg); if (gr == NULL) { fprintf(stderr, "Could not find group by name '%s'.\n", optarg); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not find group by name '%s'.\n" + "ERRNO=%i", optarg, ENOENT); +#endif return 1; } else { gid = gr->gr_gid; @@ -121,6 +165,10 @@ int main(int argc, char *argv[]) { level = LIBSEAT_LOG_LEVEL_SILENT; } else { fprintf(stderr, "Invalid loglevel: %s\n", optarg); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Invalid loglevel: %s\n" + "ERRNO=%i", optarg, EINVAL); +#endif return 1; } break; @@ -130,6 +178,10 @@ int main(int argc, char *argv[]) { // seatd-launch takes care of ownership. if (uid != -1 || gid != -1) { fprintf(stderr, "-u/-g and -z are mutually exclusive\n"); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=-u/-g and -z are mutually exclusive\n" + "ERRNO=%i", EINVAL); +#endif return 1; } unlink_existing_socket = false; @@ -167,6 +219,10 @@ int main(int argc, char *argv[]) { log_infof("Removing leftover socket at %s", SEATD_DEFAULTPATH); if (unlink(SEATD_DEFAULTPATH) == -1) { log_errorf("Could not remove leftover socket: %s", strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not remove leftover socket: %s\n" + "ERRNO=%i", strerror(errno), errno); +#endif return 1; } } @@ -175,6 +231,10 @@ int main(int argc, char *argv[]) { struct server server = {0}; if (server_init(&server) == -1) { log_errorf("server_init failed: %s", strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=server_init failed: %s\n" + "ERRNO=%i", strerror(errno), errno); +#endif return 1; } @@ -187,15 +247,26 @@ int main(int argc, char *argv[]) { if (poller_add_fd(&server.poller, socket_fd, EVENT_READABLE, server_handle_connection, &server) == NULL) { log_errorf("Could not add socket to poller: %s", strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not add socket to poller: %s\n" + "ERRNO=%i", strerror(errno), errno); +#endif close(socket_fd); goto error_socket; } log_info("seatd started"); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notify(0, "STATUS=seatd started\nREADY=1"); +#endif if (readiness != -1) { if (write(readiness, "\n", 1) == -1) { log_errorf("Could not write readiness signal: %s\n", strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not write readiness signal: %s\n" + "ERRNO=%i", strerror(errno), errno); +#endif } close(readiness); } @@ -203,6 +274,10 @@ int main(int argc, char *argv[]) { while (server.running) { if (poller_poll(&server.poller) == -1) { log_errorf("Poller failed: %s", strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Poller failed: %s\n" + "ERRNO=%i", strerror(errno), errno); +#endif goto error_socket; } } @@ -212,9 +287,20 @@ int main(int argc, char *argv[]) { error_socket: if (unlink(SEATD_DEFAULTPATH) == -1) { log_errorf("Could not remove socket: %s", strerror(errno)); +#if defined(SEATD_SERVER_SYSTEMD) + sd_notifyf(0, "STATUS=Could not remove socket: %s\n" + "ERRNO=%i", strerror(errno), errno); +#endif } error_server: server_finish(&server); log_info("seatd stopped"); +#if defined(SEATD_SERVER_SYSTEMD) + if (ret == 0) { + sd_notify(0, "STATUS=seatd stopped\nSTOPPING=1"); + } else { + sd_notify(0, "STOPPING=1"); + } +#endif return ret; } -- 2.34.1
systemd integration was already discussed here: https://lists.sr.ht/~kennylevinsen/seatd-devel/%3C20230127104421.4075465-1-m.tretter%40pengutronix.de%3E I would personally prefer not to have any systemd-specific stuff inside seatd, only service manager independent logic, wired up to systemd in the service file.