util: simplify virFirewallBackendSynchronize()

This function doesn't need to check for a backend - synchronization
with firewalld should always be done whenever firewalld is registered
and available, not just when the firewalld backend is selected.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

src/util/virfirewall.c

@@ -616,27 +616,41 @@ virFirewallBackendSynchronize(void)
 {
     const char *arg = "-V";
     g_autofree char *output = NULL;
+    int firewallDRegistered = virFirewallDIsRegistered();
 
-    switch (currentBackend) {
+    /*
-    case VIR_FIREWALL_BACKEND_DIRECT:
+     * virFirewallBackendSynchronize() should be called after
-        /* nobody to synchronize with */
+     * receiving an ownership-change event or reload event for
-        break;
+     * firewalld from dbus, prior to performing any operations on the
-    case VIR_FIREWALL_BACKEND_FIREWALLD:
+     * default table "filter".
-        /* Send a simple rule via firewalld's passthrough iptables
+     *
-         * command so that we'll be sure firewalld has fully
+     * Our iptables filter rules are added to (private chains within)
-         * initialized and caught up with its internal queue of
+     * the default table named "filter", which is flushed by firewalld
-         * iptables commands. Waiting for this will prevent our own
+     * any time it is restarted or reloads its rules. libvirt watches
-         * directly-executed iptables commands from being run while
+     * for notifications that firewalld has been restarted / its rules
-         * firewalld is still initializing.
+     * reloaded, and then reloads the libvirt rules. But it's possible
+     * for libvirt to be notified that firewalld has restarted prior
+     * to firewalld completing initialization, and when that race
+     * happens, firewalld can potentially flush out rules that libvirt
+     * has just added!
+     *
+     * To prevent this, we send a simple command ("iptables -V") via
+     * firewalld's passthrough iptables API, and wait until it's
+     * finished before sending our own directly-executed iptables
+     * commands. This assures that firewalld has fully initialized and
+     * caught up with its internal queue of iptables commands, and
+     * won't stomp all over the new rules we subsequently add.
+     *
      */
+
+    VIR_DEBUG("Firewalld is registered ? %d", firewallDRegistered);
+
+    if (firewallDRegistered < 0)
+        return; /* firewalld (or dbus?) not functional, don't sync */
+
     ignore_value(virFirewallDApplyRule(VIR_FIREWALL_LAYER_IPV4,
                                        (char **)&arg, 1, true, &output));
     VIR_DEBUG("Result of 'iptables -V' via firewalld: %s", NULLSTR(output));
-        break;
-    case VIR_FIREWALL_BACKEND_AUTOMATIC:
-    case VIR_FIREWALL_BACKEND_LAST:
-        break;
-    }
 }
 
 

src/util/viriptables.c

@@ -138,10 +138,10 @@ iptablesSetupPrivateChains(virFirewallLayer layer)
     };
     size_t i;
 
-    /* When the backend is firewalld, we need to make sure that
+    /* When firewalld.service is active, we need to make sure that
      * firewalld has been fully started and completed its
-     * initialization, otherwise firewalld might delete our rules soon
+     * initialization, otherwise it might delete our rules soon after
-     * after we add them!
+     * we add them!
      */
     virFirewallBackendSynchronize();