On Wed, Aug 07, 2024 at 04:00:09PM -0400, Laine Stump wrote:
> On 8/7/24 1:32 PM, Daniel P. Berrangé wrote:
> > On Wed, Aug 07, 2024 at 01:16:01PM -0400, Laine Stump wrote:
> > > +
> > > +import libvirt
> > > +import sys
> > > +import os
> > > +import libxml2
> > > +from ipaddress import ip_network
> > > +
> > > +# This script should be installed in
> > > +# /usr/lib/NetworkManager/dispatcher.d/50-libvirt-check-nets. It will be
> > > +# called by NetworkManager every time a network interface is taken up
> > > +# or down. When a new network comes up, it checks the libvirt virtual
> > > +# networks to see if their IP address(es) (including any static
> > > +# routes) are in conflict with the IP address(es) (or static routes)
> > > +# of the newly added interface. If so, the libvirt network is
> > > +# disabled. It is assumed that the user will notice that their guests
> > > +# no longer have network connectvity (and/or the message logged by
> > > +# this script), see that the network has been disabled, and then
> > > +# realize the conflict when they try to restart it.
> > > +#
> > > +# set checkDefaultOnly=False to check *all* active libvirt networks
> > > +# for conflicts with the new interface. Set to True to check only the
> > > +# libvirt default network (since most networks other than the default
> > > +# network are added post-install at a time when all of the hosts other
> > > +# networks are already active, it may be overkill to check all of the
> > > +# libvirt networks for conflict here (and instead just add more
> > > +# needless overheard to bringing up a new host interface).
> > > +#
> > > +checkDefaultOnly = False
> > > +
> > > +# NB: since this file is installed in /usr/lib, it really shouldn't be
> > > +# modified by the user, but instead should be copied to
> > > +# /etc/NetworkManager/dispatcher.d, where it will override the copy in
> > > +# /usr/lib. Even that isn't a proper solution though - if we're going
> > > +# to actually have this config knob, perhaps we should check for it in
> > > +# the environment, and if someone wants to modify it they can put a
> > > +# short script in /etc that exports and environment variable and then
> > > +# calls this script? Just thinking out loud here.
> > > +
> > > +
> > > +def checkconflict(conn, netname, hostnets, hostif):
> >
> > ...snip complex code...
> >
> > It occurrs to me that this python code is entirely duplicating
> > logic that already exists in virtnetworkd that checks for
> > conflicts.
>
> Possibly not *exactly* what's in virtnetworkd, since I'm not certain if NM
> includes all static routes in the list it puts in the environment variables
> (while they *would* show up in the system routing table that our network
> driver uses). Of course that sounds like another reason to prefer re-using
> the existing code!
Oh definitely a strong reason to reuse the code. We don't want a situation
where users file bugs because we don't detect clashes in one scenario, but
do in the other.
> > This makes me want to figure out a way to just re-use the existing
> > code rather than having to write it again.
> >
> > Could we just send a SIGUSR2 to virtnetworkd as a way to trigger it
> > to re-validate all running networks ? That would be easy to trigger
> > from non-network manager setups too.
>
> That sounds simpler (and probably quicker), and if we could do it that I
> would prefer it ("only works with NetworkManager" is #1 on my list of
> 'issues' above after all :-)), but what would we do in the case that
> virtnetworkd (or libvirtd) wasn't currently running? Run some virsh command
> to trigger virtnetworkd to start, and then send the SIGUSR2?
As long as there are networks running, libvirtd/virnetworkd should not
shut down due to the auto shutdown timer.
They might temporarily shutdown due to a restart on package upgrades.
If our startup code runs the "check for clashes" logic, then we'll
do the right thing.
The only problem will be if a user manually stops the daemon and
leaves it stopped, while still having a network present. I'm happy
to call that user error and not worry about solving clashes in
that case, as no normal system should get in that state.
> (this does
> > But then how about ignoring network manager entirely and going right
> > to the source, ie the kernel. Does it send out either udev or netlink
> > events (probably the latter), when NICs have their link status set
> > online/offline ? If so, virtnetworkd could just listen to the kernel
> > events and "do the right thing", regardless of what tool is managing
> > the NICs.
>
> I imagine *something* is sent out, and it's definitely worth me
> investigating. But wouldn't virtnetworkd have to be running all of the time
> in order to use something like that?
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 :|
