[libvirt] [PATCH 0/3] Fix 'make rpm' failure with systemd 230
Turns out the libsystemd-daemon library, which we've been using for the sd_notify() feature, has been deprecated for a long time and has finally been removed as of systemd 230. This means 'make rpm' no longer works on Fedora rawhide, and that other distributions such as Debian sid are silently disabling the sd_notify() feature. Switch from libsystemd-daemon to libsystemd. Andrea Bolognani (3): maint: Use libsystemd instead of libsystemd-daemon spec: Enable libsystemd on Fedora >= 21, RHEL >= 7 systemd: Guard the call to sd_notify() properly configure.ac | 4 ++-- libvirt.spec.in | 18 +++++++++++------- m4/{virt-systemd-daemon.m4 => virt-libsystemd.m4} | 14 +++++++------- src/Makefile.am | 4 ++-- src/util/virsystemd.c | 4 ++-- 5 files changed, 24 insertions(+), 20 deletions(-) rename m4/{virt-systemd-daemon.m4 => virt-libsystemd.m4} (73%) -- 2.5.5
The libsystemd-daemon library had been deprecated upstream just a few days before we started using it. Talk about bad timing :) With systemd 230, now in Debian sid and Fedora rawhide, it has finally been dropped. We should use libsystemd, its replacement, instead. --- configure.ac | 4 ++-- libvirt.spec.in | 14 +++++++------- m4/{virt-systemd-daemon.m4 => virt-libsystemd.m4} | 14 +++++++------- src/Makefile.am | 4 ++-- src/util/virsystemd.c | 4 ++-- 5 files changed, 20 insertions(+), 20 deletions(-) rename m4/{virt-systemd-daemon.m4 => virt-libsystemd.m4} (73%) diff --git a/configure.ac b/configure.ac index 74c33b3..e0e155b 100644 --- a/configure.ac +++ b/configure.ac @@ -247,6 +247,7 @@ LIBVIRT_CHECK_DBUS LIBVIRT_CHECK_FUSE LIBVIRT_CHECK_GLUSTER LIBVIRT_CHECK_HAL +LIBVIRT_CHECK_LIBSYSTEMD LIBVIRT_CHECK_NETCF LIBVIRT_CHECK_NUMACTL LIBVIRT_CHECK_OPENWSMAN @@ -256,7 +257,6 @@ LIBVIRT_CHECK_SANLOCK LIBVIRT_CHECK_SASL LIBVIRT_CHECK_SELINUX LIBVIRT_CHECK_SSH2 -LIBVIRT_CHECK_SYSTEMD_DAEMON LIBVIRT_CHECK_UDEV LIBVIRT_CHECK_WIRESHARK LIBVIRT_CHECK_NSS @@ -2778,6 +2778,7 @@ LIBVIRT_RESULT_DBUS LIBVIRT_RESULT_FUSE LIBVIRT_RESULT_GLUSTER LIBVIRT_RESULT_HAL +LIBVIRT_RESULT_LIBSYSTEMD LIBVIRT_RESULT_NETCF LIBVIRT_RESULT_NUMACTL LIBVIRT_RESULT_OPENWSMAN @@ -2787,7 +2788,6 @@ LIBVIRT_RESULT_SANLOCK LIBVIRT_RESULT_SASL LIBVIRT_RESULT_SELINUX LIBVIRT_RESULT_SSH2 -LIBVIRT_RESULT_SYSTEMD_DAEMON LIBVIRT_RESULT_UDEV LIBVIRT_RESULT_WIRESHARK LIBVIRT_RESULT_NSS diff --git a/libvirt.spec.in b/libvirt.spec.in index c7fcf85..9cae0b2 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -79,7 +79,7 @@ %define with_firewalld 0%{!?_without_firewalld:0} %define with_libssh2 0%{!?_without_libssh2:0} %define with_wireshark 0%{!?_without_wireshark:0} -%define with_systemd_daemon 0%{!?_without_systemd_daemon:0} +%define with_libsystemd 0%{!?_without_libsystemd:0} %define with_pm_utils 1 # Finally set the OS / architecture specific special cases @@ -133,7 +133,7 @@ # Fedora has systemd, libvirt still used sysvinit there. %if 0%{?fedora} || 0%{?rhel} >= 7 %define with_systemd 1 - %define with_systemd_daemon 1 + %define with_libsystemd 1 %define with_pm_utils 0 %endif @@ -274,7 +274,7 @@ BuildRequires: python %if %{with_systemd} BuildRequires: systemd-units %endif -%if %{with_systemd_daemon} +%if %{with_libsystemd} BuildRequires: systemd-devel %endif %if %{with_xen} || %{with_libxl} @@ -1067,10 +1067,10 @@ rm -rf .git %define arg_wireshark --without-wireshark-dissector %endif -%if %{with_systemd_daemon} - %define arg_systemd_daemon --with-systemd-daemon +%if %{with_libsystemd} + %define arg_libsystemd --with-libsystemd %else - %define arg_systemd_daemon --without-systemd-daemon + %define arg_libsystemd --without-libsystemd %endif %if %{with_pm_utils} @@ -1151,7 +1151,7 @@ rm -f po/stamp-po --with-driver-modules \ %{?arg_firewalld} \ %{?arg_wireshark} \ - %{?arg_systemd_daemon} \ + %{?arg_libsystemd} \ %{?arg_pm_utils} \ --with-nss-plugin \ %{arg_packager} \ diff --git a/m4/virt-systemd-daemon.m4 b/m4/virt-libsystemd.m4 similarity index 73% rename from m4/virt-systemd-daemon.m4 rename to m4/virt-libsystemd.m4 index 8516e41..5773af3 100644 --- a/m4/virt-systemd-daemon.m4 +++ b/m4/virt-libsystemd.m4 @@ -1,4 +1,4 @@ -dnl The libsystemd-daemon.so library +dnl The libsystemd.so library dnl dnl Copyright (C) 2012-2013 Red Hat, Inc. dnl @@ -17,18 +17,18 @@ dnl License along with this library. If not, see dnl <http://www.gnu.org/licenses/>. dnl -AC_DEFUN([LIBVIRT_CHECK_SYSTEMD_DAEMON],[ - LIBVIRT_CHECK_PKG([SYSTEMD_DAEMON], [libsystemd-daemon], [0.27.1]) +AC_DEFUN([LIBVIRT_CHECK_LIBSYSTEMD],[ + LIBVIRT_CHECK_PKG([LIBSYSTEMD], [libsystemd], [0.27.1]) old_CFLAGS="$CFLAGS" old_LIBS="$LIBS" - CFLAGS="$CFLAGS $SYSTEMD_DAEMON_CFLAGS" - LIBS="$LIBS $SYSTEMD_DAEMON_LIBS" + CFLAGS="$CFLAGS $LIBSYSTEMD_CFLAGS" + LIBS="$LIBS $LIBSYSTEMD_LIBS" AC_CHECK_FUNCS([sd_notify]) CFLAGS="$old_CFLAGS" LIBS="$old_LIBS" ]) -AC_DEFUN([LIBVIRT_RESULT_SYSTEMD_DAEMON],[ - LIBVIRT_RESULT_LIB([SYSTEMD_DAEMON]) +AC_DEFUN([LIBVIRT_RESULT_LIBSYSTEMD],[ + LIBVIRT_RESULT_LIB([LIBSYSTEMD]) ]) diff --git a/src/Makefile.am b/src/Makefile.am index 12b66c2..f401ed6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1106,12 +1106,12 @@ libvirt_util_la_SOURCES = \ libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ $(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \ $(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS) \ - $(SYSTEMD_DAEMON_CFLAGS) $(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) \ + $(LIBSYSTEMD_CFLAGS) $(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) \ -I$(srcdir)/conf libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ $(LIB_CLOCK_GETTIME) $(DBUS_LIBS) $(MSCOM_LIBS) $(LIBXML_LIBS) \ - $(SECDRIVER_LIBS) $(NUMACTL_LIBS) $(SYSTEMD_DAEMON_LIBS) \ + $(SECDRIVER_LIBS) $(NUMACTL_LIBS) $(LIBSYSTEMD_LIBS) \ $(POLKIT_LIBS) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 4883f94..b29418f 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -21,7 +21,7 @@ #include <config.h> -#ifdef WITH_SYSTEMD_DAEMON +#ifdef WITH_LIBSYSTEMD # include <systemd/sd-daemon.h> #endif @@ -480,7 +480,7 @@ int virSystemdTerminateMachine(const char *name) void virSystemdNotifyStartup(void) { -#ifdef WITH_SYSTEMD_DAEMON +#ifdef WITH_LIBSYSTEMD sd_notify(0, "READY=1"); #endif } -- 2.5.5
On Fri, May 27, 2016 at 02:55:06PM +0200, Andrea Bolognani wrote:
The libsystemd-daemon library had been deprecated upstream just a few days before we started using it. Talk about bad timing :)
With systemd 230, now in Debian sid and Fedora rawhide, it has finally been dropped. We should use libsystemd, its replacement, instead. --- configure.ac | 4 ++-- libvirt.spec.in | 14 +++++++------- m4/{virt-systemd-daemon.m4 => virt-libsystemd.m4} | 14 +++++++------- src/Makefile.am | 4 ++-- src/util/virsystemd.c | 4 ++-- 5 files changed, 20 insertions(+), 20 deletions(-) rename m4/{virt-systemd-daemon.m4 => virt-libsystemd.m4} (73%)
Just unconditionally dropping support for libsystemd-daemon.so means you require systemd >= 209 for libvirt to build. IMHO this is too new. The only thing we use this for is sd_notify() which is a really trivial method which gets a NUIX socket from NOTIFY_SOCKET env variable, opens it and just sends across the string. I think we could just implement this logic trivially ourselvs, and not need to use any library, thus avoiding the problem with it being moved Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Fedora 20 didn't have libsystemd, so we should not require it. --- libvirt.spec.in | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 9cae0b2..6117acc 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -133,10 +133,14 @@ # Fedora has systemd, libvirt still used sysvinit there. %if 0%{?fedora} || 0%{?rhel} >= 7 %define with_systemd 1 - %define with_libsystemd 1 %define with_pm_utils 0 %endif +# Fedora 21 / RHEL-7 are first where libsystemd is enabled +%if 0%{?fedora} >= 21 || 0%{?rhel} >= 7 + %define with_libsystemd 1 +%endif + # Fedora 18 / RHEL-7 are first where firewalld support is enabled %if 0%{?fedora} || 0%{?rhel} >= 7 %define with_firewalld 1 -- 2.5.5
Whether or not we call sd_notify() should depend on the availability of the function itself, not of the library that contains it. --- src/util/virsystemd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index b29418f..44409ca 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -480,7 +480,7 @@ int virSystemdTerminateMachine(const char *name) void virSystemdNotifyStartup(void) { -#ifdef WITH_LIBSYSTEMD +#ifdef HAVE_SD_NOTIFY sd_notify(0, "READY=1"); #endif } -- 2.5.5
On 05/27/2016 08:55 AM, Andrea Bolognani wrote:
Turns out the libsystemd-daemon library, which we've been using for the sd_notify() feature, has been deprecated for a long time and has finally been removed as of systemd 230.
This means 'make rpm' no longer works on Fedora rawhide, and that other distributions such as Debian sid are silently disabling the sd_notify() feature.
Switch from libsystemd-daemon to libsystemd.
ACK series, and I verified things continue to work fine on f24, but rawhide repos seem to be a bit broken at the moment so I couldn't verify there, but it looks good to me. Also, when pushing please close this: https://bugzilla.redhat.com/show_bug.cgi?id=1314881 Thanks, Cole
participants (3)
-
Andrea Bolognani -
Cole Robinson -
Daniel P. Berrange