On Fri, Jun 14, 2024 at 03:43:53PM GMT, Daniel P. Berrangé wrote:
> meson.build | 26 +++++++++++++++++++-------
> meson_options.txt | 2 +-
> src/network/bridge_driver_conf.c | 19 ++++++++++++++-----
> src/network/bridge_driver_linux.c | 10 ++++++++++
> src/network/bridge_driver_nop.c | 15 ++++++++++++++-
> src/util/virfirewall.c | 6 ++++++
> src/util/virfirewall.h | 1 +
> 7 files changed, 65 insertions(+), 14 deletions(-)
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.
> firewall_backend_priority = get_option('firewall_backend_priority')
> - if (not firewall_backend_priority.contains('nftables') or
> - not firewall_backend_priority.contains('iptables') or
> - firewall_backend_priority.length() != 2)
> - error('invalid value for firewall_backend_priority option')
> + if firewall_backend_priority.length() == 0
> + if host_machine.system() == 'linux'
> + firewall_backend_priority = ['nftables', 'iptables']
> + else
> + # No firewall impl on non-Linux so far, so force 'none'
> + # as placeholder
> + firewall_backend_priority = ['none']
> + endif
> + else
> + if host_machine.system() != 'linux'
> + error('firewall backend priority only supported on linux hosts')
> + endif
> endif
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.
> @@ -815,6 +816,11 @@ virFirewallApplyCmd(virFirewall *firewall,
> switch (virFirewallGetBackend(firewall)) {
> + case VIR_FIREWALL_BACKEND_NONE:
> + virReportError(VIR_ERR_NO_SUPPORT,
> + _("Firewall backend is not implemented"));
> + return -1;
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
