On Mon, Jun 17, 2024 at 04:44:01AM -0400, Andrea Bolognani wrote:
> On Fri, Jun 14, 2024 at 07:52:55PM GMT, Daniel P. Berrangé wrote:
> > 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.
>
> But in the case of qemu:///session, or when we're not on Linux, we
> already know that there is no way for the network that's being
> defined to ever work.
>
> To me that's the same as allowing a guest to be defined, when the
> corresponding QEMU binary doesn't support some of the devices. Or
> when a firmware binary satisfying the constraints isn't available. We
> reject such configurations for guests, and it would just be
> consistent to do the same for networks.
FWIW, I find the QEMU behaviour really unpleasant / hostile, when I'm
working with guests that reference a self-built QEMU, as the binary
often "doesn't exist", if I've cleaned by build temporarily. We're
forced into this practice in QEMU, because we need to expand the
XML with defaults to fix the guest ABI. I don't like the idea of
propogating this practice elsewhere though.
Ultimately I think we should not provide the network driver at all
when running as non-root, but my attempt todo that exposed wierd
edgecases, as we've assumed the secondary drivers are always present
and cleaning that up properly is quite a bit of work
> Besides, even if we decide that we want to allow such networks to be
> defined, we should report a more meaningful error for the failed
> startup, one which points out that mode=nat will never work. It
> shouldn't be the same error message that I was getting on FreeBSD
> because of a kernel driver that wasn't loaded, as the user's ability
> to make the error go away is completely different in the two
> scenarios.
Overall, there's lots that could be improved in the network driver,
but for this, I'd really just like to focus on fixing the regression,
suc that we return to the behaviour we had historically.
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 :|
