Re: [PATCH v2] network: introduce a "none" firewall backend type

On Fri, Jun 14, 2024 at 03:43:53PM GMT, Daniel P. Berrangé wrote: The test suite no longer passes after applying this. At the very least, you need to squash in the diff at the bottom of this message. This implementation allows things such as -Dfirewall_backend_priority=nftables and -Dfirewall_backend_priority=iptables,iptables At least -Dfirewall_backend_priority=iptables,nftables,iptables will be blocked, but only because it results in a compilation error: meson will happily accept it. Are we okay with that? It's IMO inferior to the much stricter checking that's performed today. This check (and the other ones you've introduced) are running too late, which leads to this behavior: $ cat default-session.xml <network> <name>default-session</name> <forward mode='nat'/> <bridge name='virbr0' stp='on' delay='0'/> <ip address='192.168.123.1' netmask='255.255.255.0'> <dhcp> <range start='192.168.123.2' end='192.168.123.254'/> </dhcp> </ip> </network> $ virsh -c qemu:///session net-define default-session.xml Network default-session defined from default-session.xml $ virsh -c qemu:///session net-start default-session error: Failed to start network default-session error: error creating bridge interface virbr0: Operation not permitted Since <forward mode='nat'/> requires firewall rules, we shouldn't allow a network that uses it to be defined in the first place. diff --git a/po/POTFILES b/po/POTFILES index 4bfbb91164..1ed4086d2c 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -143,6 +143,7 @@ src/lxc/lxc_process.c src/network/bridge_driver.c src/network/bridge_driver_conf.c src/network/bridge_driver_linux.c +src/network/bridge_driver_nop.c src/network/leaseshelper.c src/network/network_iptables.c src/network/network_nftables.c diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c index 2114d521d1..323990cf17 100644 --- a/src/network/bridge_driver_nop.c +++ b/src/network/bridge_driver_nop.c @@ -49,8 +49,8 @@ int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED, */ if (firewallBackend != VIR_FIREWALL_BACKEND_NONE) { virReportError(VIR_ERR_NO_SUPPORT, - _("Firewall backend '%s' not available on this platform"), - virFirewallBackendTypeToString(firewallBackend)); + _("Firewall backend '%1$s' not available on this platform"), + virFirewallBackendTypeToSTring(firewallBackend)); return -1; } return 0; diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index d374f54b64..090dbcdbed 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -817,7 +817,7 @@ virFirewallApplyCmd(virFirewall *firewall, switch (virFirewallGetBackend(firewall)) { case VIR_FIREWALL_BACKEND_NONE: - virReportError(VIR_ERR_NO_SUPPORT, + virReportError(VIR_ERR_NO_SUPPORT, "%s", _("Firewall backend is not implemented")); return -1; -- Andrea Bolognani / Red Hat / Virtualization