[PATCH 0/2] network: fix network driver to gracefully skip startup

Daniel P. Berrangé posted 2 patches 2 months, 4 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20240611164758.1036695-1-berrange@redhat.com
src/network/bridge_driver.c      | 14 +++++++++++++-
src/network/bridge_driver_conf.c |  8 ++++----
2 files changed, 17 insertions(+), 5 deletions(-)
[PATCH 0/2] network: fix network driver to gracefully skip startup
Posted by Daniel P. Berrangé 2 months, 4 weeks ago
We should gracefully skip startup when:

 * No network.conf firewall_backend is explicitly set, and
   neither iptables/nftables are present
 * Running unprivileged

The former fixes libvirtd startup on non-Linux, or minimal linux
installs without firewall tools.

The latter skips pointless initialization that creates a driver
that cannot do anything useful

Daniel P. Berrangé (2):
  network: skip network driver init if no firewall backend is present
  network: don't attempt to initialize if non-privileged

 src/network/bridge_driver.c      | 14 +++++++++++++-
 src/network/bridge_driver_conf.c |  8 ++++----
 2 files changed, 17 insertions(+), 5 deletions(-)

-- 
2.45.1
Re: [PATCH 0/2] network: fix network driver to gracefully skip startup
Posted by Andrea Bolognani 2 months, 3 weeks ago
On Tue, Jun 11, 2024 at 05:47:56PM GMT, Daniel P. Berrangé wrote:
> We should gracefully skip startup when:
>
>  * No network.conf firewall_backend is explicitly set, and
>    neither iptables/nftables are present
>  * Running unprivileged
>
> The former fixes libvirtd startup on non-Linux, or minimal linux
> installs without firewall tools.
>
> The latter skips pointless initialization that creates a driver
> that cannot do anything useful
>
> Daniel P. Berrangé (2):
>   network: skip network driver init if no firewall backend is present
>   network: don't attempt to initialize if non-privileged
>
>  src/network/bridge_driver.c      | 14 +++++++++++++-
>  src/network/bridge_driver_conf.c |  8 ++++----
>  2 files changed, 17 insertions(+), 5 deletions(-)

This makes things better, in that libvirtd doesn't completely fail to
start. However, we're still not handling the scenario quite
gracefully:

  $ PATH=/usr/bin virsh -c qemu:///session net-list
  error: Disconnected from qemu:///session due to end of file
  error: Failed to list networks
  error: End of file while reading data: Input/output error

libvirtd doesn't stick around after this, and existing sessions (if
any) get forcefully disconnected.

I think that having a proper null backend would allow us to handle
this better, by reporting driver initialization as successful and
only failing actual API calls.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 0/2] network: fix network driver to gracefully skip startup
Posted by Daniel P. Berrangé 2 months, 3 weeks ago
On Thu, Jun 13, 2024 at 04:57:14AM -0400, Andrea Bolognani wrote:
> On Tue, Jun 11, 2024 at 05:47:56PM GMT, Daniel P. Berrangé wrote:
> > We should gracefully skip startup when:
> >
> >  * No network.conf firewall_backend is explicitly set, and
> >    neither iptables/nftables are present
> >  * Running unprivileged
> >
> > The former fixes libvirtd startup on non-Linux, or minimal linux
> > installs without firewall tools.
> >
> > The latter skips pointless initialization that creates a driver
> > that cannot do anything useful
> >
> > Daniel P. Berrangé (2):
> >   network: skip network driver init if no firewall backend is present
> >   network: don't attempt to initialize if non-privileged
> >
> >  src/network/bridge_driver.c      | 14 +++++++++++++-
> >  src/network/bridge_driver_conf.c |  8 ++++----
> >  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> This makes things better, in that libvirtd doesn't completely fail to
> start. However, we're still not handling the scenario quite
> gracefully:
> 
>   $ PATH=/usr/bin virsh -c qemu:///session net-list
>   error: Disconnected from qemu:///session due to end of file
>   error: Failed to list networks
>   error: End of file while reading data: Input/output error
> 
> libvirtd doesn't stick around after this, and existing sessions (if
> any) get forcefully disconnected.

Oh, we've got a design flaw here.

The stateInit method impl is returning skipped, so the driver is not
initialized.

The network driver's connectOpen method correctly declines to open
the network driver. So if youuse network:///session you'll get a
clean error.

When using 'qemu://session', however, the "secondary driver" logic
runs and that doesn't trigger the network driver's connectOpen
method, and so we wire up the network driver APIs even though it
is non-functional.


The nwfilter driver also declines to initialize itself when running
unprivileged, however, its stateInit method impl is failing to do
cleanup, so it partially creates the global 'driver' object and then
skips most initialization. When run as a secondary driver, it works
mostly by luck, not design.

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 :|