From 997d61802f84c1f8793d32e463a290a7c5b35ff3 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Mon, 14 Nov 2016 15:59:56 -0500 Subject: [PATCH] Fix libvirtd endless loop when starting network with multiple IPs (bz #1393975) --- ...ess-loop-when-starting-network-with-.patch | 67 +++++ ...-notify-systemd-instead-of-using-sd_.patch | 242 ------------------ libvirt.spec | 10 +- 3 files changed, 76 insertions(+), 243 deletions(-) create mode 100644 0001-network-fix-endless-loop-when-starting-network-with-.patch delete mode 100644 0001-systemd-directly-notify-systemd-instead-of-using-sd_.patch diff --git a/0001-network-fix-endless-loop-when-starting-network-with-.patch b/0001-network-fix-endless-loop-when-starting-network-with-.patch new file mode 100644 index 0000000..a10440b --- /dev/null +++ b/0001-network-fix-endless-loop-when-starting-network-with-.patch @@ -0,0 +1,67 @@ +From: Laine Stump +Date: Fri, 28 Oct 2016 11:43:56 -0400 +Subject: [PATCH] network: fix endless loop when starting network with multiple + IPs and no dhcp + +commit 9065cfaa added the ability to disable DNS services for a +libvirt virtual network. If neither DNS nor DHCP is needed for a +network, then we don't need to start dnsmasq, so code was added to +check for this. + +Unfortunately, it was written with a great lack of attention to detail +(I can say that, because I was the author), and the loop that checked +if DHCP is needed for the network would never end if the network had +multiple IP addresses and the first had no subelement +(which would have contained a or subelement, thus +requiring DHCP services). + +This patch rewrites the check to be more compact and (more +importantly) finite. + +This bug was present in release 2.2.0 and 2.3.0, so will need to be +backported to any relevant maintainence branches. + +Reported here: + https://www.redhat.com/archives/libvirt-users/2016-October/msg00032.html + https://www.redhat.com/archives/libvirt-users/2016-October/msg00045.html + +(cherry picked from commit bbb333e4813ebe74580e75b0e8c2eb325e3d11ca) +--- + src/network/bridge_driver.c | 18 ++++++++++-------- + 1 file changed, 10 insertions(+), 8 deletions(-) + +diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c +index 7b99aca..3cf349e 100644 +--- a/src/network/bridge_driver.c ++++ b/src/network/bridge_driver.c +@@ -1408,20 +1408,22 @@ networkStartDhcpDaemon(virNetworkDriverStatePtr driver, + int ret = -1; + dnsmasqContext *dctx = NULL; + +- if (!(ipdef = virNetworkDefGetIPByIndex(network->def, AF_UNSPEC, 0))) { +- /* no IP addresses, so we don't need to run */ +- ret = 0; +- goto cleanup; +- } +- + /* see if there are any IP addresses that need a dhcp server */ +- for (i = 0; ipdef && !needDnsmasq; +- ipdef = virNetworkDefGetIPByIndex(network->def, AF_UNSPEC, i + 1)) { ++ i = 0; ++ while ((ipdef = virNetworkDefGetIPByIndex(network->def, AF_UNSPEC, i))) { ++ i++; + if (ipdef->nranges || ipdef->nhosts) + needDnsmasq = true; + } + ++ if (i == 0) { ++ /* no IP addresses at all, so we don't need to run */ ++ ret = 0; ++ goto cleanup; ++ } ++ + if (!needDnsmasq && network->def->dns.enable == VIR_TRISTATE_BOOL_NO) { ++ /* no DHCP services needed, and user disabled DNS service */ + ret = 0; + goto cleanup; + } diff --git a/0001-systemd-directly-notify-systemd-instead-of-using-sd_.patch b/0001-systemd-directly-notify-systemd-instead-of-using-sd_.patch deleted file mode 100644 index 2823f18..0000000 --- a/0001-systemd-directly-notify-systemd-instead-of-using-sd_.patch +++ /dev/null @@ -1,242 +0,0 @@ -From c0bc172383c2c955394589e5808457935ae06f1d Mon Sep 17 00:00:00 2001 -From: "Daniel P. Berrange" -Date: Mon, 6 Jun 2016 15:03:27 +0100 -Subject: [PATCH] systemd: directly notify systemd instead of using sd_notify - -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/ - -Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1314881 - -Signed-off-by: Daniel P. Berrange ---- - configure.ac | 2 -- - libvirt.spec.in | 12 ----------- - m4/virt-systemd-daemon.m4 | 34 ------------------------------ - src/Makefile.am | 4 ++-- - src/util/virsystemd.c | 53 ++++++++++++++++++++++++++++++++++++++++++----- - 5 files changed, 50 insertions(+), 55 deletions(-) - delete mode 100644 m4/virt-systemd-daemon.m4 - -diff --git a/configure.ac b/configure.ac -index f2554a4..12eb3b3 100644 ---- a/configure.ac -+++ b/configure.ac -@@ -256,7 +256,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 -@@ -2787,7 +2786,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 8b88eef..b93a53c 100644 ---- a/libvirt.spec.in -+++ b/libvirt.spec.in -@@ -79,7 +79,6 @@ - %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_pm_utils 1 - - # Finally set the OS / architecture specific special cases -@@ -133,7 +132,6 @@ - # 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_pm_utils 0 - %endif - -@@ -268,9 +266,6 @@ BuildRequires: python - %if %{with_systemd} - BuildRequires: systemd-units - %endif --%if %{with_systemd_daemon} --BuildRequires: systemd-devel --%endif - %if %{with_xen} || %{with_libxl} - BuildRequires: xen-devel - %endif -@@ -1061,12 +1056,6 @@ rm -rf .git - %define arg_wireshark --without-wireshark-dissector - %endif - --%if %{with_systemd_daemon} -- %define arg_systemd_daemon --with-systemd-daemon --%else -- %define arg_systemd_daemon --without-systemd-daemon --%endif -- - %if %{with_pm_utils} - %define arg_pm_utils --with-pm-utils - %else -@@ -1157,7 +1146,6 @@ rm -f po/stamp-po - --with-driver-modules \ - %{?arg_firewalld} \ - %{?arg_wireshark} \ -- %{?arg_systemd_daemon} \ - %{?arg_pm_utils} \ - --with-nss-plugin \ - %{arg_packager} \ -diff --git a/m4/virt-systemd-daemon.m4 b/m4/virt-systemd-daemon.m4 -deleted file mode 100644 -index 8516e41..0000000 ---- a/m4/virt-systemd-daemon.m4 -+++ /dev/null -@@ -1,34 +0,0 @@ --dnl The libsystemd-daemon.so library --dnl --dnl Copyright (C) 2012-2013 Red Hat, Inc. --dnl --dnl This library is free software; you can redistribute it and/or --dnl modify it under the terms of the GNU Lesser General Public --dnl License as published by the Free Software Foundation; either --dnl version 2.1 of the License, or (at your option) any later version. --dnl --dnl This library is distributed in the hope that it will be useful, --dnl but WITHOUT ANY WARRANTY; without even the implied warranty of --dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU --dnl Lesser General Public License for more details. --dnl --dnl You should have received a copy of the GNU Lesser General Public --dnl License along with this library. If not, see --dnl . --dnl -- --AC_DEFUN([LIBVIRT_CHECK_SYSTEMD_DAEMON],[ -- LIBVIRT_CHECK_PKG([SYSTEMD_DAEMON], [libsystemd-daemon], [0.27.1]) -- -- old_CFLAGS="$CFLAGS" -- old_LIBS="$LIBS" -- CFLAGS="$CFLAGS $SYSTEMD_DAEMON_CFLAGS" -- LIBS="$LIBS $SYSTEMD_DAEMON_LIBS" -- AC_CHECK_FUNCS([sd_notify]) -- CFLAGS="$old_CFLAGS" -- LIBS="$old_LIBS" --]) -- --AC_DEFUN([LIBVIRT_RESULT_SYSTEMD_DAEMON],[ -- LIBVIRT_RESULT_LIB([SYSTEMD_DAEMON]) --]) -diff --git a/src/Makefile.am b/src/Makefile.am -index f3c9a14..f020b92 100644 ---- a/src/Makefile.am -+++ b/src/Makefile.am -@@ -1107,12 +1107,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) \ -+ $(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) \ - $(POLKIT_LIBS) - - -diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c -index 4883f94..871db7e 100644 ---- a/src/util/virsystemd.c -+++ b/src/util/virsystemd.c -@@ -21,8 +21,9 @@ - - #include - --#ifdef WITH_SYSTEMD_DAEMON --# include -+#include -+#ifdef HAVE_SYS_UN_H -+# include - #endif - - #include "virsystemd.h" -@@ -34,6 +35,7 @@ - #include "virutil.h" - #include "virlog.h" - #include "virerror.h" -+#include "virfile.h" - - #define VIR_FROM_THIS VIR_FROM_SYSTEMD - -@@ -480,9 +482,50 @@ 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; -+ } -+ -+ /* NB sun_path field is *not* NUL-terminated, hence >, not >= */ -+ if (strlen(path) > sizeof(un.sun_path)) { -+ VIR_WARN("Systemd notify socket path '%s' too long", path); -+ return; -+ } -+ -+ memcpy(un.sun_path, path, strlen(path)); -+ if (un.sun_path[0] == '@') -+ un.sun_path[0] = '\0'; -+ -+ fd = socket(AF_UNIX, SOCK_DGRAM, 0); -+ if (fd < 0) { -+ VIR_WARN("Unable to create socket FD"); -+ return; -+ } -+ -+ if (sendmsg(fd, &mh, MSG_NOSIGNAL) < 0) -+ VIR_WARN("Failed to notify systemd"); -+ -+ VIR_FORCE_CLOSE(fd); -+#endif /* HAVE_SYS_UN_H */ - } - - static int --- -2.5.5 - diff --git a/libvirt.spec b/libvirt.spec index c81d1b7..4440f0e 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -220,7 +220,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 2.2.0 -Release: 1%{?dist}%{?extra_release} +Release: 2%{?dist}%{?extra_release} License: LGPLv2+ Group: Development/Libraries BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root @@ -231,6 +231,10 @@ URL: http://libvirt.org/ %endif Source: http://libvirt.org/sources/%{?mainturl}libvirt-%{version}.tar.xz +# Fix libvirtd endless loop when starting network with multiple IPs (bz +# #1393975) +Patch0001: 0001-network-fix-endless-loop-when-starting-network-with-.patch + Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-daemon-config-network = %{version}-%{release} Requires: libvirt-daemon-config-nwfilter = %{version}-%{release} @@ -1891,6 +1895,10 @@ exit 0 %changelog +* Mon Nov 14 2016 Cole Robinson - 2.2.0-2 +- Fix libvirtd endless loop when starting network with multiple IPs (bz + #1393975) + * Mon Sep 5 2016 Daniel P. Berrange - 2.2.0-1 - Rebase to version 2.2.0