Currently the systemd service for seatd uses Type=simple.
This is somewhat problematic as it doesn't provide a method to inform the systemd service manager when the seatd service is actually started up and fully ready, as such a dependent service may be started before the seatd daemon is ready and the seatd socket is available resulting in that dependent service failing to start up.
See documentation for sd_notify.
From my understanding we should have a meson build option to enable sd_notify support when libsystemd is present as well as change the example seatd.service file to use Type=notify instead of Type=simple.
seatd offers readiness through the s6 protocol - see
-n.If there is a good usecase, it could possibly be adapted to allow the message to be customized to match the sd-notify format - or maybe just always write READY=1. Linking with libsystemd just for notify seems silly though.
seatd offers readiness through the s6 protocol - see -n.
Best I can tell systemd services do not support s6 protocol.
Linking with libsystemd just for notify seems silly though.
I think linking to libsystemd should be fine, any systemd based system should always have libsystemd available and we pull libsystemd in when building seatd with
libseat-logindsupport already so it's not like we'd be adding an entirely new dependency.
Just happened to notice this issue, this looks like sdnotify-wrapper was designed for this sort of thing.
Best I can tell systemd services do not support s6 protocol.
I vaguely remember the two protocols overlapping if READY=1\n is sent.
I think linking to libsystemd should be fine ... and we pull libsystemd in when building seatd with libseat-logind support already
seatd never links with libsystemd - use-cases where one builds and uses libseat with logind support, and use-cases where one builds and runs seatd as a system service do not really overlap - and I do not intend on adding ifdefs and dependencies just to write a fixed string to an fd. :)
I do not intend on adding ifdefs and dependencies just to write a fixed string to an fd.
The implementation seems to be slightly more complex than that, not sure if this handles all the edge cases properly but I think it would be something like this.
diff --git a/seatd/seatd.c b/seatd/seatd.c index f88e6c9..07b628b 100644 --- a/seatd/seatd.c +++ b/seatd/seatd.c @@ -56,6 +56,74 @@ error: return -1; } +union sockaddr_union { + struct sockaddr sa; + struct sockaddr_un un; +}; + +static int systemd_notify(const char *state) { + int fd = -1, r; + struct msghdr msghdr; + struct iovec iovec; + union sockaddr_union sockaddr; + const char *e; + + if (!state) { + r = -EINVAL; + goto finish; + } + + e = getenv("NOTIFY_SOCKET"); + if (!e) + return 0; + + /* Must be an abstract socket, or an absolute path */ + if ((e[0] != '@' && e[0] != '/') || e[1] == 0) { + r = -EINVAL; + goto finish; + } + + fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0); + if (fd < 0) { + r = -errno; + goto finish; + } + + memset(&sockaddr, 0, sizeof(sockaddr)); + sockaddr.sa.sa_family = AF_UNIX; + strncpy(sockaddr.un.sun_path, e, sizeof(sockaddr.un.sun_path)); + + if (sockaddr.un.sun_path[0] == '@') + sockaddr.un.sun_path[0] = 0; + + memset(&iovec, 0, sizeof(iovec)); + iovec.iov_base = (char*) state; + iovec.iov_len = strlen(state); + + memset(&msghdr, 0, sizeof(msghdr)); + msghdr.msg_name = &sockaddr; + msghdr.msg_namelen = offsetof(struct sockaddr_un, sun_path) + strlen(e); + + if (msghdr.msg_namelen > sizeof(struct sockaddr_un)) + msghdr.msg_namelen = sizeof(struct sockaddr_un); + + msghdr.msg_iov = &iovec; + msghdr.msg_iovlen = 1; + + if (sendmsg(fd, &msghdr, MSG_NOSIGNAL) < 0) { + r = -errno; + goto finish; + } + + r = 1; + +finish: + if (fd >= 0) + close(fd); + + return r; +} + int main(int argc, char *argv[]) { const char *usage = "Usage: seatd [options]\n" "\n" @@ -193,6 +261,8 @@ int main(int argc, char *argv[]) { log_info("seatd started"); + systemd_notify("READY=1\n"); + if (readiness != -1) { if (write(readiness, "\n", 1) == -1) { log_errorf("Could not write readiness signal: %s\n", strerror(errno));
The diff would be something like this:
diff --git a/seatd/seatd.c b/seatd/seatd.c index f88e6c9..5ccf52c 100644 --- a/seatd/seatd.c +++ b/seatd/seatd.c @@ -194,7 +194,8 @@ int main(int argc, char *argv[]) { log_info("seatd started"); if (readiness != -1) { - if (write(readiness, "\n", 1) == -1) { + const char ready_str[] = "READY=1\n"; + if (write(readiness, ready_str, strlen(ready_str)) != strlen(ready_str)) { log_errorf("Could not write readiness signal: %s\n", strerror(errno)); } close(readiness);There is no need for sendmsg for a simple string, and the service file can specify $NOTIFY_SOCKET as the
-nparameter.
the service file can specify $NOTIFY_SOCKET as the -n parameter.
How would that work? The normal
sd_notifyseems to require some logic around parsing/converting theNOTIFY_SOCKETenv variable.
Hmm, from what I can tell from systemd source, NOTIFY_SOCKET is always a path to the socket, so it should not need parsing. It might need opening though.
For reference, the "READY=1\n" thing was what was implemented for swaylock over in https://github.com/swaywm/swaylock/pull/281, where I believe we made the assumption that you'd be able to get an fd one way or another... Maybe emersion remembers the details?
There's a lot of fallback/special handling in the function parsing NOTIFY_SOCKET from what I can tell.
I wrote a quick patch that implements sd_notify with status and error code reporting.