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@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?
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.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list