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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 06, 2016 at 15:50:46 +0100, Daniel Berrange wrote:
> 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/InterfacePortabilityAndStabilityChart/
> > > 
> > > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> > > ---
> > >  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 :-)

Ah. I learned something at least.

But then there's still a problem as virStrcpy doesn't know about this
unix socket quirk and it subtracts '1' from the third argument before
comparing it with the length of the second argument and won't copy
anything in the corner case if the path has exactly 108 bytes.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]