network: properly update iptables rules during net-update

This patch resolves:

  https://bugzilla.redhat.com/show_bug.cgi?id=1035336

The basic problem is that during a network update, the required
iptables rules sometimes change, and this was being handled by simply
removing and re-adding the rules. However, the removal of the old
rules was done based on the *new* state of the network, which would
mean that some of the rules would not match those currently in the
system, so the old rules wouldn't be removed.

This patch removes the old rules prior to updating the network
definition then adds the new rules as soon as the definition is
updated. Note that this could lead to a stray packet or two during the
interim, but that was already a problem before (the period of limbo is
now just slightly longer).

While moving the location for the rules, I added a few more sections
that should result in the iptables rules being redone:

DHCP_RANGE and DHCP_HOST - these are needed because adding/removing a dhcp
host entry could lead to the dhcp service being started/stopped, which
would require that the mangle rule that fixes up dhcp response
checksums sould need to be added/removed, and this wasn't being done.

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 83dc93148d6bbcd96a76a735aa570988b6c49362..1e4cc7014aa4334567884761377e2f4bb7a14dd4 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2642,6 +2642,7 @@ networkUpdate(virNetworkPtr net,
     size_t i;
     virNetworkIpDefPtr ipdef;
     bool oldDhcpActive = false;
+    bool needFirewallRefresh = false;
 
 
     virCheckFlags(VIR_NETWORK_UPDATE_AFFECT_LIVE |
@@ -2683,8 +2684,40 @@ networkUpdate(virNetworkPtr net,
             flags |= VIR_NETWORK_UPDATE_AFFECT_CONFIG;
     }
 
+    if (isActive && (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE)) {
+        /* Take care of anything that must be done before updating the
+         * live NetworkDef.
+         */
+        if (network->def->forward.type == VIR_NETWORK_FORWARD_NONE ||
+            network->def->forward.type == VIR_NETWORK_FORWARD_NAT ||
+            network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
+            switch (section) {
+            case VIR_NETWORK_SECTION_FORWARD:
+            case VIR_NETWORK_SECTION_FORWARD_INTERFACE:
+            case VIR_NETWORK_SECTION_IP:
+            case VIR_NETWORK_SECTION_IP_DHCP_RANGE:
+            case VIR_NETWORK_SECTION_IP_DHCP_HOST:
+                /* these could affect the firewall rules, so remove the
+                 * old rules (and remember to load new ones after the
+                 * update).
+                 */
+                networkRemoveFirewallRules(network);
+                needFirewallRefresh = true;
+                break;
+            default:
+                break;
+            }
+        }
+    }
+
     /* update the network config in memory/on disk */
-    if (virNetworkObjUpdate(network, command, section, parentIndex, xml, flags) < 0)
+    if (virNetworkObjUpdate(network, command, section, parentIndex, xml, flags) < 0) {
+        if (needFirewallRefresh)
+            ignore_value(networkAddFirewallRules(network));
+        goto cleanup;
+    }
+
+    if (needFirewallRefresh && networkAddFirewallRules(network) < 0)
         goto cleanup;
 
     if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) {
@@ -2754,19 +2787,6 @@ networkUpdate(virNetworkPtr net,
                 goto cleanup;
         }
 
-        if ((section == VIR_NETWORK_SECTION_IP ||
-             section == VIR_NETWORK_SECTION_FORWARD ||
-             section == VIR_NETWORK_SECTION_FORWARD_INTERFACE) &&
-           (network->def->forward.type == VIR_NETWORK_FORWARD_NONE ||
-            network->def->forward.type == VIR_NETWORK_FORWARD_NAT ||
-            network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE)) {
-            /* these could affect the iptables rules */
-            networkRemoveFirewallRules(network);
-            if (networkAddFirewallRules(network) < 0)
-                goto cleanup;
-
-        }
-
         /* save current network state to disk */
         if ((ret = virNetworkSaveStatus(driverState->stateDir,
                                         network)) < 0) {