Re: [libvirt] [PATCH] systemd: directly notify systemd instead of using sd_notify

On Mon, Jun 06, 2016 at 04:46:50PM +0200, Peter Krempa wrote: > On Mon, Jun 06, 2016 at 15:16:50 +0100, Daniel Berrange wrote: > > The sd_notify method is used to tell systemd when libvirtd > > has finished starting up. All it does is send a datagram > > containing the string parameter to systemd on a UNIX socket > > named in the NOTIFY_SOCKET environment variable. Rather than > > pulling in the systemd libraries for this, just code the > > notification directly in libvirt as this is a stable ABI > > from systemd's POV which explicitly allows independant > > implementations: > > > > See "Reimplementable Independently" column in the > > "$NOTIFY_SOCKET Daemon Notifications" row: > > > > https://www.freedesktop.org/wiki/Software/systemd/InterfacePortabilityAnd... > > > > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; > > --- > > configure.ac | 2 -- > > libvirt.spec.in | 12 ----------- > > m4/virt-systemd-daemon.m4 | 34 ------------------------------- > > src/Makefile.am | 4 ++-- > > src/util/virsystemd.c | 52 ++++++++++++++++++++++++++++++++++++++++++----- > > 5 files changed, 49 insertions(+), 55 deletions(-) > > delete mode 100644 m4/virt-systemd-daemon.m4 > > [...] > > > diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c > > index 4883f94..71b8f33 100644 > > --- a/src/util/virsystemd.c > > +++ b/src/util/virsystemd.c > > [...] > > > @@ -480,9 +482,49 @@ int virSystemdTerminateMachine(const char *name) > > void > > virSystemdNotifyStartup(void) > > { > > -#ifdef WITH_SYSTEMD_DAEMON > > - sd_notify(0, "READY=1"); > > -#endif > > +#ifdef HAVE_SYS_UN_H > > + const char *path; > > + const char *msg = "READY=1"; > > + int fd; > > + struct sockaddr_un un = { > > + .sun_family = AF_UNIX, > > + }; > > + struct iovec iov = { > > + .iov_base = (char *)msg, > > + .iov_len = strlen(msg), > > + }; > > + struct msghdr mh = { > > + .msg_name = &un, > > + .msg_namelen = sizeof(un), > > + .msg_iov = &iov, > > + .msg_iovlen = 1, > > + }; > > + > > + if (!(path = virGetEnvBlockSUID("NOTIFY_SOCKET"))) { > > + VIR_DEBUG("Skipping systemd notify, not requested"); > > + return; > > + } > > + > > + if (strlen(path) > sizeof(un.sun_path)) { > > Off-by-one by not considering the trailing \0 UNIX socket addresses are *not* NUL-terminated strings. The full 108 bytes in the 'sun_path' field are considered to be the path. So even if you do have a NUL in your string, everything following it is also considered part of the address - its why its important to ensure the sun_path is set to all-zeros :-)