On Fri, May 17, 2024 at 01:29:49PM -0400, Laine Stump wrote:
> It still can have only one useful value ("iptables"), but once a 2nd
> value is supported, it will be selectable by setting
> "firewall_backend=nftables" in /etc/libvirt/network.conf.
>
> If firewall_backend isn't set in network.conf, then libvirt will check
> to see if FIREWALL_BACKEND_DEFAULT_1 is available and, if so, set
> that. (Since FIREWALL_BACKEND_DEFAULT_1 is currently "iptables", this
> means checking to see it the iptables binary is present on the
> system). If the default backend isn't available, that is considered a
> fatal error (since no networks can be started anyway), so an error is
> logged and startup of the network driver fails.
>
> NB: network.conf is itself created from network.conf.in at build time,
> and the advertised default setting of firewall_backend (in a commented
> out line) is set from the meson_options.txt setting
> "firewall_backend_default_1". This way the conf file will have correct
> information no matter what ordering is chosen for default backend at
> build time (as more backends are added, settings will be added for
> "firewall_backend_default_n", and those will be settable in
> meson_options.txt and on the meson commandline to change the ordering
> of the auto-detection when no backend is set in network.conf).
>
> virNetworkLoadDriverConfig() may look more complicated than necessary,
> but as additional backends are added, it will be easier to add checks
> for those backends (and to re-order the checks based on builders'
> preferences).
>
> Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
> @@ -62,18 +62,75 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED,
> const char *filename)
> {
> g_autoptr(virConf) conf = NULL;
> + g_autofree char *fwBackendStr = NULL;
> + bool fwBackendSelected = false;
> + size_t i;
> + int fwBackends[] = { FIREWALL_BACKEND_DEFAULT_1 };
> + G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == VIR_FIREWALL_BACKEND_LAST);
I'd say we shoule remove this assert and use:
size_t nfwBackends = G_N_ELEMENTS(fwBackends));
> +
> + if (access(filename, R_OK) == 0) {
> +
> + conf = virConfReadFile(filename, 0);
> + if (!conf)
> + return -1;
> +
> + /* use virConfGetValue*(conf, ...) functions to read any settings into cfg */
> +
> + if (virConfGetValueString(conf, "firewall_backend", &fwBackendStr) < 0)
> + return -1;
> +
> + if (fwBackendStr) {
> + fwBackends[0] = virFirewallBackendTypeFromString(fwBackendStr);
> +
> + if (fwBackends[0] < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unrecognized 'firewall_backend = '%1$s' set in network driver config file %2$s"),
> + fwBackendStr, filename);
> + return -1;
> + }
> + VIR_INFO("firewall_backend setting requested from config file %s: '%s'",
> + virFirewallBackendTypeToString(fwBackends[0]), filename);
Here set
nfwBackends = 1;
since when a config param is set, we don't want to ever fallback to
the 2nd entry
> + }
> + }
>
> - /* if file doesn't exist or is unreadable, ignore the "error" */
> - if (access(filename, R_OK) == -1)
> - return 0;
> + for (i = 0; i < G_N_ELEMENTS(fwBackends) && !fwBackendSelected; i++) {
s/G_N_ELEMENTS(...)/nfwBackends/;
>
> - conf = virConfReadFile(filename, 0);
> - if (!conf)
> - return -1;
> + switch ((virFirewallBackend)fwBackends[i]) {
> + case VIR_FIREWALL_BACKEND_IPTABLES: {
> + g_autofree char *iptablesInPath = virFindFileInPath(IPTABLES);
>
> - /* use virConfGetValue*(conf, ...) functions to read any settings into cfg */
> + if (iptablesInPath)
> + fwBackendSelected = true;
> + break;
> + }
> + case VIR_FIREWALL_BACKEND_LAST:
> + virReportEnumRangeError(virFirewallBackend, fwBackends[i]);
> + return -1;
> + }
>
> - return 0;
> + if (fwBackendSelected) {
> +
> + cfg->firewallBackend = fwBackends[i];
> +
> + } else if (fwBackendStr) {
> +
> + /* explicitly requested backend not found - this is a failure */
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("requested firewall_backend '%1$s' is not available"),
> + virFirewallBackendTypeToString(fwBackends[i]));
> + return -1;
> + }
> + }
> +
> + if (fwBackendSelected) {
> + VIR_INFO("using firewall_backend: '%s'",
> + virFirewallBackendTypeToString(cfg->firewallBackend));
> + return 0;
> + } else {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("could not find a usable firewall backend"));
> + return -1;
> + }
> }
>
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 :|
