On Sun, Apr 21, 2024 at 10:53:18PM -0400, Laine Stump wrote:
> (This paragraph is for historical reference only, described only to
> avoid confusion of past use of the name with its new use) In a past
> life, virFirewallBackend had been a private static in virfirewall.c
> that was set at daemon init time, and used to globally (i.e. for all
> drivers in the daemon) determine whether to directly execute iptables
> commands, or to run them indirectly via the firewalld passthrough
> API. This was removed in commit d566cc55, since we decided that using
> the firewalld passthrough API is never appropriate.
>
> Now the same enum, virFirewallBackend, is being reintroduced, with a
> different meaning and usage pattern. It will be used to pick between
> using nftables commands or iptables commands (in either case directly
> handled by libvirt, *not* via firewalld). Additionally, rather than
> being a static known only within virfirewall.c and applying to all
> firewall commands for all drivers, each virFirewall object will have
> its own backend setting, which will be set during virFirewallNew() by
> the driver who wants to add a firewall rule.
>
> This will allow the nwfilter and network drivers to each have their
> own backend setting, even when they coexist in a single unified
> daemon. At least as important as that, it will also allow an instance
> of the network driver to remove iptables rules that had been added by
> a previous instance, and then add nftables rules for the new instance
> (in the case that an admin, or possibly an update, switches the driver
> backend from iptables to nftable)
>
> Initially, the enum will only have one usable value -
> VIR_FIREWALL_BACKEND_IPTABLES, and that will be hardcoded into all
> calls to virFirewallNew(). The other enum value (along with a method
> of setting it for each driver) will be added later, when it can be
> used (when the nftables backend is in the code).
>
> Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
> ---
> src/libvirt_private.syms | 3 +++
> src/network/network_iptables.c | 6 +++---
> src/nwfilter/nwfilter_ebiptables_driver.c | 16 ++++++++--------
> src/util/virebtables.c | 4 ++--
> src/util/virfirewall.c | 16 +++++++++++++++-
> src/util/virfirewall.h | 12 +++++++++++-
> tests/virfirewalltest.c | 20 ++++++++++----------
> 7 files changed, 52 insertions(+), 25 deletions(-)
>
> diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
> index 56d43bfdde..489482fd83 100644
> --- a/src/util/virfirewall.c
> +++ b/src/util/virfirewall.c
> @@ -35,6 +35,11 @@
>
> VIR_LOG_INIT("util.firewall");
>
> +VIR_ENUM_IMPL(virFirewallBackend,
> + VIR_FIREWALL_BACKEND_LAST,
> + "UNSET", /* not yet set */
> + "iptables");
> +
snip
> diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h
> index 956bf0e2bf..7f0fee047d 100644
> --- a/src/util/virfirewall.h
> +++ b/src/util/virfirewall.h
> @@ -34,9 +35,18 @@ typedef enum {
> VIR_FIREWALL_LAYER_LAST,
> } virFirewallLayer;
>
> -virFirewall *virFirewallNew(void);
> +typedef enum {
> + VIR_FIREWALL_BACKEND_UNSET,
> + VIR_FIREWALL_BACKEND_IPTABLES,
> +
> + VIR_FIREWALL_BACKEND_LAST,
> +} virFirewallBackend;
We're forcing the callers to all pass VIR_FIREWALL_BACKEND_IPTABLES,
so I'm wondering if we actually need to have VIR_FIREWALL_BACKEND_UNSET
at all ? If we eliminate it, then that removes need to check for this
illegal value and report errors I guess.
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
