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>

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:

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);

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)
 {
 }

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);