[libvirt PATCH 03/28] util: determine ignoreErrors value when creating rule, not when applying

[Michal Prívozník]

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 at 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);

Nit pick - we usually put logical operands at the end of previous line.

>          VIR_APPEND_ELEMENT_COPY(group->action, group->naction, rule);
>      }
>  

Michal