On Sun, Apr 21, 2024 at 10:53:28PM -0400, Laine Stump wrote: > When destroying a network, the network driver has always assumed that > it knew what firewall rules had been added as the network was > started. This was usually correct - I only recall one time in the past > that the firewall rules added by libvirt were changed. But if the > exact rules used for a network *were* ever changed from one > build/version of libvirt to another, then we would end up attempting > to remove rules that hadn't been added, and could possibly *not* > remove rules that had been added. > > The solution to this to not make such brash assumptions about the > past, but instead to save (in the network status object at network > start time) a list of all the rules needed to remove the rules that > were added for the network, and then use that saved list during > network destroy to remove exactly what was previous added. > > Beyond making net-destroy more precise, there are other benefits: > > 1) We can change the details of the rules we add for networks from one > build/release of libvirt to another and painlessly upgrade. > > 2) The user can switch from one firewall backend to another by simply > changing the setting in network.conf and restarting > libvirtd/virtnetworkd. > > In both cases, the restarted libvirtd/virtnetworkd will remove all the > rules that had been previously added (based on the network status), > and then add new rules (saving the new removal commands back into the > network status) > > Signed-off-by: Laine Stump <laine@xxxxxxxxxx> > > == > > NB: the current implementation saves only the commands necessary to > remove the network's firewall, and names this <firewall> in the status > XML. It would be simple to instead save the *entire* virFirewall > object (thus also including the commands that were used to add the > firewall, as well as the commands needed to remove it) - although very > verbose, it's possible it could be useful when debugging a firewall > issue (since it's not obvious which rules were added for which network > when just looking at the output of "nft list ruleset". Alternately, we > could continue to store only the removal commands, but maybe change > the name of the element in XML from <firewall> to <fwRemoval> (which > would leave the door open to expanding what is saved in the > future). Any opinions on this? IMHO we should just stick with recording the info we need from a functional POV. Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| _______________________________________________ Devel mailing list -- devel@xxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx