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>

diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index bb14a367d94fe8dc6d3835cbdbfbbf34c0ca5fc4..2fc9f9472920a36b68b7d3a5b5c6ee7757488b40 100644 (file)
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -616,27 +616,41 @@ virFirewallBackendSynchronize(void)
 {
     const char *arg = "-V";
     g_autofree char *output = NULL;
+    int firewallDRegistered = virFirewallDIsRegistered();
+
+    /*
+     * virFirewallBackendSynchronize() should be called after
+     * receiving an ownership-change event or reload event for
+     * firewalld from dbus, prior to performing any operations on the
+     * default table "filter".
+     *
+     * Our iptables filter rules are added to (private chains within)
+     * the default table named "filter", which is flushed by firewalld
+     * any time it is restarted or reloads its rules. libvirt watches
+     * for notifications that firewalld has been restarted / its rules
+     * 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.
+     *
+     */
 
-    switch (currentBackend) {
-    case VIR_FIREWALL_BACKEND_DIRECT:
-        /* nobody to synchronize with */
-        break;
-    case VIR_FIREWALL_BACKEND_FIREWALLD:
-        /* Send a simple rule via firewalld's passthrough iptables
-         * command so that we'll be sure firewalld has fully
-         * initialized and caught up with its internal queue of
-         * iptables commands. Waiting for this will prevent our own
-         * directly-executed iptables commands from being run while
-         * firewalld is still initializing.
-         */
-        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;
-    }
+    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));
 }
 
 

diff --git a/src/util/viriptables.c b/src/util/viriptables.c
index d2bc10a6524eaa3f941176959119d32d58382199..34ce9cd0182af2916424e17f9b0e20130b4797df 100644 (file)
--- a/src/util/viriptables.c
+++ b/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
-     * after we add them!
+     * initialization, otherwise it might delete our rules soon after
+     * we add them!
      */
     virFirewallBackendSynchronize();