[libvirt] [PATCH] Re-add use of locking with iptables/ip6tables/ebtables

[Daniel P. Berrange]

On Tue, Nov 18, 2014 at 06:26:54AM -0700, Eric Blake wrote:
> On 11/11/2014 05:42 AM, Daniel P. Berrange wrote:
> > A previous commit introduced use of locking with invocation
> > of iptables in the viriptables.c module
> > 
> >   commit ba95426d6f39aec1da6e069dd7222f7a8c6a5862
> >   Author: Serge Hallyn <serge.hallyn at ubuntu.com>
> >   Date:   Fri Nov 1 12:36:59 2013 -0500
> > 
> >     util: use -w flag when calling iptables
> > 
> > This only ever had effect with the virtual network driver,
> > as it was not wired up into the nwfilter driver. Unfortunately
> > in the firewall refactoring the use of the -w flag was
> > accidentally lost.
> > 
> > This patch introduces it to the virfirewall.c module so that
> > both the virtual network and nwfilter drivers will be using
> > it. It also ensures that the equivalent --concurrent flag
> > to ebtables is used.
> > ---
> >  src/util/virfirewall.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++---
> >  src/util/viriptables.c |  2 --
> >  2 files changed, 63 insertions(+), 6 deletions(-)
> 
> With this patch applied, and testing on Fedora 20, I see the following
> messages at startup of libvirtd:
> 
> 2014-11-18 13:22:49.979+0000: 31329: info : libvirt version: 1.2.11
> 2014-11-18 13:22:49.979+0000: 31329: error : virCommandWait:2532 :
> internal error: Child process (/usr/sbin/iptables -w -L -n) unexpected
> exit status 2: iptables v1.4.19.1: unknown option "-w"
> Try `iptables -h' or 'iptables --help' for more information.
> 
> 2014-11-18 13:22:49.982+0000: 31329: error : virCommandWait:2532 :
> internal error: Child process (/usr/sbin/ip6tables -w -L -n) unexpected
> exit status 2: ip6tables v1.4.19.1: unknown option "-w"
> Try `ip6tables -h' or 'ip6tables --help' for more information.
> 
> Do we need a followup patch to avoid logging failures when probing for
> whether -w works?
> 
> 
> > +
> > +static void
> > +virFirewallCheckUpdateLock(bool *lockflag,
> > +                           const char *const*args)
> > +{
> > +    virCommandPtr cmd = virCommandNewArgs(args);
> > +    if (virCommandRun(cmd, NULL) < 0) {
> > +        VIR_INFO("locking not supported by %s", args[0]);
> 
> Generally, it would be done by passing a non-NULL parameter to
> virCommandRun and checking the status ourselves (since virCommandRun
> with a NULL argument logs all non-zero exit).

Ok, I'll look at doing that. I think that's actually what the original
code did before it got accidentally lost.

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 :|