Re: [PATCH 17/26] Convert nwfilter ebiptablesTearOldRules to virFirewall

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/08/2014 11:38 AM, Daniel P. Berrange wrote:
Convert the nwfilter ebiptablesTearOldRules 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@xxxxxxxxxx>



  static void
@@ -4248,46 +4271,31 @@ ebiptablesTearNewRules(const char *ifname)
  static int
  ebiptablesTearOldRules(const char *ifname)
  {
-    virBuffer buf = VIR_BUFFER_INITIALIZER;
-
-    /* switch to new iptables user defined chains */
-    if (iptables_cmd_path) {
-        NWFILTER_SET_IPTABLES_SHELLVAR(&buf);
-
-        iptablesUnlinkRootChains(&buf, ifname);
-        iptablesRemoveRootChains(&buf, ifname);
-
-        iptablesRenameTmpRootChains(&buf, ifname);
-        ebiptablesExecCLI(&buf, true, NULL);
-    }
-
-    if (ip6tables_cmd_path) {
-        NWFILTER_SET_IP6TABLES_SHELLVAR(&buf);
-
-        iptablesUnlinkRootChains(&buf, ifname);
-        iptablesRemoveRootChains(&buf, ifname);
-
-        iptablesRenameTmpRootChains(&buf, ifname);
-        ebiptablesExecCLI(&buf, true, NULL);
-    }
-
-    if (ebtables_cmd_path) {
-        NWFILTER_SET_EBTABLES_SHELLVAR(&buf);
-
-        ebtablesUnlinkRootChain(&buf, true, ifname);
-        ebtablesUnlinkRootChain(&buf, false, ifname);
+    virFirewallPtr fw = virFirewallNew();
+    int ret = -1;

-        ebtablesRemoveSubChains(&buf, ifname);
+    virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);

-        ebtablesRemoveRootChain(&buf, true, ifname);
-        ebtablesRemoveRootChain(&buf, false, ifname);
+    iptablesUnlinkRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
+    iptablesRemoveRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);
+    iptablesRenameTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV4, ifname);

-        ebtablesRenameTmpSubAndRootChains(&buf, ifname);
+    iptablesUnlinkRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV6, ifname);
+    iptablesRemoveRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV6, ifname);
+    iptablesRenameTmpRootChainsFW(fw, VIR_FIREWALL_LAYER_IPV6, ifname);

-        ebiptablesExecCLI(&buf, true, NULL);
-    }
+    ebtablesUnlinkRootChainFW(fw, true, ifname);
+    ebtablesUnlinkRootChainFW(fw, false, ifname);
+    ebtablesRemoveSubChainsFW(fw, ifname);
+    ebtablesRemoveRootChainFW(fw, true, ifname);
+    ebtablesRemoveRootChainFW(fw, false, ifname);
+    ebtablesRenameTmpSubAndRootChainsFW(fw, ifname);

-    return 0;
+    virMutexLock(&execCLIMutex);
+    ret = virFirewallApply(fw);
+    virMutexUnlock(&execCLIMutex);
+    virFirewallFree(fw);
+    return ret;
  }

Looks like the transformations I have seen in the other patches - really amazing!. I suppose we wouldn't get here if either iptables, ip6tables, or ebtables weren't installed? Besides I see the lock being grabbed here. You shouldn't need this lock anymore if you lock in the virFirewall code, where I guess you have to have a libvirt-internal centralized lock (possibly 3 locks , one for iptables, ip6tables, and ebtables if they don't mutually influence each other -- would need to test this -- or one lock for all of them) in case of direct eb/ip/ip6tables execution and none in case of firewalld, which should do its own locking.

Regards,
Stefan

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]