Re: [PATCH 1/2] tools/{c,o}xenstored: Don't link against libsystemd

[Jürgen Groß <jgross@xxxxxxxx>]

On 25.04.24 19:32, Andrew Cooper wrote:
> libsystemd is a giant dependency for one single function, but in the wake of
> the xz backdoor, it turns out that even systemd leadership recommend against
> linking against libsystemd for sd_notify().
>
> Since commit 7b61011e1450 ("tools: make xenstore domain easy configurable") in
> Xen 4.8, the launch-xenstore script invokes systemd-notify directly, so its
> not even necessary for the xenstored's to call sd_notify() themselves.
You are aware that in the daemon case the call of systemd-notify does not
signal readyness of xenstored? It is just called with the "--booted" parameter
in order to detect whether systemd is active or not.

So in order to just drop the sd_notify() call from xenstored you need to
modify the launch-xenstore script, too.


Juergen

> Therefore, just drop the calls to sd_notify() and stop linking against
> libsystemd.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> CC: Juergen Gross <jgross@xxxxxxxx>
> CC: Christian Lindig <christian.lindig@xxxxxxxxxx>
> CC: Edwin Török <edwin.torok@xxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> ---
>   tools/ocaml/xenstored/Makefile        | 12 +------
>   tools/ocaml/xenstored/systemd.ml      | 15 ---------
>   tools/ocaml/xenstored/systemd.mli     | 16 ---------
>   tools/ocaml/xenstored/systemd_stubs.c | 47 ---------------------------
>   tools/ocaml/xenstored/xenstored.ml    |  1 -
>   tools/xenstored/Makefile              |  5 ---
>   tools/xenstored/posix.c               |  9 -----
>   7 files changed, 1 insertion(+), 104 deletions(-)
>   delete mode 100644 tools/ocaml/xenstored/systemd.ml
>   delete mode 100644 tools/ocaml/xenstored/systemd.mli
>   delete mode 100644 tools/ocaml/xenstored/systemd_stubs.c
>
> diff --git a/tools/ocaml/xenstored/Makefile b/tools/ocaml/xenstored/Makefile
> index e8aaecf2e630..1e4b51cc5432 100644
> --- a/tools/ocaml/xenstored/Makefile
> +++ b/tools/ocaml/xenstored/Makefile
> @@ -4,8 +4,6 @@ include $(OCAML_TOPLEVEL)/common.make
>   
>   # Include configure output (config.h)
>   CFLAGS += -include $(XEN_ROOT)/tools/config.h
> -CFLAGS-$(CONFIG_SYSTEMD)  += $(SYSTEMD_CFLAGS)
> -LDFLAGS-$(CONFIG_SYSTEMD) += $(SYSTEMD_LIBS)
>   
>   CFLAGS  += $(CFLAGS-y)
>   CFLAGS  += $(APPEND_CFLAGS)
> @@ -25,13 +23,6 @@ poll_OBJS = poll
>   poll_C_OBJS = select_stubs
>   OCAML_LIBRARY = syslog poll
>   
> -LIBS += systemd.cma systemd.cmxa
> -systemd_OBJS = systemd
> -systemd_C_OBJS = systemd_stubs
> -OCAML_LIBRARY += systemd
> -
> -LIBS_systemd += $(LDFLAGS-y)
> -
>   OBJS = paths \
>         define \
>         stdext \
> @@ -56,12 +47,11 @@ OBJS = paths \
>         process \
>         xenstored
>   
> -INTF = symbol.cmi trie.cmi syslog.cmi systemd.cmi poll.cmi
> +INTF = symbol.cmi trie.cmi syslog.cmi poll.cmi
>   
>   XENSTOREDLIBS = \
>         unix.cmxa \
>         -ccopt -L -ccopt . syslog.cmxa \
> -       -ccopt -L -ccopt . systemd.cmxa \
>         -ccopt -L -ccopt . poll.cmxa \
>         -ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/mmap 
> $(OCAML_TOPLEVEL)/libs/mmap/xenmmap.cmxa \
>         -ccopt -L -ccopt $(OCAML_TOPLEVEL)/libs/eventchn 
> $(OCAML_TOPLEVEL)/libs/eventchn/xeneventchn.cmxa \
> diff --git a/tools/ocaml/xenstored/systemd.ml b/tools/ocaml/xenstored/systemd.ml
> deleted file mode 100644
> index 39127f712d72..000000000000
> --- a/tools/ocaml/xenstored/systemd.ml
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -(*
> - * Copyright (C) 2014 Luis R. Rodriguez <mcgrof@xxxxxxxx>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU Lesser General Public License as published
> - * by the Free Software Foundation; version 2.1 only. with the special
> - * exception on linking described in file LICENSE.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU Lesser General Public License for more details.
> - *)
> -
> -external sd_notify_ready: unit -> unit = "ocaml_sd_notify_ready"
> diff --git a/tools/ocaml/xenstored/systemd.mli 
> b/tools/ocaml/xenstored/systemd.mli
> deleted file mode 100644
> index 18b9331031f9..000000000000
> --- a/tools/ocaml/xenstored/systemd.mli
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -(*
> - * Copyright (C) 2014 Luis R. Rodriguez <mcgrof@xxxxxxxx>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU Lesser General Public License as published
> - * by the Free Software Foundation; version 2.1 only. with the special
> - * exception on linking described in file LICENSE.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU Lesser General Public License for more details.
> - *)
> -
> -(** Tells systemd we're ready *)
> -external sd_notify_ready: unit -> unit = "ocaml_sd_notify_ready"
> diff --git a/tools/ocaml/xenstored/systemd_stubs.c 
> b/tools/ocaml/xenstored/systemd_stubs.c
> deleted file mode 100644
> index f4c875075abe..000000000000
> --- a/tools/ocaml/xenstored/systemd_stubs.c
> +++ /dev/null
> @@ -1,47 +0,0 @@
> -/*
> - * Copyright (C) 2014 Luis R. Rodriguez <mcgrof@xxxxxxxx>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU Lesser General Public License as published
> - * by the Free Software Foundation; version 2.1 only. with the special
> - * exception on linking described in file LICENSE.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU Lesser General Public License for more details.
> - */
> -
> -#include <string.h>
> -#include <stdio.h>
> -#include <stdbool.h>
> -#include <errno.h>
> -#include <caml/mlvalues.h>
> -#include <caml/memory.h>
> -#include <caml/alloc.h>
> -#include <caml/custom.h>
> -#include <caml/signals.h>
> -#include <caml/fail.h>
> -
> -#if defined(HAVE_SYSTEMD)
> -
> -#include <systemd/sd-daemon.h>
> -
> -CAMLprim value ocaml_sd_notify_ready(value ignore)
> -{
> -       CAMLparam1(ignore);
> -
> -       sd_notify(1, "READY=1");
> -
> -       CAMLreturn(Val_unit);
> -}
> -
> -#else
> -
> -CAMLprim value ocaml_sd_notify_ready(value ignore)
> -{
> -       CAMLparam1(ignore);
> -
> -       CAMLreturn(Val_unit);
> -}
> -#endif
> diff --git a/tools/ocaml/xenstored/xenstored.ml 
> b/tools/ocaml/xenstored/xenstored.ml
> index 1aaa3e995e1f..6bcd6d11da55 100644
> --- a/tools/ocaml/xenstored/xenstored.ml
> +++ b/tools/ocaml/xenstored/xenstored.ml
> @@ -621,7 +621,6 @@ let () =
>       process_domains store cons domains
>     in
>   
> -  Systemd.sd_notify_ready ();
>     let live_update = ref false in
>     while not (!quit && Connections.prevents_quit cons = [])
>     do
> diff --git a/tools/xenstored/Makefile b/tools/xenstored/Makefile
> index e0897ed1ba30..09adfe1d5064 100644
> --- a/tools/xenstored/Makefile
> +++ b/tools/xenstored/Makefile
> @@ -9,11 +9,6 @@ xenstored: LDLIBS += $(LDLIBS_libxenctrl)
>   xenstored: LDLIBS += -lrt
>   xenstored: LDLIBS += $(SOCKET_LIBS)
>   
> -ifeq ($(CONFIG_SYSTEMD),y)
> -$(XENSTORED_OBJS-y): CFLAGS += $(SYSTEMD_CFLAGS)
> -xenstored: LDLIBS += $(SYSTEMD_LIBS)
> -endif
> -
>   TARGETS := xenstored
>   
>   .PHONY: all
> diff --git a/tools/xenstored/posix.c b/tools/xenstored/posix.c
> index d88c82d972d7..dc4ed8db591f 100644
> --- a/tools/xenstored/posix.c
> +++ b/tools/xenstored/posix.c
> @@ -26,9 +26,6 @@
>   #include <sys/mman.h>
>   #include <sys/socket.h>
>   #include <sys/un.h>
> -#if defined(HAVE_SYSTEMD)
> -#include <systemd/sd-daemon.h>
> -#endif
>   #include <xen-tools/xenstore-common.h>
>   
>   #include "utils.h"
> @@ -390,12 +387,6 @@ void handle_special_fds(void)
>   
>   void late_init(bool live_update)
>   {
> -#if defined(HAVE_SYSTEMD)
> -       if (!live_update) {
> -               sd_notify(1, "READY=1");
> -               fprintf(stderr, SD_NOTICE "xenstored is ready\n");
> -       }
> -#endif
>   }
>   
>   int get_socket_fd(void)