On 5/1/23 05:19, Laine Stump wrote:
> 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);
Nit pick - we usually put logical operands at the end of previous line.
> VIR_APPEND_ELEMENT_COPY(group->action, group->naction, rule);
> }
>
Michal
