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

On 11/11/2014 01:42 PM, 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(a)ubuntu.com&gt; > 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(-) > > diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c > index bab1634..c83fdc6 100644 > --- a/src/util/virfirewall.c > +++ b/src/util/virfirewall.c > @@ -104,6 +104,44 @@ virFirewallOnceInit(void) > > VIR_ONCE_GLOBAL_INIT(virFirewall) > > +static bool iptablesUseLock; > +static bool ip6tablesUseLock; > +static bool ebtablesUseLock; > + > +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]); > + } else { > + VIR_INFO("using locking for %s", args[0]); > + *lockflag = true; > + } > + virCommandFree(cmd); > +} > + > +static void > +virFirewallCheckUpdateLocking(void) > +{ > + const char *iptablesArgs[] = { > + IPTABLES_PATH, "-w", "-L", "-n", NULL, > + }; > + const char *ip6tablesArgs[] = { > + IP6TABLES_PATH, "-w", "-L", "-n", NULL, > + }; > + const char *ebtablesArgs[] = { > + EBTABLES_PATH, "--concurrent", "-L", NULL, > + }; > + virFirewallCheckUpdateLock(&iptablesUseLock, > + iptablesArgs); > + virFirewallCheckUpdateLock(&ip6tablesUseLock, > + ip6tablesArgs); > + virFirewallCheckUpdateLock(&ebtablesUseLock, > + ebtablesArgs); > +} > + > static int > virFirewallValidateBackend(virFirewallBackend backend) > { > @@ -161,6 +199,9 @@ virFirewallValidateBackend(virFirewallBackend > backend) > } > > currentBackend = backend; > + > + virFirewallCheckUpdateLocking(); > + > return 0; > } > > @@ -201,6 +242,9 @@ virFirewallPtr virFirewallNew(void) > { > virFirewallPtr firewall; > > + if (virFirewallInitialize() < 0) > + return NULL; > + > if (VIR_ALLOC(firewall) < 0) > return NULL; > > @@ -321,6 +365,23 @@ virFirewallAddRuleFullV(virFirewallPtr firewall, > rule->queryOpaque = opaque; > rule->ignoreErrors = ignoreErrors; > > + switch (rule->layer) { > + case VIR_FIREWALL_LAYER_ETHERNET: > + if (ebtablesUseLock) > + ADD_ARG(rule, "--concurrent"); > + break; > + case VIR_FIREWALL_LAYER_IPV4: > + if (iptablesUseLock) > + ADD_ARG(rule, "-w"); > + break; > + case VIR_FIREWALL_LAYER_IPV6: > + if (ip6tablesUseLock) > + ADD_ARG(rule, "-w"); > + break; > + case VIR_FIREWALL_LAYER_LAST: > + break; > + } > + By adding these parameters dynamically based on the above added support checking logic will the network filter tests still work without any code change?