We know at the time a virFirewallRule is created (with virFirewallAddRule*()) whether or not we will later want to ignore errors encountered when attempting to apply that rule - if ignoreErrors is set in the AddRule or if the group has already had VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS set, then we ignore the errors. Rather than setting the rule->ignoreErrors rule only according to the arg sent to virFirewallAddRuleFull(), and then later (at ApplyRule-time) combining that with the group transactionFlags setting (and passing it all the way down the call chain), just combine the two flags right away and store this final value in rule->ignoreErrors when the rule is created (thus avoiding the need to look at anything other than rule->ignoreErrors at the time the rule is applied). And since we now have an API for retrieving the setting of ignoreErrors from a rule, just grab that with the API down in vir*ApplyRule() rather than cluttering up the argument list on the entire call chain. Signed-off-by: Laine Stump <laine@xxxxxxxxxx> --- src/util/virfirewall.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 15c8db3702..e3ba8f7846 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -211,14 +211,20 @@ virFirewallAddRuleFullV(virFirewall *firewall, rule->layer = layer; rule->queryCB = cb; rule->queryOpaque = opaque; - rule->ignoreErrors = ignoreErrors; while ((str = va_arg(args, char *)) != NULL) ADD_ARG(rule, str); if (group->addingRollback) { + rule->ignoreErrors = true; /* always ignore errors when rolling back */ VIR_APPEND_ELEMENT_COPY(group->rollback, group->nrollback, rule); } else { + /* when not rolling back, ignore errors if this group (transaction) + * was started with VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS *or* + * if this specific rule was created with ignoreErrors == true + */ + rule->ignoreErrors = ignoreErrors + || (group->actionFlags & VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); VIR_APPEND_ELEMENT_COPY(group->action, group->naction, rule); } @@ -496,7 +502,6 @@ virFirewallRuleToString(const char *cmd, static int virFirewallApplyRuleDirect(virFirewallRule *rule, - bool ignoreErrors, char **output) { size_t i; @@ -541,7 +546,7 @@ virFirewallApplyRuleDirect(virFirewallRule *rule, return -1; if (status != 0) { - if (ignoreErrors) { + if (virFirewallRuleGetIgnoreErrors(rule)) { VIR_DEBUG("Ignoring error running command"); } else { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -558,16 +563,12 @@ virFirewallApplyRuleDirect(virFirewallRule *rule, static int virFirewallApplyRule(virFirewall *firewall, - virFirewallRule *rule, - bool ignoreErrors) + virFirewallRule *rule) { g_autofree char *output = NULL; g_auto(GStrv) lines = NULL; - if (rule->ignoreErrors) - ignoreErrors = rule->ignoreErrors; - - if (virFirewallApplyRuleDirect(rule, ignoreErrors, &output) < 0) + if (virFirewallApplyRuleDirect(rule, &output) < 0) return -1; if (rule->queryCB && output) { @@ -594,7 +595,7 @@ virFirewallApplyGroup(virFirewall *firewall, size_t idx) { virFirewallGroup *group = firewall->groups[idx]; - bool ignoreErrors = (group->actionFlags & VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); + size_t i; VIR_INFO("Starting transaction for firewall=%p group=%p flags=0x%x", @@ -602,9 +603,7 @@ virFirewallApplyGroup(virFirewall *firewall, firewall->currentGroup = idx; group->addingRollback = false; for (i = 0; i < group->naction; i++) { - if (virFirewallApplyRule(firewall, - group->action[i], - ignoreErrors) < 0) + if (virFirewallApplyRule(firewall, group->action[i]) < 0) return -1; } return 0; @@ -621,11 +620,8 @@ virFirewallRollbackGroup(virFirewall *firewall, VIR_INFO("Starting rollback for group %p", group); firewall->currentGroup = idx; group->addingRollback = true; - for (i = 0; i < group->nrollback; i++) { - ignore_value(virFirewallApplyRule(firewall, - group->rollback[i], - true)); - } + for (i = 0; i < group->nrollback; i++) + ignore_value(virFirewallApplyRule(firewall, group->rollback[i])); } -- 2.39.2