From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id 7C43AEEE8; Wed, 01 Oct 2025 16:06:20 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 18FDBEED8; Wed, 01 Oct 2025 16:06:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DMARC_MISSING,RCVD_IN_DNSWL_LOW,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=4.0.1 Received: from fhigh-b3-smtp.messagingengine.com (fhigh-b3-smtp.messagingengine.com [202.12.124.154]) by atuin.qyliss.net (Postfix) with ESMTPS id 3C2EBEED7 for ; Wed, 01 Oct 2025 16:06:16 +0000 (UTC) Received: from phl-compute-06.internal (phl-compute-06.internal [10.202.2.46]) by mailfhigh.stl.internal (Postfix) with ESMTP id 5C2187A0439; Wed, 1 Oct 2025 12:06:14 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-06.internal (MEProxy); Wed, 01 Oct 2025 12:06:14 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h=cc :cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm1; t=1759334774; x=1759421174; bh=DQw4ESbXwv 0FmTbYmCvJ8WOkUq0baF3aKomMFeLuCc4=; b=KYPPUYkWLj+KssOG/o9NN/0rOX 2yHAX9tVx18DCN9CAdlMoF9PHB7zTQnd58ZTM9vYp523VkGb6UxdiPuZ+y2MNoNL disWu/tyhlhvy2xEAuRvRwpQQooolR+J0zx5Eno/Q2etolM1eaZi6LYP65n2ne/k REz+GWVb1FLT/WrMks8kABSi9jozyhE53iiUuZIDVJJ2pzG3hlCwaZF3z8pOeaoh u7lvNonH0fwPRRywkuEgbFDUm7/JGD4WYIqJeKyvbHgdceznpj886XECjC8CsQJu CnIEDFZ5L/hnXBvDUyK7s5+IAc9NPRjL6aOGlIX6gY8KEzFfopJLSfB0MqxA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t= 1759334774; x=1759421174; bh=DQw4ESbXwv0FmTbYmCvJ8WOkUq0baF3aKom MFeLuCc4=; b=OWmO+vwCbUi3xJFQps+JMC7WlaazIxk9NfJtsa18fR0yey7FBwT IWa4MHjlahc8xY09e2hBVdkXGoihX7KZysVueF9WdHfHLEAOePWT01gdznta7OQi ViuWpobmPTWuoPIOYcvaI/jNoyBLs+qFDGrBFgw973eVT5hsg+6Qu/FFR1hIGbEf YneyARt8YnZdiE2KoZnbomMB34eP+ich2vxjVD7dlhDkEb5csLqogVdYzYzmJNRX TObOXa1MCs6xPNUqfJovY209oPzYOq3dLy0syG+zsK5r28aZOOcBexcEcVagm30x n96doCHbv8j/onGsI7nGPSYjfoCDvfbVfBw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggdekfeehjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpuffrtefokffrpgfnqfghnecuuegr ihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjug hrpefhvfevufgjfhffkfggtgesghdtreertddtjeenucfhrhhomheptehlhihsshgrucft ohhsshcuoehhihesrghlhihsshgrrdhisheqnecuggftrfgrthhtvghrnhepteehvedugf ejgfehhfeijeduleekleejgedvkeeuuefhhfegvdevfeetveegteeinecuvehluhhsthgv rhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhephhhisegrlhihshhsrgdrih hspdhnsggprhgtphhtthhopedvpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopegu vghmihhosggvnhhouhhrsehgmhgrihhlrdgtohhmpdhrtghpthhtohepuggvvhgvlhessh hpvggtthhruhhmqdhoshdrohhrgh X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 1 Oct 2025 12:06:13 -0400 (EDT) Received: by mbp.qyliss.net (Postfix, from userid 1000) id 8F6125F65C52; Wed, 01 Oct 2025 18:06:11 +0200 (CEST) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH v3 1/2] tools: Add adapter tool for services using sd_notify In-Reply-To: <20250928-udev-v3-1-bb0e9612c415@gmail.com> References: <20250928-udev-v3-0-bb0e9612c415@gmail.com> <20250928-udev-v3-1-bb0e9612c415@gmail.com> Date: Wed, 01 Oct 2025 18:06:09 +0200 Message-ID: <87plb6bo9a.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: 6JSYLZ37INKGLS3S7V3GVVPQLOL6ASRJ X-Message-ID-Hash: 6JSYLZ37INKGLS3S7V3GVVPQLOL6ASRJ X-MailFrom: hi@alyssa.is X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-devel.spectrum-os.org-0; header-match-devel.spectrum-os.org-1; header-match-devel.spectrum-os.org-2; header-match-devel.spectrum-os.org-3; header-match-devel.spectrum-os.org-4; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Spectrum OS Development X-Mailman-Version: 3.3.9 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Demi Marie Obenour writes: > This adapts programs using sd_notify for use with s6 readiness > notification. stdin and stdout are hard-coded for simplicity. > > Signed-off-by: Demi Marie Obenour > --- > systemd readiness notification has two > strict advantages over the s6 version: > > 1. It allows reliable reloading. > 2. It allows providing a status message that the service manager > can show in status output. > > s6 would actually benefit from both of these features. > --- > Changes since v1: > > - Hard-code file descriptors. > - Run wrapper as background process. > - Massively reduce code size. > - Use // instead of /* */ for comments. > - Check that the notification FD is a pipe and that the listening socket > is a socket. > - Rely on s6-ipc-socketbinder to create the listening socket. > - Do not unlink the listening socket. > --- > tools/default.nix | 1 + > tools/meson.build | 1 + > tools/sd-notify-adapter/meson.build | 4 + > tools/sd-notify-adapter/sd-notify-adapter.c | 127 ++++++++++++++++++++++= ++++++ > 4 files changed, 133 insertions(+) > > diff --git a/tools/default.nix b/tools/default.nix > index f1157f0072f58c2ad9e741ca60bab2ed6507b72d..3634ef8ee892697b1bc7fff25= 9eaccd54fddcd2f 100644 > --- a/tools/default.nix > +++ b/tools/default.nix > @@ -74,6 +74,7 @@ stdenv.mkDerivation (finalAttrs: { > ./lsvm > ./start-vmm > ./subprojects > + ./sd-notify-adapter Sort. > ] ++ lib.optionals driverSupport [ > ./xdp-forwarder > ])); > diff --git a/tools/meson.build b/tools/meson.build > index 17b4c16c07c9c6847306c47fbb7fe54f5a6e8cc5..d679afa64966d7b237f332096= 281a9bc9ef76e94 100644 > --- a/tools/meson.build > +++ b/tools/meson.build > @@ -23,6 +23,7 @@ if get_option('host') >=20=20 > subdir('lsvm') > subdir('start-vmm') > + subdir('sd-notify-adapter') > endif >=20=20 > if get_option('app') > diff --git a/tools/sd-notify-adapter/meson.build b/tools/sd-notify-adapte= r/meson.build > new file mode 100644 > index 0000000000000000000000000000000000000000..6032a3a7704d49cae0655b43d= 0189444d3b15e4d > --- /dev/null > +++ b/tools/sd-notify-adapter/meson.build > @@ -0,0 +1,4 @@ > +# SPDX-License-Identifier: ISC Atypical license. > +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour > + > +executable('sd-notify-adapter', 'sd-notify-adapter.c', install: true) > diff --git a/tools/sd-notify-adapter/sd-notify-adapter.c b/tools/sd-notif= y-adapter/sd-notify-adapter.c > new file mode 100644 > index 0000000000000000000000000000000000000000..2767dea30b8db69986d9c2a02= d6be28d205b3b4b > --- /dev/null > +++ b/tools/sd-notify-adapter/sd-notify-adapter.c > @@ -0,0 +1,127 @@ > +// SPDX-License-Identifier: MIT > +// SPDX-FileCopyrightText: 2025 Demi Marie Obenour > + > +#define _GNU_SOURCE 1 This should be done in the build system, like how other programs in Spectrum do it. (That way we can't forget it needs to be set before any headers are included.) > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include I used to like sysexits.h too, but the BSD systems they originally came from now consider them bad practice and discourage them in new code, so that's why other code in Spectrum doesn't use them, and just uses EXIT_FAILURE. > +#include > + > +#define ARRAY_SIZE(s) (sizeof(s)/sizeof(s[0])) > + > +static bool ready; > + > +enum { > + socket_fd, > + notification_fd, > +}; > + > +#define READY "READY=3D1" > +#define READY_SIZE (sizeof(READY) - 1) > + > +static void > +process_notification(struct iovec *const msg) { > + ssize_t data =3D recv(socket_fd, msg->iov_base, msg->iov_len, > + MSG_DONTWAIT | MSG_TRUNC | MSG_PEEK); Why MSG_DONTWAIT? We know there's data waiting for us. I find "data" to be quite a confusing name for a size. > + if (data =3D=3D -1) { > + if (errno =3D=3D EINTR) { > + return; // signal caught > + } > + if (errno =3D=3D EAGAIN || errno =3D=3D EWOULDBLOCK) { > + return; // spurious wakeup > + } > + err(EX_OSERR, "recv from notification socket"); > + } > + assert(data >=3D 0 && data <=3D INT_MAX); Why do we care about the upper range? We cast it to size_t after this, not int. > + size_t size =3D (size_t)data; > + if (size =3D=3D 0) > + return; // avoid arithmetic on NULL pointer > + if (size > msg->iov_len) { > + char *b =3D (size =3D=3D 0 ? malloc(size) : realloc(msg->iov_base, siz= e)); We already know size !=3D 0 at this point. > + if (b =3D=3D NULL) { > + err(EX_OSERR, "allocation failure"); > + } > + msg->iov_base =3D b; We could just msg->iov_base =3D realloc(=E2=80=A6), without the b variable. > + msg->iov_len =3D size; > + } > + data =3D recv(socket_fd, msg->iov_base, msg->iov_len, > + MSG_CMSG_CLOEXEC | MSG_DONTWAIT | MSG_TRUNC); > + if (data < 0) { > + if (errno =3D=3D EINTR || errno =3D=3D EAGAIN || errno =3D=3D EWOULDBL= OCK) > + return; > + err(EX_OSERR, "recv from notification socket"); > + } EAGAIN or EWOULDBLOCK would be quite unexpected. > + for (char *next, *cursor =3D msg->iov_base, *end =3D cursor + size; > + cursor !=3D NULL; cursor =3D (next =3D=3D NULL ? NULL : next + 1))= { > + next =3D memchr(cursor, '\n', (size_t)(end - cursor)); > + size_t message_size =3D (size_t)((next =3D=3D NULL ? end : next) - cur= sor); > + if (message_size =3D=3D READY_SIZE && > + memcmp(cursor, READY, READY_SIZE) =3D=3D 0) { > + data =3D write(notification_fd, "\n", 1); > + if (data !=3D 1) { > + err(EX_OSERR, "writing to notification descriptor"); > + } > + exit(0); > + } > + } > +} > + > +int main(int argc, char **argv [[gnu::unused]]) { > + if (argc !=3D 1) { > + errx(EX_USAGE, "stdin is listening socket, stdout is notification pipe= "); > + } This is C23 =E2=80=94 you can just write int main(int argc, char **). > + // Main event loop. > + struct iovec v =3D { > + .iov_base =3D NULL, > + .iov_len =3D 0, > + }; > + for (;;) { > + struct pollfd p[] =3D { > + { > + .fd =3D socket_fd, > + .events =3D POLLIN, > + .revents =3D 0, > + }, > + { > + .fd =3D notification_fd, > + .events =3D 0, > + .revents =3D 0, > + }, > + }; > + int r =3D poll(p, 2, -1); Might as well use ARRAY_SIZE here, given we have it. > + if (r < 0) { > + if (errno =3D=3D EINTR) > + continue; > + err(EX_OSERR, "poll"); > + } > + if (p[0].revents) { > + if (p[0].revents & POLLERR) > + errx(EX_OSERR, "unexpected POLLERR"); > + if (p[0].revents & POLLIN) > + process_notification(&v); > + break; break? So when we get a non-READY message, we just exit? > + } > + if (p[1].revents) { > + if (ready) { > + // Normal exit > + return 0; > + } ready is never written, so this never happens. > + errx(EX_PROTOCOL, "s6 closed its pipe before the child was ready"); > + } > + } > +} > > --=20 > 2.51.0 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQRV/neXydHjZma5XLJbRZGEIw/wogUCaN1RcQAKCRBbRZGEIw/w oki5AQD+7mSbj/I4ZIXUQjgQz3VEH7KbBjqkadH2ccbQ6QxJTAD/QscAN3Nl5EM3 LIzY19tn2JnKy4/+fe8wrvr5/n7NoQI= =B2fI -----END PGP SIGNATURE----- --=-=-=--