~kennylevinsen/seatd#20: 
Seatd should support systemd sd_notify

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.

Status
REPORTED
Submitter
~jameshilliard
Assigned to
No-one
Submitted
6 months ago
Updated
6 months ago
Labels
enhancement seatd

~kennylevinsen 6 months ago

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.

~jameshilliard 6 months ago

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-logind support already so it's not like we'd be adding an entirely new dependency.

~aren 6 months ago

Just happened to notice this issue, this looks like sdnotify-wrapper was designed for this sort of thing.

~kennylevinsen 6 months ago

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. :)

~jameshilliard 6 months ago

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));

~kennylevinsen 6 months ago*

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 -n parameter.

~jameshilliard 6 months ago

the service file can specify $NOTIFY_SOCKET as the -n parameter.

How would that work? The normal sd_notify seems to require some logic around parsing/converting the NOTIFY_SOCKET env variable.

~kennylevinsen 6 months ago

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?

~jameshilliard 6 months ago

There's a lot of fallback/special handling in the function parsing NOTIFY_SOCKET from what I can tell.

~jameshilliard 6 months ago

I wrote a quick patch that implements sd_notify with status and error code reporting.

Register here or Log in to comment, or comment via email.