138 lines
4.7 KiB
Diff
138 lines
4.7 KiB
Diff
From b990740b12117eaaf2797141a53a30b41f07c791 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
|
|
Date: Mon, 18 Mar 2019 17:31:21 +0000
|
|
Subject: [PATCH 3/5] network: improve error report when firewall chain
|
|
creation fails
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
During startup we create some top level chains in which all
|
|
virtual network firewall rules will be placed. The upfront
|
|
creation is done to avoid slowing down creation of individual
|
|
virtual networks by checking for chain existance every time.
|
|
|
|
There are some factors which can cause this upfront creation
|
|
to fail and while a message will get into the libvirtd log
|
|
this won't be seen by users who later try to start a virtual
|
|
network. Instead they'll just get a message saying that the
|
|
libvirt top level chain does not exist. This message is
|
|
accurate, but unhelpful for solving the root cause.
|
|
|
|
This patch thus saves any error during daemon startup and
|
|
reports it when trying to create a virtual network later.
|
|
|
|
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
|
|
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
|
|
(cherry picked from commit 9f4e35dc73ec9e940aa61bc7c140c2b800218ef3)
|
|
---
|
|
src/network/bridge_driver.c | 3 +--
|
|
src/network/bridge_driver_linux.c | 31 +++++++++++++++++++++-------
|
|
src/network/bridge_driver_nop.c | 3 +--
|
|
src/network/bridge_driver_platform.h | 2 +-
|
|
4 files changed, 27 insertions(+), 12 deletions(-)
|
|
|
|
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
|
|
index b3ca5b8a15..1da60f0a21 100644
|
|
--- a/src/network/bridge_driver.c
|
|
+++ b/src/network/bridge_driver.c
|
|
@@ -2108,8 +2108,7 @@ static void
|
|
networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup)
|
|
{
|
|
VIR_INFO("Reloading iptables rules");
|
|
- if (networkPreReloadFirewallRules(startup) < 0)
|
|
- return;
|
|
+ networkPreReloadFirewallRules(startup);
|
|
virNetworkObjListForEach(driver->networks,
|
|
networkReloadFirewallRulesHelper,
|
|
NULL);
|
|
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
|
|
index b10d0a6c4d..c899f4b6d0 100644
|
|
--- a/src/network/bridge_driver_linux.c
|
|
+++ b/src/network/bridge_driver_linux.c
|
|
@@ -35,11 +35,25 @@ VIR_LOG_INIT("network.bridge_driver_linux");
|
|
|
|
#define PROC_NET_ROUTE "/proc/net/route"
|
|
|
|
-int networkPreReloadFirewallRules(bool startup)
|
|
+static virErrorPtr errInit;
|
|
+
|
|
+void networkPreReloadFirewallRules(bool startup)
|
|
{
|
|
- int ret = iptablesSetupPrivateChains();
|
|
- if (ret < 0)
|
|
- return -1;
|
|
+ int rc;
|
|
+
|
|
+ /* We create global rules upfront as we don't want
|
|
+ * the perf hit of conditionally figuring out whether
|
|
+ * to create them each time a network is started.
|
|
+ *
|
|
+ * Any errors here are saved to be reported at time
|
|
+ * of starting the network though as that makes them
|
|
+ * more likely to be seen by a human
|
|
+ */
|
|
+ rc = iptablesSetupPrivateChains();
|
|
+ if (rc < 0) {
|
|
+ errInit = virSaveLastError();
|
|
+ virResetLastError();
|
|
+ }
|
|
|
|
/*
|
|
* If this is initial startup, and we just created the
|
|
@@ -54,10 +68,8 @@ int networkPreReloadFirewallRules(bool startup)
|
|
* rules will be present. Thus we can safely just tell it
|
|
* to always delete from the builin chain
|
|
*/
|
|
- if (startup && ret == 1)
|
|
+ if (startup && rc == 1)
|
|
iptablesSetDeletePrivate(false);
|
|
-
|
|
- return 0;
|
|
}
|
|
|
|
|
|
@@ -671,6 +683,11 @@ int networkAddFirewallRules(virNetworkDefPtr def)
|
|
virFirewallPtr fw = NULL;
|
|
int ret = -1;
|
|
|
|
+ if (errInit) {
|
|
+ virSetError(errInit);
|
|
+ return -1;
|
|
+ }
|
|
+
|
|
if (def->bridgeZone) {
|
|
|
|
/* if a firewalld zone has been specified, fail/log an error
|
|
diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c
|
|
index a0e57012f9..ea9db338cb 100644
|
|
--- a/src/network/bridge_driver_nop.c
|
|
+++ b/src/network/bridge_driver_nop.c
|
|
@@ -19,9 +19,8 @@
|
|
|
|
#include <config.h>
|
|
|
|
-int networkPreReloadFirewallRules(bool startup ATTRIBUTE_UNUSED)
|
|
+void networkPreReloadFirewallRules(bool startup ATTRIBUTE_UNUSED)
|
|
{
|
|
- return 0;
|
|
}
|
|
|
|
|
|
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
|
|
index baeb22bc3e..95fd64bdc7 100644
|
|
--- a/src/network/bridge_driver_platform.h
|
|
+++ b/src/network/bridge_driver_platform.h
|
|
@@ -58,7 +58,7 @@ struct _virNetworkDriverState {
|
|
typedef struct _virNetworkDriverState virNetworkDriverState;
|
|
typedef virNetworkDriverState *virNetworkDriverStatePtr;
|
|
|
|
-int networkPreReloadFirewallRules(bool startup);
|
|
+void networkPreReloadFirewallRules(bool startup);
|
|
void networkPostReloadFirewallRules(bool startup);
|
|
|
|
int networkCheckRouteCollision(virNetworkDefPtr def);
|
|
--
|
|
2.20.1
|
|
|