Fix libvirtd endless loop when starting network with multiple IPs (bz #1393975)

This commit is contained in:
Cole Robinson 2016-11-14 15:59:56 -05:00
parent fc2ebb7646
commit 997d61802f
3 changed files with 76 additions and 243 deletions

View File

@ -0,0 +1,67 @@
From: Laine Stump <laine@laine.org>
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 <ip> had no <dhcp> subelement
(which would have contained a <range> or <host> 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;
}

View File

@ -1,242 +0,0 @@
From c0bc172383c2c955394589e5808457935ae06f1d Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
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 <berrange@redhat.com>
---
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 <http://www.gnu.org/licenses/>.
-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 <config.h>
-#ifdef WITH_SYSTEMD_DAEMON
-# include <systemd/sd-daemon.h>
+#include <sys/socket.h>
+#ifdef HAVE_SYS_UN_H
+# include <sys/un.h>
#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

View File

@ -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 <crobinso@redhat.com> - 2.2.0-2
- Fix libvirtd endless loop when starting network with multiple IPs (bz
#1393975)
* Mon Sep 5 2016 Daniel P. Berrange <berrange@redhat.com> - 2.2.0-1
- Rebase to version 2.2.0