Re: [PATCH v5 12/30] network: support setting firewallBackend from network.conf
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(a)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"));