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@xxxxxxxxxx> --- 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