10:19 p.m.
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(a)redhat.com>
---
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