[DTrace-devel] [PATCH v2 01/14] No longer depend on libsystemd
Kris Van Hees
kris.van.hees at oracle.com
Tue Oct 29 18:27:41 UTC 2024
You are your own enemy :) Since you chose to use all spaces for indentation
in your configure script, you now introduced inconsistency by adding lines
with tabs! See below...
On Mon, Oct 28, 2024 at 09:17:50PM +0000, Nick Alcock wrote:
> We only need this for the systemd notification protocol, and that's so
> simple there is MIT-0-licensed reusable code in the sd_notify manpage
> showing you how to use it. That code looks pretty much OK, so I modified it
> lightly and pulled it into libport, with one tweak to remove the use of
> __attribute__((cleanup)) (it could be replaced by one line of perfectly
> trivial standard C code, saving five or six lines in all).
>
> With libsystemd linkage gone, we need another way for the user to indicate
> that systemd is not in use and the systemd unit files are not wanted: so
> introduce a new --without-systemd (and corresponding --with-systemd=no,
> etc) in configure. (Someone directly running make can just pass
> WITH_SYSTEMD= to make.)
>
> We lose one feature of libsystemd's sd_notify: we no longer remove the
> NOTIFY_SOCKET env var from the environment, so our children could in theory
> pretend to be us and notify systemd on our behalf. Since our only child
> is in a seccomped jail and is part of dtprobed anyway, this loss of
> functionality is purely theoretical.
>
> Bug: https://github.com/oracle/dtrace-utils/issues/92
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
> GNUmakefile | 1 +
> Makeconfig | 1 -
> configure | 8 +++--
> dtprobed/Build | 7 +---
> dtprobed/dtprobed.c | 8 +----
> include/port.h | 1 +
> libport/Build | 4 +--
> libport/systemd_notify.c | 70 ++++++++++++++++++++++++++++++++++++++++
> 8 files changed, 82 insertions(+), 18 deletions(-)
> create mode 100644 libport/systemd_notify.c
>
> diff --git a/GNUmakefile b/GNUmakefile
> index e7657d466df5..cbf27b6be111 100644
> --- a/GNUmakefile
> +++ b/GNUmakefile
> @@ -102,6 +102,7 @@ PKGCONFIGDIR = $(prefix)/share/pkgconfig
> INSTPKGCONFIGDIR = $(DESTDIR)$(PKGCONFIGDIR)
> TESTDIR = $(LIBDIR)/dtrace/testsuite
> INSTTESTDIR = $(DESTDIR)$(TESTDIR)
> +WITH_SYSTEMD = y
> TARGETS =
>
> DTRACE ?= $(objdir)/dtrace
> diff --git a/Makeconfig b/Makeconfig
> index f3d47c086f4a..346078598624 100644
> --- a/Makeconfig
> +++ b/Makeconfig
> @@ -218,7 +218,6 @@ $(eval $(call check-symbol-rule,ELF_GETSHDRSTRNDX,elf_getshdrstrndx,elf))
> $(eval $(call check-symbol-rule,LIBCTF,ctf_open,ctf,t))
> $(eval $(call check-symbol-rule,STRRSTR,strrstr,c))
> $(eval $(call check-symbol-rule,PTHREAD_ATFORK,pthread_atfork,c))
> -$(eval $(call check-symbol-rule,LIBSYSTEMD,sd_notify,systemd,t))
> ifndef WANTS_LIBFUSE2
> $(eval $(call check-symbol-rule,FUSE_LOG,fuse_set_log_func,fuse3))
> $(eval $(call check-symbol-rule,LIBFUSE3,fuse_session_receive_buf,fuse3,t))
> diff --git a/configure b/configure
> index 4026a3eafcd3..72eb734a8380 100755
> --- a/configure
> +++ b/configure
> @@ -93,9 +93,13 @@ built locally, none of these should be needed):
>
> EOF
> make help-overrides-header help-overrides-option
> + cat >&2 <<'EOF'
Leading tab whereas all other lines use spaces.
> +--with-systemd=[yes/no] Install the systemd unit files (default: yes)
> +EOF
> echo >&2
> make help-overrides
> cat >&2 <<'EOF'
> +
> Options controlling the compiler (pass on the command line):
>
> CC C compiler (may be a cross-compiler)
> @@ -153,12 +157,12 @@ for option in "$@"; do
> --kernel-obj-dir=*) write_make_var KERNELOBJDIR "$option";;
> --kernel-src-suffix=*) write_make_var KERNELSRCNAME "$option";;
> --kernel-obj-suffix=*) write_make_var KERNELBLDNAME "$option";;
> + --with-systemd|--with-systemd=y*) write_make_var WITH_SYSTEMD "y";;
> + --with-systemd=n*|--without-systemd) write_make_var WITH_SYSTEMD "";;
> HAVE_ELF_GETSHDRSTRNDX=*) write_config_var ELF_GETSHDRSTRNDX "$option";;
> --with-libctf=*) write_config_var LIBCTF "$option";;
> HAVE_LIBCTF=*) write_config_var LIBCTF "$option";;
> HAVE_STRRSTR=*) write_config_var STRRSTR "$option";;
> - --with-libsystemd=*) write_config_var LIBSYSTEMD "$option";;
> - HAVE_LIBSYSTEMD=*) write_config_var LIBSYSTEMD "$option";;
> HAVE_FUSE_LOG=*) write_config_var FUSE_LOG "$option";;
> --with-libfuse3=*) write_config_var LIBFUSE3 "$option";;
> HAVE_LIBFUSE3=*) write_config_var LIBFUSE3 "$option";;
> diff --git a/dtprobed/Build b/dtprobed/Build
> index 9132c3e31c8a..346cd6aee190 100644
> --- a/dtprobed/Build
> +++ b/dtprobed/Build
> @@ -29,11 +29,6 @@ dtprobed_DEPS := libproc.a libcommon.a libport.a
> dtprobed_SOURCES := dtprobed.c dof_stash.c seccomp-assistance.c
> dtprobed_LIBSOURCES := libproc libcommon
>
> -ifdef HAVE_LIBSYSTEMD
> -dtprobed_CFLAGS += $(shell pkg-config --cflags libsystemd)
> -dtprobed_LIBS += $(shell pkg-config --libs libsystemd)
> -endif
> -
> ifndef HAVE_FUSE_LOG
> dtprobed_SOURCES += rpl_fuse_log.c
> endif
> @@ -43,7 +38,7 @@ seccomp-assistance.c_CFLAGS := -fno-lto
> endif
>
> install-dtprobed-autostart::
> -ifdef HAVE_LIBSYSTEMD
> +ifneq ($(WITH_SYSTEMD),)
> mkdir -p $(INSTSYSTEMDUNITDIR) $(INSTSYSTEMDPRESETDIR)
> $(call describe-install-target,$(INSTSYSTEMDUNITDIR),dtprobed.service)
> sed 's, at SBINDIR@,$(SBINDIR),' < $(dtprobed_DIR)dtprobed.service.in > $(INSTSYSTEMDUNITDIR)/dtprobed.service
> diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
> index fdcdee14f851..c83f0adb56c4 100644
> --- a/dtprobed/dtprobed.c
> +++ b/dtprobed/dtprobed.c
> @@ -61,10 +61,6 @@
>
> #include <dtrace/ioctl.h>
>
> -#ifdef HAVE_LIBSYSTEMD
> -#include <systemd/sd-daemon.h>
> -#endif
> -
> #include <dt_list.h>
> #include "dof_parser.h"
> #include "dof_stash.h"
> @@ -1073,9 +1069,7 @@ main(int argc, char *argv[])
> sync_fd = -1;
> }
>
> -#ifdef HAVE_LIBSYSTEMD
> - sd_notify(1, "READY=1");
> -#endif
> + systemd_notify("READY=1");
>
> ret = loop();
>
> diff --git a/include/port.h b/include/port.h
> index 427129000b4f..6ce8611e9025 100644
> --- a/include/port.h
> +++ b/include/port.h
> @@ -29,6 +29,7 @@ int p_online(int cpun);
> #define MUTEX_HELD(x) ((x)->__data.__count == 0)
>
> int daemonize(int close_fds);
> +int systemd_notify(const char *message);
>
> _dt_noreturn_ void daemon_perr(int fd, const char *err, int err_no);
> _dt_printflike_(2, 3) void daemon_log(int fd, const char *fmt, ...);
> diff --git a/libport/Build b/libport/Build
> index 48ed99e2b017..822049f5e547 100644
> --- a/libport/Build
> +++ b/libport/Build
> @@ -1,5 +1,5 @@
> # Oracle Linux DTrace.
> -# Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
> +# Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.
> # Licensed under the Universal Permissive License v 1.0 as shown at
> # http://oss.oracle.com/licenses/upl.
>
> @@ -8,7 +8,7 @@ LIBS += libport
>
> libport_TARGET = libport
> libport_DIR := $(current-dir)
> -libport_SOURCES = gmatch.c linux_version_code.c strlcat.c strlcpy.c p_online.c time.c daemonize.c
> +libport_SOURCES = gmatch.c linux_version_code.c strlcat.c strlcpy.c p_online.c time.c daemonize.c systemd_notify.c
> ifndef HAVE_CLOSE_RANGE
> libport_SOURCES += close_range.c
> endif
> diff --git a/libport/systemd_notify.c b/libport/systemd_notify.c
> new file mode 100644
> index 000000000000..f78bca6e14c5
> --- /dev/null
> +++ b/libport/systemd_notify.c
> @@ -0,0 +1,70 @@
> +/* SPDX-License-Identifier: MIT-0 */
> +/* Lightly modified from the sd_notify manpage. */
> +
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <stddef.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +int
> +systemd_notify(const char *message)
> +{
> + union sockaddr_union {
> + struct sockaddr sa;
> + struct sockaddr_un sun;
> + } socket_addr = {
> + .sun.sun_family = AF_UNIX,
> + };
> +
> + size_t path_length, message_length;
> + int fd = -1;
> + const char *socket_path;
> +
> + /* Verify the argument first */
> + if (!message)
> + return -EINVAL;
> +
> + message_length = strlen(message);
> + if (message_length == 0)
> + return -EINVAL;
> +
> + /* If the variable is not set, the protocol is a noop */
> + socket_path = getenv("NOTIFY_SOCKET");
> + if (!socket_path)
> + return 0; /* Not set? Nothing to do */
> +
> + /* Only AF_UNIX is supported, with path or abstract sockets */
> + if (socket_path[0] != '/' && socket_path[0] != '@')
> + return -EAFNOSUPPORT;
> +
> + path_length = strlen(socket_path);
> + /* Ensure there is room for NUL byte */
> + if (path_length >= sizeof(socket_addr.sun.sun_path))
> + return -E2BIG;
> +
> + memcpy(socket_addr.sun.sun_path, socket_path, path_length);
> +
> + /* Support for abstract socket */
> + if (socket_addr.sun.sun_path[0] == '@')
> + socket_addr.sun.sun_path[0] = 0;
> +
> + fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0);
> + if (fd < 0)
> + return -errno;
> +
> + if (connect(fd, &socket_addr.sa, offsetof(struct sockaddr_un, sun_path) + path_length) != 0) {
> + close(fd);
> + return -errno;
> + }
> +
> + ssize_t written = write(fd, message, message_length);
> + if (written != (ssize_t) message_length) {
> + close(fd);
> + return written < 0 ? -errno : -EPROTO;
> + }
> +
> + return 1; /* Notified! */
> +}
> --
> 2.46.0.278.g36e3a12567
>
More information about the DTrace-devel
mailing list