9:32 p.m.
Any time the firewalld zone for an interface is set, by definition
that removes it from any previous zone that it was in, so there is
really no point in unsetting the zone if it's just going to be
immediately set again.
(incoming "weave" - it meanders a bit, but then ties together into a
point. Bigly.)
This is useful because when firewalld reloads its rules, 3 things happen:
1) firewalld flushes *all* firewall rules (including those added by libvirt)
2) firewalld unsets the zones for all interfaces (including those set
by libvirt)
3) firewalld re-adds its own rules, and sets the zone for all the
interfaces it manages
4) firewalld sends a dbus message that libvirt is watching for, and
when libvirt receives that message, it reloads all of the
libvirt-generated rules, and also re-sets the firewalld zone for
the bridge interfaces managed by libvirt.
libvirt accomplishes step 4 by a) calling
networkRemoveFirewallRules(), and then b) calling
networkAddFirewallRules(). But (because it is useful in other
contexts) networkRemoveFirewallRules() will attempt to *unset* the
zone for each bridge interface, and when firewalld receives this
request, it will that the bridge interface *has no zone* (because it
was unset by firewalld in step (2) above), and thus logs an error
message.
There is no way for libvirt to suppress an error message that is
logged by firewalld when a request to firewalld fails. But what
libvirt *can* do is realize that in these cases, the firewalld zone is
about to be set again anyway, and so we don't need to call firewalld
to unset the zone in the first place.
This patch handles that by adding a bool unsetZone to the arguments of
networkRemoveFirewallRules(); most calls to networkRemoveFirewallRules()
have unsetZone=true, but in two cases where the zone is about to be
reset, networkRemoveFirewallRules() is called with unsetZone=false,
which prevents the call to virFirewallDInterfaceUnsetZone() and thus
avoids the unnecessary (and confusing to users!) error message that
would have been logged by firewalld.
(see - that weave ended up sewed together, right?)
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/network/bridge_driver.c | 8 ++++----
src/network/bridge_driver_linux.c | 10 ++++++----
src/network/bridge_driver_nop.c | 4 +++-
src/network/bridge_driver_platform.h | 3 ++-
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 550759881a..d408f17de7 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1752,7 +1752,7 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj,
* and this functionality is also handled by
* networkAdd/RemoveFirewallRules()
*/
- networkRemoveFirewallRules(obj);
+ networkRemoveFirewallRules(obj, false);
ignore_value(networkAddFirewallRules(def, cfg->firewallBackend,
&fwRemoval));
virNetworkObjSetFwRemoval(obj, fwRemoval);
saveStatus = true;
@@ -2129,7 +2129,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
ignore_value(virNetDevSetOnline(def->bridge, false));
if (firewalRulesAdded)
- networkRemoveFirewallRules(obj);
+ networkRemoveFirewallRules(obj, true);
virNetworkObjUnrefMacMap(obj);
@@ -2166,7 +2166,7 @@ networkShutdownNetworkVirtual(virNetworkObj *obj)
ignore_value(virNetDevSetOnline(def->bridge, false));
- networkRemoveFirewallRules(obj);
+ networkRemoveFirewallRules(obj, true);
ignore_value(virNetDevBridgeDelete(def->bridge));
@@ -3332,7 +3332,7 @@ networkUpdate(virNetworkPtr net,
* old rules (and remember to load new ones after the
* update).
*/
- networkRemoveFirewallRules(obj);
+ networkRemoveFirewallRules(obj, false);
needFirewallRefresh = true;
break;
default:
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index 6c3ec403a4..86f6a5915f 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -447,7 +447,8 @@ networkAddFirewallRules(virNetworkDef *def,
void
-networkRemoveFirewallRules(virNetworkObj *obj)
+networkRemoveFirewallRules(virNetworkObj *obj,
+ bool unsetZone)
{
virNetworkDef *def = virNetworkObjGetDef(obj);
virFirewall *fw;
@@ -484,9 +485,10 @@ networkRemoveFirewallRules(virNetworkObj *obj)
* same interface name wants *no* zone set. To avoid this, we must
* "unset" the zone if we set it when the network was started.
*/
- if (virFirewallDIsRegistered() == 0 &&
- (def->forward.type != VIR_NETWORK_FORWARD_OPEN ||
- def->bridgeZone)) {
+ if (unsetZone
+ && virFirewallDIsRegistered() == 0
+ && (def->forward.type != VIR_NETWORK_FORWARD_OPEN
+ || def->bridgeZone)) {
VIR_DEBUG("unsetting zone for '%s' (current zone is
'%s')",
def->bridge, def->bridgeZone);
diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c
index 8bf3367bff..59fc0e3c96 100644
--- a/src/network/bridge_driver_nop.c
+++ b/src/network/bridge_driver_nop.c
@@ -56,6 +56,8 @@ int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED,
return 0;
}
-void networkRemoveFirewallRules(virNetworkObj *obj G_GNUC_UNUSED)
+void
+networkRemoveFirewallRules(virNetworkObj *obj G_GNUC_UNUSED,
+ bool unsetZone G_GNUC_UNUSED)
{
}
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index cd2e3fa7b5..6a393c9733 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -36,4 +36,5 @@ int networkAddFirewallRules(virNetworkDef *def,
virFirewallBackend firewallBackend,
virFirewall **fwRemoval);
-void networkRemoveFirewallRules(virNetworkObj *obj);
+void networkRemoveFirewallRules(virNetworkObj *obj,
+ bool unsetZone);
--
2.47.0