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

On Fri, Jun 14, 2024 at 12:22:50PM -0400, Andrea Bolognani wrote: > On Fri, Jun 14, 2024 at 03:43:53PM GMT, Daniel P. Berrangé wrote: > > @@ -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. This is the behaviour we already had before the nftables backend was created, and its not been a problem. There's no functional reason why we should refuse to allow it to be defined, if the binaries aren't needed until startup.