Re: [PATCH v2 2/4] util: pass layer into firewall query callback

[Laine Stump <laine@xxxxxxxxx>]

On 12/7/18 11:21 AM, Daniel P. Berrangé wrote:
> Some of the query callbacks want to know the firewall layer that was
> being used for triggering the query to avoid duplicating that data.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>


Reviewed-by: Laine Stump <laine@xxxxxxxxx>

> ---
>   src/nwfilter/nwfilter_ebiptables_driver.c | 17 ++++++++++-------
>   src/util/virfirewall.c                    |  2 +-
>   src/util/virfirewall.h                    |  1 +
>   tests/virfirewalltest.c                   |  3 ++-
>   4 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c
> index 5be1c9b07a..a9b40988dd 100644
> --- a/src/nwfilter/nwfilter_ebiptables_driver.c
> +++ b/src/nwfilter/nwfilter_ebiptables_driver.c
> @@ -2703,6 +2703,7 @@ ebtablesCreateTmpSubChainFW(virFirewallPtr fw,
>   
>   static int
>   ebtablesRemoveSubChainsQuery(virFirewallPtr fw,
> +                             virFirewallLayer layer,
>                                const char *const *lines,
>                                void *opaque)
>   {
> @@ -2719,14 +2720,14 @@ ebtablesRemoveSubChainsQuery(virFirewallPtr fw,
>               if (tmp[0] == chainprefixes[j] &&
>                   tmp[1] == '-') {
>                   VIR_DEBUG("Processing chain '%s'", tmp);
> -                virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
> +                virFirewallAddRuleFull(fw, layer,
>                                          false, ebtablesRemoveSubChainsQuery,
>                                          (void *)chainprefixes,
>                                           "-t", "nat", "-L", tmp, NULL);
> -                virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
> +                virFirewallAddRuleFull(fw, layer,
>                                          true, NULL, NULL,
>                                          "-t", "nat", "-F", tmp, NULL);
> -                virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
> +                virFirewallAddRuleFull(fw, layer,
>                                          true, NULL, NULL,
>                                          "-t", "nat", "-X", tmp, NULL);
>               }
> @@ -2804,6 +2805,7 @@ ebtablesRenameTmpRootChainFW(virFirewallPtr fw,
>   
>   static int
>   ebtablesRenameTmpSubAndRootChainsQuery(virFirewallPtr fw,
> +                                       virFirewallLayer layer,
>                                          const char *const *lines,
>                                          void *opaque ATTRIBUTE_UNUSED)
>   {
> @@ -2828,17 +2830,17 @@ ebtablesRenameTmpSubAndRootChainsQuery(virFirewallPtr fw,
>           else
>               newchain[0] = CHAINPREFIX_HOST_OUT;
>           VIR_DEBUG("Renaming chain '%s' to '%s'", tmp, newchain);
> -        virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
> +        virFirewallAddRuleFull(fw, layer,
>                                  false, ebtablesRenameTmpSubAndRootChainsQuery,
>                                  NULL,
>                                  "-t", "nat", "-L", tmp, NULL);
> -        virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
> +        virFirewallAddRuleFull(fw, layer,
>                                  true, NULL, NULL,
>                                  "-t", "nat", "-F", newchain, NULL);
> -        virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
> +        virFirewallAddRuleFull(fw, layer,
>                                  true, NULL, NULL,
>                                  "-t", "nat", "-X", newchain, NULL);
> -        virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
> +        virFirewallAddRule(fw, layer,
>                              "-t", "nat", "-E", tmp, newchain, NULL);
>       }
>   
> @@ -3760,6 +3762,7 @@ ebiptablesDriverProbeCtdir(void)
>   
>   static int
>   ebiptablesDriverProbeStateMatchQuery(virFirewallPtr fw ATTRIBUTE_UNUSED,
> +                                     virFirewallLayer layer ATTRIBUTE_UNUSED,
>                                        const char *const *lines,
>                                        void *opaque)
>   {
> diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
> index c786d7671b..42819cedb0 100644
> --- a/src/util/virfirewall.c
> +++ b/src/util/virfirewall.c
> @@ -828,7 +828,7 @@ virFirewallApplyRule(virFirewallPtr firewall,
>               return -1;
>   
>           VIR_DEBUG("Invoking query %p with '%s'", rule->queryCB, output);
> -        if (rule->queryCB(firewall, (const char *const *)lines, rule->queryOpaque) < 0)
> +        if (rule->queryCB(firewall, rule->layer, (const char *const *)lines, rule->queryOpaque) < 0)
>               return -1;
>   
>           if (firewall->err == ENOMEM) {
> diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h
> index e024e88cc2..15f33223be 100644
> --- a/src/util/virfirewall.h
> +++ b/src/util/virfirewall.h
> @@ -59,6 +59,7 @@ void virFirewallFree(virFirewallPtr firewall);
>            virFirewallAddRuleFull(firewall, layer, false, NULL, NULL, __VA_ARGS__)
>   
>   typedef int (*virFirewallQueryCallback)(virFirewallPtr firewall,
> +                                        virFirewallLayer layer,
>                                           const char *const *lines,
>                                           void *opaque);
>   
> diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c
> index dda736cae5..d0bb824836 100644
> --- a/tests/virfirewalltest.c
> +++ b/tests/virfirewalltest.c
> @@ -992,11 +992,12 @@ testFirewallQueryHook(const char *const*args,
>   
>   static int
>   testFirewallQueryCallback(virFirewallPtr fw,
> +                          virFirewallLayer layer,
>                             const char *const *lines,
>                             void *opaque ATTRIBUTE_UNUSED)
>   {
>       size_t i;
> -    virFirewallAddRule(fw, VIR_FIREWALL_LAYER_IPV4,
> +    virFirewallAddRule(fw, layer,
>                          "-A", "INPUT",
>                          "--source-host", "!192.168.122.129",
>                          "--jump", "REJECT", NULL);


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list