util: determine ignoreErrors value when creating virFirewallCmd, not when applying

We know at the time a virFirewallCmd is created (with
virFirewallAddCmd*()) whether or not we will later want to ignore
errors encountered when attempting to apply that command - if
ignoreErrors is set in the AddCmd or if the group has already had
VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS set, then we ignore the errors.

Rather than setting the fwCmd->ignoreErrors only according to the arg
sent to virFirewallAddCmdFull(), and then later (at ApplyCmd-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 fwCmd->ignoreErrors when the
virFirewallCmd is created (thus avoiding the need to look at anything
other than fwCmd->ignoreErrors at the time the command is applied). Once
that is done, we can simply grab ignoreErrors from the object down in
virFirewallApply() rather than cluttering up the argument list on the
entire call chain.

Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Laine Stump 2024-04-19 22:19:42 -04:00
parent 4ee30ecd57
commit 5543179cea

View File

@ -211,14 +211,19 @@ virFirewallAddCmdFullV(virFirewall *firewall,
fwCmd->layer = layer; fwCmd->layer = layer;
fwCmd->queryCB = cb; fwCmd->queryCB = cb;
fwCmd->queryOpaque = opaque; fwCmd->queryOpaque = opaque;
fwCmd->ignoreErrors = ignoreErrors;
while ((str = va_arg(args, char *)) != NULL) while ((str = va_arg(args, char *)) != NULL)
ADD_ARG(fwCmd, str); ADD_ARG(fwCmd, str);
if (group->addingRollback) { if (group->addingRollback) {
fwCmd->ignoreErrors = true; /* always ignore errors when rolling back */
VIR_APPEND_ELEMENT_COPY(group->rollback, group->nrollback, fwCmd); VIR_APPEND_ELEMENT_COPY(group->rollback, group->nrollback, fwCmd);
} else { } 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
*/
fwCmd->ignoreErrors = ignoreErrors || (group->actionFlags & VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
VIR_APPEND_ELEMENT_COPY(group->action, group->naction, fwCmd); VIR_APPEND_ELEMENT_COPY(group->action, group->naction, fwCmd);
} }
@ -466,7 +471,6 @@ virFirewallCmdToString(const char *cmd,
static int static int
virFirewallApplyCmdDirect(virFirewallCmd *fwCmd, virFirewallApplyCmdDirect(virFirewallCmd *fwCmd,
bool ignoreErrors,
char **output) char **output)
{ {
size_t i; size_t i;
@ -511,7 +515,7 @@ virFirewallApplyCmdDirect(virFirewallCmd *fwCmd,
return -1; return -1;
if (status != 0) { if (status != 0) {
if (ignoreErrors) { if (fwCmd->ignoreErrors) {
VIR_DEBUG("Ignoring error running command"); VIR_DEBUG("Ignoring error running command");
} else { } else {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
@ -528,22 +532,18 @@ virFirewallApplyCmdDirect(virFirewallCmd *fwCmd,
static int static int
virFirewallApplyCmd(virFirewall *firewall, virFirewallApplyCmd(virFirewall *firewall,
virFirewallCmd *fwCmd, virFirewallCmd *fwCmd)
bool ignoreErrors)
{ {
g_autofree char *output = NULL; g_autofree char *output = NULL;
g_auto(GStrv) lines = NULL; g_auto(GStrv) lines = NULL;
if (fwCmd->ignoreErrors)
ignoreErrors = fwCmd->ignoreErrors;
if (fwCmd->argsLen == 0) { if (fwCmd->argsLen == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Can't apply empty firewall command")); _("Can't apply empty firewall command"));
return -1; return -1;
} }
if (virFirewallApplyCmdDirect(fwCmd, ignoreErrors, &output) < 0) if (virFirewallApplyCmdDirect(fwCmd, &output) < 0)
return -1; return -1;
if (fwCmd->queryCB && output) { if (fwCmd->queryCB && output) {
@ -570,7 +570,7 @@ virFirewallApplyGroup(virFirewall *firewall,
size_t idx) size_t idx)
{ {
virFirewallGroup *group = firewall->groups[idx]; virFirewallGroup *group = firewall->groups[idx];
bool ignoreErrors = (group->actionFlags & VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
size_t i; size_t i;
VIR_INFO("Starting transaction for firewall=%p group=%p flags=0x%x", VIR_INFO("Starting transaction for firewall=%p group=%p flags=0x%x",
@ -578,9 +578,7 @@ virFirewallApplyGroup(virFirewall *firewall,
firewall->currentGroup = idx; firewall->currentGroup = idx;
group->addingRollback = false; group->addingRollback = false;
for (i = 0; i < group->naction; i++) { for (i = 0; i < group->naction; i++) {
if (virFirewallApplyCmd(firewall, if (virFirewallApplyCmd(firewall, group->action[i]) < 0)
group->action[i],
ignoreErrors) < 0)
return -1; return -1;
} }
return 0; return 0;
@ -598,7 +596,7 @@ virFirewallRollbackGroup(virFirewall *firewall,
firewall->currentGroup = idx; firewall->currentGroup = idx;
group->addingRollback = true; group->addingRollback = true;
for (i = 0; i < group->nrollback; i++) for (i = 0; i < group->nrollback; i++)
ignore_value(virFirewallApplyCmd(firewall, group->rollback[i], true)); ignore_value(virFirewallApplyCmd(firewall, group->rollback[i]));
} }