network: don't unset the firewalld zone if it's going to be immediately re-set

Any time the firewalld zone for an interface is set, by definition
that removes it from any previous zone that it was in, so there is
really no point in unsetting the zone if it's just going to be
immediately set again.

This is useful because when firewalld reloads its rules, 3 things happen:

1) firewalld flushes *all* firewall rules (including those added by libvirt)

2) firewalld unsets the zones for all interfaces (including those set
   by libvirt)

3) firewalld re-adds its own rules, and sets the zone for all the
   interfaces it manages

4) firewalld sends a dbus message that libvirt is watching for, and
   when libvirt receives that message, it reloads all of the
   libvirt-generated rules, and also re-sets the firewalld zone for
   the bridge interfaces managed by libvirt.

libvirt accomplishes step 4 by a) calling
networkRemoveFirewallRules(), and then b) calling
networkAddFirewallRules(). But (because it is useful in other
contexts) networkRemoveFirewallRules() will attempt to *unset* the
zone for each bridge interface, and when firewalld receives this
request, it sees that the bridge interface *has no zone* (because it
was unset by firewalld in step (2) above), and thus logs an error
message.

There is no way for libvirt to suppress an error message that is
logged by firewalld when a request to firewalld fails. But what
libvirt *can* do is realize that in these cases, the firewalld zone is
about to be set again anyway, and so we don't need to unset the zone.

This patch handles that by adding a bool unsetZone to the arguments of
networkRemoveFirewallRules(); most calls to networkRemoveFirewallRules()
have unsetZone=true, but in two cases where the zone is about to be
reset, networkRemoveFirewallRules() is called with unsetZone=false,
which prevents the call to virFirewallDInterfaceUnsetZone() and thus
avoids the unnecessary (and confusing to users!) error message that
would have been logged by firewalld.

Signed-off-by: Laine Stump <laine@redat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 550759881ac00410688e29212159201fa68339ed..d408f17de73a205df065e5136c26ca33a7d8795f 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1752,7 +1752,7 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj,
              * and this functionality is also handled by
              * networkAdd/RemoveFirewallRules()
              */
-            networkRemoveFirewallRules(obj);
+            networkRemoveFirewallRules(obj, false);
             ignore_value(networkAddFirewallRules(def, cfg->firewallBackend, &fwRemoval));
             virNetworkObjSetFwRemoval(obj, fwRemoval);
             saveStatus = true;
@@ -2129,7 +2129,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
         ignore_value(virNetDevSetOnline(def->bridge, false));
 
     if (firewalRulesAdded)
-        networkRemoveFirewallRules(obj);
+        networkRemoveFirewallRules(obj, true);
 
     virNetworkObjUnrefMacMap(obj);
 
@@ -2166,7 +2166,7 @@ networkShutdownNetworkVirtual(virNetworkObj *obj)
 
     ignore_value(virNetDevSetOnline(def->bridge, false));
 
-    networkRemoveFirewallRules(obj);
+    networkRemoveFirewallRules(obj, true);
 
     ignore_value(virNetDevBridgeDelete(def->bridge));
 
@@ -3332,7 +3332,7 @@ networkUpdate(virNetworkPtr net,
                  * old rules (and remember to load new ones after the
                  * update).
                  */
-                networkRemoveFirewallRules(obj);
+                networkRemoveFirewallRules(obj, false);
                 needFirewallRefresh = true;
                 break;
             default:

diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index 6c3ec403a45372088d16ea1e7bb9f28c20102f4a..86f6a5915f74bab2882a49136863fcd06ef38eb1 100644 (file)
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -447,7 +447,8 @@ networkAddFirewallRules(virNetworkDef *def,
 
 
 void
-networkRemoveFirewallRules(virNetworkObj *obj)
+networkRemoveFirewallRules(virNetworkObj *obj,
+                           bool unsetZone)
 {
     virNetworkDef *def = virNetworkObjGetDef(obj);
     virFirewall *fw;
@@ -484,9 +485,10 @@ networkRemoveFirewallRules(virNetworkObj *obj)
      * same interface name wants *no* zone set. To avoid this, we must
      * "unset" the zone if we set it when the network was started.
      */
-    if (virFirewallDIsRegistered() == 0 &&
-        (def->forward.type != VIR_NETWORK_FORWARD_OPEN ||
-         def->bridgeZone)) {
+    if (unsetZone
+        && virFirewallDIsRegistered() == 0
+        && (def->forward.type != VIR_NETWORK_FORWARD_OPEN
+            || def->bridgeZone)) {
 
         VIR_DEBUG("unsetting zone for '%s' (current zone is '%s')",
                   def->bridge, def->bridgeZone);

diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c
index 8bf3367bffa2de4330a3ce01eab07b08ed2628fb..59fc0e3c96dc2ba9e60ca0084d5af43408b81375 100644 (file)
--- a/src/network/bridge_driver_nop.c
+++ b/src/network/bridge_driver_nop.c
@@ -56,6 +56,8 @@ int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED,
     return 0;
 }
 
-void networkRemoveFirewallRules(virNetworkObj *obj G_GNUC_UNUSED)
+void
+networkRemoveFirewallRules(virNetworkObj *obj G_GNUC_UNUSED,
+                           bool unsetZone G_GNUC_UNUSED)
 {
 }

diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index cd2e3fa7b52303e6393b97b537cc9ccd49a5db47..6a393c97336a92d01388b154864b47c70519a007 100644 (file)
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -36,4 +36,5 @@ int networkAddFirewallRules(virNetworkDef *def,
                             virFirewallBackend firewallBackend,
                             virFirewall **fwRemoval);
 
-void networkRemoveFirewallRules(virNetworkObj *obj);
+void networkRemoveFirewallRules(virNetworkObj *obj,
+                                bool unsetZone);