Re: [PATCH 1/2] network: skip network driver init if no firewall backend is present

[Laine Stump <laine@xxxxxxxxxx>]

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@xxxxxxxxxx>

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