[libvirt] [PATCH 17/26] Convert nwfilter ebiptablesTearOldRules to virFirewall
Stefan Berger
stefanb at linux.vnet.ibm.com
Wed Apr 16 12:08:03 UTC 2014
On 04/16/2014 07:55 AM, Daniel P. Berrange wrote:
> On Wed, Apr 16, 2014 at 07:41:10AM -0400, Stefan Berger wrote:
>> 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 at redhat.com>
>>>
>>
>>> 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?
> The RPM will ensure they are all available, and the virfirewall
> code will complain if they're missing, so IMHO that's sufficient.
>
>> 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.
> These locks are just to protect things during the intermediate
> part-converted stage. They go away at the end of this series
> so we rely on the lock in virfirewall.c, which obsoletes the
> execCLIMutex.
With this explanation: ACK
More information about the libvir-list
mailing list