[libvirt] [PATCH] use -w flag if supported by iptables
Serge Hallyn
serge.hallyn at ubuntu.com
Fri Nov 1 14:51:57 UTC 2013
Quoting Daniel P. Berrange (berrange at redhat.com):
> On Fri, Nov 01, 2013 at 09:17:58AM -0500, Serge Hallyn wrote:
> > Quoting Daniel P. Berrange (berrange at redhat.com):
> > > On Thu, Oct 31, 2013 at 04:36:24PM -0500, Serge Hallyn wrote:
> > > > This will properly lock libvirt's usage of iptables with
> > > > others (like ufw).
> > > >
> > > > (See https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1245322)
> > > >
> > > > Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> > > > ---
> > > > src/util/viriptables.c | 30 ++++++++++++++++++++++++++----
> > > > 1 file changed, 26 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/src/util/viriptables.c b/src/util/viriptables.c
> > > > index 16f571e..30d59b6 100644
> > > > --- a/src/util/viriptables.c
> > > > +++ b/src/util/viriptables.c
> > > > @@ -50,19 +50,25 @@
> > > > #include "virstring.h"
> > > > #include "virutil.h"
> > > >
> > > > +bool iptables_supports_xlock = false;
> > > > +
> > > > #if HAVE_FIREWALLD
> > > > static char *firewall_cmd_path = NULL;
> > > > +#endif
> > > >
> > > > static int
> > > > virIpTablesOnceInit(void)
> > > > {
> > > > + virCommandPtr cmd;
> > > > + int status;
> > > > +
> > > > +#if HAVE_FIREWALLD
> > > > firewall_cmd_path = virFindFileInPath("firewall-cmd");
> > > > if (!firewall_cmd_path) {
> > > > VIR_INFO("firewall-cmd not found on system. "
> > > > "firewalld support disabled for iptables.");
> > > > } else {
> > > > - virCommandPtr cmd = virCommandNew(firewall_cmd_path);
> > > > - int status;
> > > > + virCommandNew(firewall_cmd_path);
> > > >
> > > > virCommandAddArgList(cmd, "--state", NULL);
> > > > if (virCommandRun(cmd, &status) < 0 || status != 0) {
> > > > @@ -74,13 +80,26 @@ virIpTablesOnceInit(void)
> > > > }
> > > > virCommandFree(cmd);
> > > > }
> > > > +
> > > > + if (firewall_cmd_path)
> > > > + return 0;
> > > > +
> > > > +#endif
> > > > +
> > > > + cmd = virCommandNew(IPTABLES_PATH);
> > > > + virCommandAddArgList(cmd, "-w", "-L", "-n", NULL);
> > >
> > > What version of iptables actually has this '-w' flag ? The ubuntu bug
> >
> > It was introduced in 1.4.20, specifically with
> >
> > commit 93587a04d0f2511e108bbc4d87a8b9d28a5c5dd8
> > Author: Phil Oester <kernel at linuxace.com>
> > Date: Fri May 31 09:07:04 2013 -0400
> >
> > ip[6]tables: Add locking to prevent concurrent instances
> >
> > > points to an upstream patch, which does locking unconditionally, no
> > > mention of any '-w' flag.
> >
> > Well yes it locks unconditionally, but without -w it won't wait. So
> > if there is another app using iptables at the time, then the call fails
> > and libvirt fails to create the interface.
> >
> > > Looking on a per-rule basis is pretty lame locking, since changes
> > > to iptables are often comprised of many rule changes :-(
> > >
> > > > + if (virCommandRun(cmd, &status) < 0 || status != 0) {
> > > > + VIR_INFO("xtables locking not supported by your iptables");
> > > > + } else {
> > > > + VIR_INFO("using xtables locking for iptables");
> > > > + iptables_supports_xlock = true;
> > > > + }
> > > > + virCommandFree(cmd);
> > > > return 0;
> > > > }
> > > >
> > > > VIR_ONCE_GLOBAL_INIT(virIpTables)
> > > >
> > > > -#endif
> > > > -
> > >
> > > The call to virIptablesInitialize is protected by
> > > #if HAVE_FIREWALLD too, so this code will not run
> > > on older distros.
> >
> > I thought I changed that, but I may not properly understand the
> > libvirt init code. I moved the VIR_ONCE_GLOBAL_INIT(virIptTables)
> > out from under that ifdef.
>
> That macro just does the declaration of the initializer - this still
> needs to be actually invoked - which is what the (conditional) calls
> to virIptablesInitialize() later in the file do.
Ooooh. I couldn't find it bc of the capitalized T. Thanks. Updated
version below. I also updated the description, as my original one was
wrong as you pointed out
>From 757bb5ad2d2d761e9e703a8905e9a4603e83c142 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn at ubuntu.com>
Date: Thu, 31 Oct 2013 15:22:16 -0500
Subject: [PATCH 1/1] use -w flag if supported by iptables
Ask iptables to wait rather than fail if it is in use by another
caller (like ufw).
(See https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1245322)
Changelog:
v2: take virIpTablesInitialize() out of HAVE_FIREWALLD ifdef.
Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
src/util/viriptables.c | 32 +++++++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/src/util/viriptables.c b/src/util/viriptables.c
index 16f571e..bedd837 100644
--- a/src/util/viriptables.c
+++ b/src/util/viriptables.c
@@ -50,19 +50,25 @@
#include "virstring.h"
#include "virutil.h"
+bool iptables_supports_xlock = false;
+
#if HAVE_FIREWALLD
static char *firewall_cmd_path = NULL;
+#endif
static int
virIpTablesOnceInit(void)
{
+ virCommandPtr cmd;
+ int status;
+
+#if HAVE_FIREWALLD
firewall_cmd_path = virFindFileInPath("firewall-cmd");
if (!firewall_cmd_path) {
VIR_INFO("firewall-cmd not found on system. "
"firewalld support disabled for iptables.");
} else {
- virCommandPtr cmd = virCommandNew(firewall_cmd_path);
- int status;
+ virCommandNew(firewall_cmd_path);
virCommandAddArgList(cmd, "--state", NULL);
if (virCommandRun(cmd, &status) < 0 || status != 0) {
@@ -74,13 +80,26 @@ virIpTablesOnceInit(void)
}
virCommandFree(cmd);
}
+
+ if (firewall_cmd_path)
+ return 0;
+
+#endif
+
+ cmd = virCommandNew(IPTABLES_PATH);
+ virCommandAddArgList(cmd, "-w", "-L", "-n", NULL);
+ if (virCommandRun(cmd, &status) < 0 || status != 0) {
+ VIR_INFO("xtables locking not supported by your iptables");
+ } else {
+ VIR_INFO("using xtables locking for iptables");
+ iptables_supports_xlock = true;
+ }
+ virCommandFree(cmd);
return 0;
}
VIR_ONCE_GLOBAL_INIT(virIpTables)
-#endif
-
#define VIR_FROM_THIS VIR_FROM_NONE
enum {
@@ -92,8 +111,8 @@ static virCommandPtr
iptablesCommandNew(const char *table, const char *chain, int family, int action)
{
virCommandPtr cmd = NULL;
-#if HAVE_FIREWALLD
virIpTablesInitialize();
+#if HAVE_FIREWALLD
if (firewall_cmd_path) {
cmd = virCommandNew(firewall_cmd_path);
virCommandAddArgList(cmd, "--direct", "--passthrough",
@@ -104,6 +123,9 @@ iptablesCommandNew(const char *table, const char *chain, int family, int action)
if (cmd == NULL) {
cmd = virCommandNew((family == AF_INET6)
? IP6TABLES_PATH : IPTABLES_PATH);
+
+ if (iptables_supports_xlock)
+ virCommandAddArgList(cmd, "-w", NULL);
}
virCommandAddArgList(cmd, "--table", table,
--
1.8.3.2
More information about the libvir-list
mailing list