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