[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 1/4] network: add platform driver callbacks around firewall reload



On 12/7/18 11:21 AM, Daniel P. Berrangé wrote:
Allow the platform driver impls to run logic before and after the
firewall reload process.

Signed-off-by: Daniel P. Berrangé <berrange redhat com>
---
  src/network/bridge_driver.c          | 13 ++++++++-----
  src/network/bridge_driver_linux.c    | 11 +++++++++++
  src/network/bridge_driver_nop.c      | 11 +++++++++++
  src/network/bridge_driver_platform.h |  3 +++
  4 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 4bbc4f5a6d..11095bf974 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -165,7 +165,7 @@ static int
  networkShutdownNetworkExternal(virNetworkObjPtr obj);
static void
-networkReloadFirewallRules(virNetworkDriverStatePtr driver);
+networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup);
static void
  networkRefreshDaemons(virNetworkDriverStatePtr driver);
@@ -553,7 +553,7 @@ firewalld_dbus_filter_bridge(DBusConnection *connection ATTRIBUTE_UNUSED,
                                 "Reloaded"))
      {
          VIR_DEBUG("Reload in bridge_driver because of firewalld.");
-        networkReloadFirewallRules(driver);
+        networkReloadFirewallRules(driver, false);


Okay, I think I get what you're doing here - you wanted a place to put code that would only be run once when libvirtd starts, and not when the rules are reloaded due to firewalld restarting, right? But doesn't a restart of firewalld also remove all the private chains?


Or did I miss the point? I guess I must have, because when I built with these patches and tried restarting firewalld, I still saw the private chains after the restart.


So, since your method seems to work, and this code is all very straightforward:


Reviewed-by: Laine Stump <laine laine org>


      }
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
@@ -753,7 +753,7 @@ networkStateInitialize(bool privileged,
      virNetworkObjListPrune(network_driver->networks,
                             VIR_CONNECT_LIST_NETWORKS_INACTIVE |
                             VIR_CONNECT_LIST_NETWORKS_TRANSIENT);
-    networkReloadFirewallRules(network_driver);
+    networkReloadFirewallRules(network_driver, true);
      networkRefreshDaemons(network_driver);
network_driver->networkEventState = virObjectEventStateNew();
@@ -829,7 +829,7 @@ networkStateReload(void)
      virNetworkObjLoadAllConfigs(network_driver->networks,
                                  network_driver->networkConfigDir,
                                  network_driver->networkAutostartDir);
-    networkReloadFirewallRules(network_driver);
+    networkReloadFirewallRules(network_driver, false);
      networkRefreshDaemons(network_driver);
      virNetworkObjListForEach(network_driver->networks,
                               networkAutostartConfig,
@@ -2181,12 +2181,15 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr obj,
static void
-networkReloadFirewallRules(virNetworkDriverStatePtr driver)
+networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup)
  {
      VIR_INFO("Reloading iptables rules");
+    if (networkPreReloadFirewallRules(startup) < 0)
+        return;
      virNetworkObjListForEach(driver->networks,
                               networkReloadFirewallRulesHelper,
                               NULL);
+    networkPostReloadFirewallRules(startup);
  }
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index fb09954b8f..5650e1e061 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -36,6 +36,17 @@ VIR_LOG_INIT("network.bridge_driver_linux");
#define PROC_NET_ROUTE "/proc/net/route" +int networkPreReloadFirewallRules(bool startup ATTRIBUTE_UNUSED)
+{
+    return 0;
+}
+
+
+void networkPostReloadFirewallRules(bool startup ATTRIBUTE_UNUSED)
+{
+}
+
+
  /* XXX: This function can be a lot more exhaustive, there are certainly
   *      other scenarios where we can ruin host network connectivity.
   * XXX: Using a proper library is preferred over parsing /proc
diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c
index 5e1acd07b4..64ff768b2f 100644
--- a/src/network/bridge_driver_nop.c
+++ b/src/network/bridge_driver_nop.c
@@ -21,6 +21,17 @@
#include <config.h> +int networkPreReloadFirewallRules(bool startup ATTRIBUTE_UNUSED)
+{
+    return 0;
+}
+
+
+void networkPostReloadFirewallRules(bool startup ATTRIBUTE_UNUSED)
+{
+}
+
+
  int networkCheckRouteCollision(virNetworkDefPtr def ATTRIBUTE_UNUSED)
  {
      return 0;
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index 706000df4e..2589948933 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -60,6 +60,9 @@ struct _virNetworkDriverState {
  typedef struct _virNetworkDriverState virNetworkDriverState;
  typedef virNetworkDriverState *virNetworkDriverStatePtr;
+int networkPreReloadFirewallRules(bool startup);
+void networkPostReloadFirewallRules(bool startup);
+
  int networkCheckRouteCollision(virNetworkDefPtr def);
int networkAddFirewallRules(virNetworkDefPtr def);



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]