On Mon, Oct 07, 2024 at 00:19:41 -0400, Laine Stump wrote: > (this is a remake of commit v10.7.0-78-g200f60b2e1, which was reverted > due to a regression in another patch it was dependent on. The new > implementation just adds the call to virFirewallDInterfaceUnsetZone() > into the existing networkRemoveFirewallRules() (but only if we had set > a zone when the network was first started). > > Replaces: 200f60b2e12e68d618f6d59f0173bb507b678838 > Resolves: https://issues.redhat.com/browse/RHEL-61576 > Signed-off-by: Laine Stump <laine@xxxxxxxxxx> > --- > src/libvirt_private.syms | 1 + > src/network/bridge_driver_linux.c | 29 +++++++++++++++++++++++------ > src/util/virfirewalld.c | 23 +++++++++++++++++++++++ > src/util/virfirewalld.h | 2 ++ > 4 files changed, 49 insertions(+), 6 deletions(-) > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index cafb41166b..e09fb98596 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -2452,6 +2452,7 @@ virFirewallDGetPolicies; > virFirewallDGetVersion; > virFirewallDGetZones; > virFirewallDInterfaceSetZone; > +virFirewallDInterfaceUnsetZone; > virFirewallDIsRegistered; > virFirewallDPolicyExists; > virFirewallDSynchronize; > diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c > index 8956d38ab1..bafa9e26f9 100644 > --- a/src/network/bridge_driver_linux.c > +++ b/src/network/bridge_driver_linux.c > @@ -459,19 +459,36 @@ networkRemoveFirewallRules(virNetworkObj *obj) > } else { > > if ((fw = virNetworkObjGetFwRemoval(obj)) == NULL) { > + > /* No information about firewall rules in the network status, > * so we assume the old iptables-based rules from 10.2.0 and > * earlier. > */ > VIR_DEBUG("No firewall info in status of network '%s', assuming old-style iptables", def->name); > iptablesRemoveFirewallRules(def); > - return; > + > + } else { > + > + /* fwRemoval info was stored in the network status, so use that to > + * remove the firewall > + */ > + VIR_DEBUG("Removing firewall rules of network '%s' using commands saved in status", def->name); > + virFirewallApply(fw); > } > + } > > - /* fwRemoval info was stored in the network status, so use that to > - * remove the firewall > - */ > - VIR_DEBUG("Removing firewall rules of network '%s' using commands saved in status", def->name); > - virFirewallApply(fw); > + /* all forward modes could have had a zone set, even 'open' mode > + * iff it was specified in the config. firewalld preserves the > + * name of an interface in a zone's list even after the interface > + * has been deleted, which is problematic if the next use of that > + * 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 == NULL)) { We put LF after &&. Also personally I would find if (virFirewallDIsRegistered() == 0 && (def->forward.type != VIR_NETWORK_FORWARD_OPEN || def->bridgeZone)) { easier to read without having to think about it too much :-) > + > + VIR_DEBUG("unsetting zone for '%s' (current zone is '%s')", > + def->bridge, def->bridgeZone); > + virFirewallDInterfaceUnsetZone(def->bridge); > } > } Jirka
