On 4/23/24 6:10 AM, Daniel P. Berrangé wrote:
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,
That is the case now, but as soon as we add another backend, then it's
possible for it to be UNSET (if the init code that sets it hasn't been
run yet (should never happen), or (and this is more likely) it can't
find either iptables or nft and so the autodetect fails.
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
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
