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@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@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; } }

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