10:26 a.m.
Some of the nwfilter tests are now failing since --concurrent shows
up in the ebtables command. To avoid this, implement a function
preventing the probing for lock support in the eb/iptables tools
and use it in the tests.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/libvirt_private.syms | 1 +
src/util/virfirewall.c | 9 +++++++++
src/util/virfirewall.h | 2 ++
tests/nwfilterebiptablestest.c | 3 +++
tests/nwfilterxml2firewalltest.c | 2 ++
5 files changed, 17 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2647d36..22d9116 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1362,6 +1362,7 @@ virFirewallRuleAddArgList;
virFirewallRuleAddArgSet;
virFirewallRuleGetArgCount;
virFirewallSetBackend;
+virFirewallSetLockOverride;
virFirewallStartRollback;
virFirewallStartTransaction;
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
index 8496062..b536912 100644
--- a/src/util/virfirewall.c
+++ b/src/util/virfirewall.c
@@ -107,6 +107,13 @@ VIR_ONCE_GLOBAL_INIT(virFirewall)
static bool iptablesUseLock;
static bool ip6tablesUseLock;
static bool ebtablesUseLock;
+static bool lockOverride; /* true to avoid lock probes */
+
+void
+virFirewallSetLockOverride(bool avoid)
+{
+ lockOverride = avoid;
+}
static void
virFirewallCheckUpdateLock(bool *lockflag,
@@ -135,6 +142,8 @@ virFirewallCheckUpdateLocking(void)
const char *ebtablesArgs[] = {
EBTABLES_PATH, "--concurrent", "-L", NULL,
};
+ if (lockOverride)
+ return;
virFirewallCheckUpdateLock(&iptablesUseLock,
iptablesArgs);
virFirewallCheckUpdateLock(&ip6tablesUseLock,
diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h
index 1129219..dbf3975 100644
--- a/src/util/virfirewall.h
+++ b/src/util/virfirewall.h
@@ -106,4 +106,6 @@ void virFirewallStartRollback(virFirewallPtr firewall,
int virFirewallApply(virFirewallPtr firewall);
+void virFirewallSetLockOverride(bool avoid);
+
#endif /* __VIR_FIREWALL_H__ */
diff --git a/tests/nwfilterebiptablestest.c b/tests/nwfilterebiptablestest.c
index e04bc21..e1330ef 100644
--- a/tests/nwfilterebiptablestest.c
+++ b/tests/nwfilterebiptablestest.c
@@ -24,6 +24,7 @@
#include "testutils.h"
#include "nwfilter/nwfilter_ebiptables_driver.h"
#include "virbuffer.h"
+#include "virfirewall.h"
#define __VIR_FIREWALL_PRIV_H_ALLOW__
#include "virfirewallpriv.h"
@@ -522,6 +523,8 @@ mymain(void)
{
int ret = 0;
+ virFirewallSetLockOverride(true);
+
if (virFirewallSetBackend(VIR_FIREWALL_BACKEND_DIRECT) < 0) {
ret = -1;
goto cleanup;
diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c
index 01527f4..167ad42 100644
--- a/tests/nwfilterxml2firewalltest.c
+++ b/tests/nwfilterxml2firewalltest.c
@@ -474,6 +474,8 @@ mymain(void)
ret = -1; \
} while (0)
+ virFirewallSetLockOverride(true);
+
if (virFirewallSetBackend(VIR_FIREWALL_BACKEND_DIRECT) < 0) {
ret = -1;
goto cleanup;
--
1.9.3
10:53 a.m.
On 11/26/2014 09:26 AM, Stefan Berger wrote:
Some of the nwfilter tests are now failing since --concurrent shows
up in the ebtables command. To avoid this, implement a function
preventing the probing for lock support in the eb/iptables tools
and use it in the tests.
Now that I've read Martin and Prerna's exchange, I'm wondering if we
should instead make this override force the locking flags ON, and adjust
the expected test output to expect the -w/--concurrent.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:18 p.m.
On 11/26/2014 11:53 AM, Eric Blake wrote:
On 11/26/2014 09:26 AM, Stefan Berger wrote:
> Some of the nwfilter tests are now failing since --concurrent shows
> up in the ebtables command. To avoid this, implement a function
> preventing the probing for lock support in the eb/iptables tools
> and use it in the tests.
Now that I've read Martin and Prerna's exchange, I'm wondering if we
should instead make this override force the locking flags ON, and adjust
the expected test output to expect the -w/--concurrent.
Not sure... this patch is definitely 'simpler' and that one call we add
to virsocket.h would only to be called by test cases. What we would test
if we considered the --concurrent and -w flag is basically not much,
just that some probing happened.
Regards,
Stefan
2:37 a.m.
On Wed, Nov 26, 2014 at 01:18:53PM -0500, Stefan Berger wrote:
On 11/26/2014 11:53 AM, Eric Blake wrote:
> On 11/26/2014 09:26 AM, Stefan Berger wrote:
>> Some of the nwfilter tests are now failing since --concurrent shows
>> up in the ebtables command. To avoid this, implement a function
>> preventing the probing for lock support in the eb/iptables tools
>> and use it in the tests.
> Now that I've read Martin and Prerna's exchange, I'm wondering if we
> should instead make this override force the locking flags ON, and adjust
> the expected test output to expect the -w/--concurrent.
>
Not sure... this patch is definitely 'simpler' and that one call we add
to virsocket.h would only to be called by test cases. What we would test
if we considered the --concurrent and -w flag is basically not much,
just that some probing happened.
I must say I agree with Stefan (or at least for now). I started
working on the second variant (forcing the flag "on" for tests), and
not only I've found out that the tests themselves (not talking about
the outputs) are not adapted to that, but even though it's still not
fully fixed, I already have a diff of 1051(+), 1014(-).
I can finish it or at least share somewhere if someone else wants to
continue.
Martin
9:52 a.m.
On 11/26/2014 11:53 AM, Eric Blake wrote:
On 11/26/2014 09:26 AM, Stefan Berger wrote:
> Some of the nwfilter tests are now failing since --concurrent shows
> up in the ebtables command. To avoid this, implement a function
> preventing the probing for lock support in the eb/iptables tools
> and use it in the tests.
Now that I've read Martin and Prerna's exchange, I'm wondering if we
should instead make this override force the locking flags ON, and adjust
the expected test output to expect the -w/--concurrent.
Either this or the other patch should have made it into v1.2.11...
3:37 a.m.
On Sun, Dec 21, 2014 at 10:52:39AM -0500, Stefan Berger wrote:
On 11/26/2014 11:53 AM, Eric Blake wrote:
> On 11/26/2014 09:26 AM, Stefan Berger wrote:
>> Some of the nwfilter tests are now failing since --concurrent shows
>> up in the ebtables command. To avoid this, implement a function
>> preventing the probing for lock support in the eb/iptables tools
>> and use it in the tests.
> Now that I've read Martin and Prerna's exchange, I'm wondering if we
> should instead make this override force the locking flags ON, and adjust
> the expected test output to expect the -w/--concurrent.
>
Either this or the other patch should have made it into v1.2.11...
I was under the impression that it did. As I said earlier, changing
tests to always use concurrent is a long run, not a simple "sed -i
..." or something.
And I don't feel like it's of a huge importance as I, for example,
have around 3 other tests failing when building as root.
Martin
9:26 a.m.
On 12/22/2014 04:37 AM, Martin Kletzander wrote:
On Sun, Dec 21, 2014 at 10:52:39AM -0500, Stefan Berger wrote:
> On 11/26/2014 11:53 AM, Eric Blake wrote:
>> On 11/26/2014 09:26 AM, Stefan Berger wrote:
>>> Some of the nwfilter tests are now failing since --concurrent shows
>>> up in the ebtables command. To avoid this, implement a function
>>> preventing the probing for lock support in the eb/iptables tools
>>> and use it in the tests.
>> Now that I've read Martin and Prerna's exchange, I'm wondering if we
>> should instead make this override force the locking flags ON, and
>> adjust
>> the expected test output to expect the -w/--concurrent.
>>
>
> Either this or the other patch should have made it into v1.2.11...
>
I was under the impression that it did. As I said earlier, changing
tests to always use concurrent is a long run, not a simple "sed -i
..." or something.
And I don't feel like it's of a huge importance as I, for example,
have around 3 other tests failing when building as root.
On Fedora 20 I get 3 skips and 2 failures. The 2 failures are solved by
the posted patch.
Stefan
2:54 p.m.
On Mon, Dec 22, 2014 at 10:26:01AM -0500, Stefan Berger wrote:
On 12/22/2014 04:37 AM, Martin Kletzander wrote:
> On Sun, Dec 21, 2014 at 10:52:39AM -0500, Stefan Berger wrote:
>> On 11/26/2014 11:53 AM, Eric Blake wrote:
>>> On 11/26/2014 09:26 AM, Stefan Berger wrote:
>>>> Some of the nwfilter tests are now failing since --concurrent shows
>>>> up in the ebtables command. To avoid this, implement a function
>>>> preventing the probing for lock support in the eb/iptables tools
>>>> and use it in the tests.
>>> Now that I've read Martin and Prerna's exchange, I'm wondering if
we
>>> should instead make this override force the locking flags ON, and
>>> adjust
>>> the expected test output to expect the -w/--concurrent.
>>>
>>
>> Either this or the other patch should have made it into v1.2.11...
>>
>
> I was under the impression that it did. As I said earlier, changing
> tests to always use concurrent is a long run, not a simple "sed -i
> ..." or something.
>
> And I don't feel like it's of a huge importance as I, for example,
> have around 3 other tests failing when building as root.
On Fedora 20 I get 3 skips and 2 failures. The 2 failures are solved by
the posted patch.
ACK for this version if it fixes all problems on F20. I need to check
why it still fails for me though, but that's nothing that should
concern you.
Martin
3623
days inactive
3649
days old
7 comments
3 participants
participants (3)
-
Eric Blake -
Martin Kletzander -
Stefan Berger