11:47 a.m.
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
11:47 a.m.
New subject: [PATCH 1/2] network: skip network driver init if no firewall backend is present
If neither iptables or nftables are present, and no explicit config
setting was made, skip network driver initialization, rather than
making it a hard error.
This allows libvirtd to carry on operating with the network driver
disabled, while ensuring virtnetworkd will shutdown.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/network/bridge_driver.c | 8 +++++++-
src/network/bridge_driver_conf.c | 8 ++++----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 32572c755f..371bc2bae6 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -588,6 +588,7 @@ networkStateInitialize(bool privileged,
#ifdef WITH_FIREWALLD
GDBusConnection *sysbus = NULL;
#endif
+ int ret = VIR_DRV_STATE_INIT_ERROR;
if (root != NULL) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -611,6 +612,11 @@ networkStateInitialize(bool privileged,
if (!(network_driver->config = cfg = virNetworkDriverConfigNew(privileged)))
goto error;
+ if (network_driver->config->firewallBackend == -1) {
+ ret = VIR_DRV_STATE_INIT_SKIPPED;
+ goto error;
+ }
+
if ((network_driver->lockFD =
virPidFileAcquire(cfg->stateDir, "driver", getpid())) < 0)
goto error;
@@ -689,7 +695,7 @@ networkStateInitialize(bool privileged,
error:
networkStateCleanup();
- return VIR_DRV_STATE_INIT_ERROR;
+ return ret;
}
diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c
index e2f3613a41..f6c89ddddf 100644
--- a/src/network/bridge_driver_conf.c
+++ b/src/network/bridge_driver_conf.c
@@ -132,7 +132,7 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED,
if (fwBackendSelected) {
VIR_INFO("using firewall_backend: '%s'",
virFirewallBackendTypeToString(cfg->firewallBackend));
- return 0;
+ return 1;
} else if (fwBackendStr) {
@@ -143,9 +143,9 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED,
return -1;
} else {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("could not find a usable firewall backend"));
- return -1;
+ cfg->firewallBackend = -1;
+ VIR_ERROR(_("could not find a usable firewall backend"));
+ return 0;
}
}
--
2.45.1
7:47 a.m.
New subject: [PATCH 1/2] network: skip network driver init if no firewall
backend is present
On 6/11/24 12:47 PM, Daniel P. Berrangé wrote:
If neither iptables or nftables are present, and no explicit config
setting was made, skip network driver initialization, rather than
making it a hard error.
This allows libvirtd to carry on operating with the network driver
disabled, while ensuring virtnetworkd will shutdown.
My assumption after reading the report about this from Roman was that he
must have been using <forward mode='open'/>, which would never use any
firewall functions and thus would succeed. If that's the case, then this
change would still fail for him.
If, on the other hand, FreeBSD users are only using <interface
type='bridge'/), then this patch will be fine.
If I re-assume to the latter, then:
Reviewed-by: Laine Stump <laine(a)redhat.com>
(and soon to be Tested-by, but first I have some errands to run :-)
but we should make sure they aren't trying to use <forward mode='open'/>
on platforms with no supported firewall.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/network/bridge_driver.c | 8 +++++++-
src/network/bridge_driver_conf.c | 8 ++++----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 32572c755f..371bc2bae6 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -588,6 +588,7 @@ networkStateInitialize(bool privileged,
#ifdef WITH_FIREWALLD
GDBusConnection *sysbus = NULL;
#endif
+ int ret = VIR_DRV_STATE_INIT_ERROR;
if (root != NULL) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
@@ -611,6 +612,11 @@ networkStateInitialize(bool privileged,
if (!(network_driver->config = cfg = virNetworkDriverConfigNew(privileged)))
goto error;
+ if (network_driver->config->firewallBackend == -1) {
+ ret = VIR_DRV_STATE_INIT_SKIPPED;
+ goto error;
+ }
+
if ((network_driver->lockFD =
virPidFileAcquire(cfg->stateDir, "driver", getpid())) < 0)
goto error;
@@ -689,7 +695,7 @@ networkStateInitialize(bool privileged,
error:
networkStateCleanup();
- return VIR_DRV_STATE_INIT_ERROR;
+ return ret;
}
diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c
index e2f3613a41..f6c89ddddf 100644
--- a/src/network/bridge_driver_conf.c
+++ b/src/network/bridge_driver_conf.c
@@ -132,7 +132,7 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg
G_GNUC_UNUSED,
if (fwBackendSelected) {
VIR_INFO("using firewall_backend: '%s'",
virFirewallBackendTypeToString(cfg->firewallBackend));
- return 0;
+ return 1;
} else if (fwBackendStr) {
@@ -143,9 +143,9 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg
G_GNUC_UNUSED,
return -1;
} else {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("could not find a usable firewall backend"));
- return -1;
+ cfg->firewallBackend = -1;
+ VIR_ERROR(_("could not find a usable firewall backend"));
+ return 0;
}
}
11:47 a.m.
New subject: [PATCH 2/2] network: don't attempt to initialize if non-privileged
Running any of the firewall tools is unsupported when non-root. Rather
than attempt to initialize the driver, which will then be unusable,
just skip initialization entirely and decline startup.
This allows libvirtd to carry on operating with the network driver
disabled, while ensuring virtnetworkd will shutdown.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/network/bridge_driver.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 371bc2bae6..ce69c56464 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -596,6 +596,12 @@ networkStateInitialize(bool privileged,
return -1;
}
+ /* Can't manipulate the firewall when non-root */
+ if (!privileged) {
+ ret = VIR_DRV_STATE_INIT_SKIPPED;
+ goto error;
+ }
+
network_driver = g_new0(virNetworkDriverState, 1);
network_driver->lockFD = -1;
--
2.45.1
4:44 p.m.
New subject: [PATCH 2/2] network: don't attempt to initialize if
non-privileged
On 6/11/24 12:47 PM, Daniel P. Berrangé wrote:
Running any of the firewall tools is unsupported when non-root.
Rather
than attempt to initialize the driver, which will then be unusable,
just skip initialization entirely and decline startup.
This allows libvirtd to carry on operating with the network driver
disabled, while ensuring virtnetworkd will shutdown.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/network/bridge_driver.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 371bc2bae6..ce69c56464 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -596,6 +596,12 @@ networkStateInitialize(bool privileged,
return -1;
}
+ /* Can't manipulate the firewall when non-root */
+ if (!privileged) {
+ ret = VIR_DRV_STATE_INIT_SKIPPED;
+ goto error;
+ }
+
Reviewed-by: Laine Stump <laine(a)redhat.com>
About once every 3 or 4 years I've wondered why we load the network
driver for unprivileged libvirt, since it's unusable. I haven't had the
attention span to ask anyone and write this patch though :-)
3:57 a.m.
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
4:56 a.m.
New subject: [PATCH 0/2] network: fix network driver to gracefully skip
startup
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 :|
193
days inactive
195
days old
6 comments
3 participants
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Laine Stump