[libvirt PATCH 08/12] 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(a)redhat.com>
---
src/util/virfirewall.c | 54 ++++++++++++++++++++++++++----------------
src/util/viriptables.c | 6 ++---
2 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index bb14a367d9..2fc9f94729 100644
--- 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 d2bc10a652..34ce9cd018 100644
--- 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();
--
2.33.1