[libvirt] [PATCH 16/26] Convert nwfilter ebiptablesAllTeardown to virFirewall

Stefan Berger stefanb at linux.vnet.ibm.com
Tue Apr 15 17:19:10 UTC 2014


On 04/08/2014 11:38 AM, Daniel P. Berrange wrote:
> Convert the nwfilter ebiptablesAllTeardown method to use the
> virFirewall object APIs instead of creating shell scripts
> using virBuffer APIs. This provides a performance improvement
> through allowing direct use of firewalld dbus APIs and will
> facilitate automated testing.
>
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>

> @@ -2972,6 +3153,58 @@ _ebtablesRemoveSubChains(virBufferPtr buf,
>       virBufferAddLit(buf, "rm_chains $chains\n");
>   }
>
> +
> +static int
> +ebtablesRemoveSubChainsQuery(virFirewallPtr fw,
> +                             const char *const *lines,
> +                             void *opaque)
> +{
> +    size_t i, j;
> +    const char *chains = opaque;
> +

The problem here is 'opaque' may have gone out of scope ... [continue below]

> +    for (i = 0; lines[i] != NULL; i++) {
> +        VIR_DEBUG("Considering '%s'", lines[i]);
> +        char *tmp = strstr(lines[i], "-j ");
> +        if (!tmp)
> +            continue;
> +        tmp = tmp + 3;
> +        for (j = 0; chains[j]; j++) {
> +            if (tmp[0] == chains[j] &&
> +                tmp[1] == '-') {
> +                VIR_DEBUG("Processing chain '%s'", tmp);
> +                virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
> +                                       false, ebtablesRemoveSubChainsQuery,
> +                                       (void *)chains,
> +                                        "-t", "nat", "-L", tmp, NULL);
> +                virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
> +                                   "-t", "nat", "-F", tmp, NULL);
> +                virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
> +                                   "-t", "nat", "-X", tmp, NULL);
> +            }
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static void
> +_ebtablesRemoveSubChainsFW(virFirewallPtr fw,
> +                           const char *ifname,
> +                           const char *chains)
> +{
> +    char rootchain[MAX_CHAINNAME_LENGTH];
> +    size_t i;
> +

[...]

@@ -2986,6 +3219,19 @@ ebtablesRemoveSubChains(virBufferPtr buf,
  }

  static void
+ebtablesRemoveSubChainsFW(virFirewallPtr fw,
+                          const char *ifname)
+{
+    char chains[3] = {
+        CHAINPREFIX_HOST_IN,
+        CHAINPREFIX_HOST_OUT,
+        0
+    };
+


... due to these arrays residing on the stack. So I prepended 'static' 
to these arrays and the ebtables rules cleanup started working.

My suggested modification to this patch would be this here:

Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -139,6 +139,19 @@ static const struct ushort_map l3_protoc
  };


+static char chainprefixes_host[3] = {
+    CHAINPREFIX_HOST_IN,
+    CHAINPREFIX_HOST_OUT,
+    0
+};
+
+static char chainprefixes_host_temp[3] = {
+    CHAINPREFIX_HOST_IN_TEMP,
+    CHAINPREFIX_HOST_OUT_TEMP,
+    0
+};
+
+
  static int
  printVar(virNWFilterVarCombIterPtr vars,
           char *buf, int bufsize,
@@ -2621,7 +2634,7 @@ ebtablesRemoveSubChainsQuery(virFirewall
                               void *opaque)
  {
      size_t i, j;
-    const char *chains = opaque;
+    const char *chainprefixes = opaque;

      for (i = 0; lines[i] != NULL; i++) {
          VIR_DEBUG("Considering '%s'", lines[i]);
@@ -2629,13 +2642,13 @@ ebtablesRemoveSubChainsQuery(virFirewall
          if (!tmp)
              continue;
          tmp = tmp + 3;
-        for (j = 0; chains[j]; j++) {
-            if (tmp[0] == chains[j] &&
+        for (j = 0; chainprefixes[j]; j++) {
+            if (tmp[0] == chainprefixes[j] &&
                  tmp[1] == '-') {
                  VIR_DEBUG("Processing chain '%s'", tmp);
                  virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
                                         false, 
ebtablesRemoveSubChainsQuery,
-                                       (void *)chains,
+                                       (void *)chainprefixes,
                                          "-t", "nat", "-L", tmp, NULL);
                  virFirewallAddRule(fw, VIR_FIREWALL_LAYER_ETHERNET,
                                     "-t", "nat", "-F", tmp, NULL);
@@ -2652,16 +2665,16 @@ ebtablesRemoveSubChainsQuery(virFirewall
  static void
  _ebtablesRemoveSubChainsFW(virFirewallPtr fw,
                             const char *ifname,
-                           const char *chains)
+                           const char *chainprefixes)
  {
      char rootchain[MAX_CHAINNAME_LENGTH];
      size_t i;

-    for (i = 0; chains[i] != 0; i++) {
-        PRINT_ROOT_CHAIN(rootchain, chains[i], ifname);
+    for (i = 0; chainprefixes[i] != 0; i++) {
+        PRINT_ROOT_CHAIN(rootchain, chainprefixes[i], ifname);
          virFirewallAddRuleFull(fw, VIR_FIREWALL_LAYER_ETHERNET,
                                 false, ebtablesRemoveSubChainsQuery,
-                               (void *)chains,
+                               (void *)chainprefixes,
                                 "-t", "nat", "-L", rootchain, NULL);
      }
  }
@@ -2670,13 +2683,7 @@ static void
  ebtablesRemoveSubChainsFW(virFirewallPtr fw,
                            const char *ifname)
  {
-    char chains[3] = {
-        CHAINPREFIX_HOST_IN,
-        CHAINPREFIX_HOST_OUT,
-        0
-    };
-
-    _ebtablesRemoveSubChainsFW(fw, ifname, chains);
+    _ebtablesRemoveSubChainsFW(fw, ifname, chainprefixes_host);
  }


@@ -2684,13 +2691,7 @@ static void
  ebtablesRemoveTmpSubChainsFW(virFirewallPtr fw,
                               const char *ifname)
  {
-    char chains[3] = {
-        CHAINPREFIX_HOST_IN_TEMP,
-        CHAINPREFIX_HOST_OUT_TEMP,
-        0
-    };
-
-    _ebtablesRemoveSubChainsFW(fw, ifname, chains);
+    _ebtablesRemoveSubChainsFW(fw, ifname, chainprefixes_host_temp);
  }

  static void

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140415/33623321/attachment-0001.htm>


More information about the libvir-list mailing list