On 5/22/24 10:36 AM, Daniel P. Berrangé wrote:
On Mon, May 20, 2024 at 12:14:26PM -0400, Laine Stump wrote: [...]
That is taken care of by [see [*] below], but I'll add this suggestion anyway to reduce the brain cells required for someone reading through the code :-) (and also as a backup in case the code at [*] gets changed in the future).
Yeah, the current code works, but is a bit confusing to me. Meanwhile
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Okay, I'll squash this in before I push (to summarize, I add a variable for nFwBackends, then set it to 1 if a backend is specified in the config file, and finally I move the " else if (fwBackendStr)" clause (the code that's executed if a backend was set in the conf file and it failed) down below the loop (with the rest of the log messages). diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c index a083fa68e6..8f4956dace 100644 --- a/src/network/bridge_driver_conf.c +++ b/src/network/bridge_driver_conf.c @@ -69,6 +69,7 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, size_t i; int fwBackends[] = { FIREWALL_BACKEND_DEFAULT_1, FIREWALL_BACKEND_DEFAULT_2 }; G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == VIR_FIREWALL_BACKEND_LAST); + int nFwBackends = G_N_ELEMENTS(fwBackends); if (access(filename, R_OK) == 0) { @@ -83,6 +84,7 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, if (fwBackendStr) { fwBackends[0] = virFirewallBackendTypeFromString(fwBackendStr); + nFwBackends = 1; if (fwBackends[0] < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -95,7 +97,7 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, } } - for (i = 0; i < G_N_ELEMENTS(fwBackends) && !fwBackendSelected; i++) { + for (i = 0; i < nFwBackends && !fwBackendSelected; i++) { switch ((virFirewallBackend)fwBackends[i]) { case VIR_FIREWALL_BACKEND_IPTABLES: { @@ -119,24 +121,23 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, return -1; } - if (fwBackendSelected) { - + 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 if (fwBackendStr) { + + /* the explicitly requested backend wasn't found - this is a failure */ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("requested firewall_backend '%1$s' is not available"), + fwBackendStr); + return -1; + } else { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("could not find a usable firewall backend"));