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.
Therefore, just drop the calls to sd_notify() and stop linking against
libsystemd.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@citrix.com>
CC: Juergen Gross <jgross@suse.com>
CC: Christian Lindig <christian.lindig@citrix.com>
CC: Edwin Török <edwin.torok@cloud.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
---
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@suse.com>
- *
- * 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@suse.com>
- *
- * 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@suse.com>
- *
- * 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)
--
2.30.2
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@citrix.com>
> ---
> CC: Anthony PERARD <anthony.perard@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Christian Lindig <christian.lindig@citrix.com>
> CC: Edwin Török <edwin.torok@cloud.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> ---
> 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@suse.com>
> - *
> - * 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@suse.com>
> - *
> - * 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@suse.com>
> - *
> - * 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)
On Thu, Apr 25, 2024 at 06:32:15PM +0100, 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
That's not enough, it's needs to be `systemd-notify --ready` to be a
replacement for sd_notify(READY=1).
> not even necessary for the xenstored's to call sd_notify() themselves.
So, sd_notify() or equivalent is still necessary.
> Therefore, just drop the calls to sd_notify() and stop linking against
> libsystemd.
Sounds good, be we need to replace the call by something like:
echo READY=1 > $NOTIFY_SOCKET
implemented in C and ocaml. Detail to be checked.
Otherwise, things won't work.
Thanks,
--
Anthony PERARD
On 25/04/2024 7:06 pm, Anthony PERARD wrote:
> On Thu, Apr 25, 2024 at 06:32:15PM +0100, 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
> That's not enough, it's needs to be `systemd-notify --ready` to be a
> replacement for sd_notify(READY=1).
>
>> not even necessary for the xenstored's to call sd_notify() themselves.
> So, sd_notify() or equivalent is still necessary.
>
>> Therefore, just drop the calls to sd_notify() and stop linking against
>> libsystemd.
> Sounds good, be we need to replace the call by something like:
> echo READY=1 > $NOTIFY_SOCKET
> implemented in C and ocaml. Detail to be checked.
>
> Otherwise, things won't work.
Hmm. It worked in XenRT when stripping this all out, but that is
extremely unintuitive behaviour for `systemd-notify --booted`, seeing as
it's entirely different to --ready.
I've got no interest in keeping the C around, but if:
[ -n "$NOTIFY_SOCKET" ] && echo READY=1 > $NOTIFY_SOCKET
works, then can't we just use that after waiting for the the pidfile ?
~Andrew
On Thu, Apr 25, 2024 at 07:16:23PM +0100, Andrew Cooper wrote:
> On 25/04/2024 7:06 pm, Anthony PERARD wrote:
> > On Thu, Apr 25, 2024 at 06:32:15PM +0100, 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
> > That's not enough, it's needs to be `systemd-notify --ready` to be a
> > replacement for sd_notify(READY=1).
> >
> >> not even necessary for the xenstored's to call sd_notify() themselves.
> > So, sd_notify() or equivalent is still necessary.
> >
> >> Therefore, just drop the calls to sd_notify() and stop linking against
> >> libsystemd.
> > Sounds good, be we need to replace the call by something like:
> > echo READY=1 > $NOTIFY_SOCKET
> > implemented in C and ocaml. Detail to be checked.
> >
> > Otherwise, things won't work.
>
> Hmm. It worked in XenRT when stripping this all out, but that is
I don't know how XenServer is setup, maybe it doesn't matter? Anyway...
> extremely unintuitive behaviour for `systemd-notify --booted`, seeing as
> it's entirely different to --ready.
Yes, this --booted option should probably not exist, and there's
`systemctl is-system-running` that does something similar.
>
> I've got no interest in keeping the C around, but if:
>
> [ -n "$NOTIFY_SOCKET" ] && echo READY=1 > $NOTIFY_SOCKET
>
> works, then can't we just use that after waiting for the the pidfile ?
Run `systemd-notify --ready` instead. Hopefully, that will be enough.
($NOTIFY_SOCKET is a socket, and a bit more complicated that I though,
it can start with "@" for example)
Cheers,
--
Anthony PERARD
On Fri, Apr 26, 2024 at 09:51:47AM +0100, Anthony PERARD wrote: > Run `systemd-notify --ready` instead. Hopefully, that will be enough. > ($NOTIFY_SOCKET is a socket, and a bit more complicated that I though, > it can start with "@" for example) FTR: If it turns out that calling systemd-notify binary isn't working well enough, we could have an implementation of sd_notify() in our tree, openssh are doing there own here: https://bugzilla.mindrot.org/show_bug.cgi?id=2641 and there's an example implementation on systemd's documentation: https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes (Nothing for ocaml) But let's go with `systemd-notify --ready` as it is just easier to write a bit of shell script. Cheers, -- Anthony PERARD
On 01/05/2024 2:29 pm, Anthony PERARD wrote: > On Fri, Apr 26, 2024 at 09:51:47AM +0100, Anthony PERARD wrote: >> Run `systemd-notify --ready` instead. Hopefully, that will be enough. >> ($NOTIFY_SOCKET is a socket, and a bit more complicated that I though, >> it can start with "@" for example) > FTR: If it turns out that calling systemd-notify binary isn't working > well enough, we could have an implementation of sd_notify() in our tree, > openssh are doing there own here: > https://bugzilla.mindrot.org/show_bug.cgi?id=2641 > and there's an example implementation on systemd's documentation: > https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes > (Nothing for ocaml) > > But let's go with `systemd-notify --ready` as it is just easier to > write a bit of shell script. I was already thinking of going down the small-library-function route. Given that I miss-analysed the launch-xenstore, script, I'm not overly enthused with just falling back to waiting on the pidfile, because that's adding technical debt rather than removing it. ~Andrew
On 26/04/2024 9:51 am, Anthony PERARD wrote:
> On Thu, Apr 25, 2024 at 07:16:23PM +0100, Andrew Cooper wrote:
>> On 25/04/2024 7:06 pm, Anthony PERARD wrote:
>>> On Thu, Apr 25, 2024 at 06:32:15PM +0100, 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
>>> That's not enough, it's needs to be `systemd-notify --ready` to be a
>>> replacement for sd_notify(READY=1).
>>>
>>>> not even necessary for the xenstored's to call sd_notify() themselves.
>>> So, sd_notify() or equivalent is still necessary.
>>>
>>>> Therefore, just drop the calls to sd_notify() and stop linking against
>>>> libsystemd.
>>> Sounds good, be we need to replace the call by something like:
>>> echo READY=1 > $NOTIFY_SOCKET
>>> implemented in C and ocaml. Detail to be checked.
>>>
>>> Otherwise, things won't work.
>> Hmm. It worked in XenRT when stripping this all out, but that is
> I don't know how XenServer is setup, maybe it doesn't matter?
In theory it's straight systemd, but I could also believe that Xapi
checks for the pidfile too.
> Anyway...
>
>> extremely unintuitive behaviour for `systemd-notify --booted`, seeing as
>> it's entirely different to --ready.
> Yes, this --booted option should probably not exist, and there's
> `systemctl is-system-running` that does something similar.
>
>> I've got no interest in keeping the C around, but if:
>>
>> [ -n "$NOTIFY_SOCKET" ] && echo READY=1 > $NOTIFY_SOCKET
>>
>> works, then can't we just use that after waiting for the the pidfile ?
> Run `systemd-notify --ready` instead. Hopefully, that will be enough.
> ($NOTIFY_SOCKET is a socket, and a bit more complicated that I though,
> it can start with "@" for example)
I'll do a prep patch to adjust launch-xenstore after which this patch
should be fine in this form (modulo a tweak in the commit message).
~Andrew
© 2016 - 2026 Red Hat, Inc.