[libvirt] [PATCH v2] test: fix nwfilter tests following changes in virfirewall.c
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@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
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
On 11/26/2014 11:53 AM, Eric Blake 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
On 11/26/2014 09:26 AM, Stefan Berger wrote: 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
On Wed, Nov 26, 2014 at 01:18:53PM -0500, Stefan Berger wrote:
On 11/26/2014 11:53 AM, Eric Blake 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
On 11/26/2014 09:26 AM, Stefan Berger wrote: 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
On 11/26/2014 11:53 AM, Eric Blake 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
On 11/26/2014 09:26 AM, Stefan Berger wrote: the expected test output to expect the -w/--concurrent.
Either this or the other patch should have made it into v1.2.11...
On Sun, Dec 21, 2014 at 10:52:39AM -0500, Stefan Berger wrote:
On 11/26/2014 11:53 AM, Eric Blake 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
On 11/26/2014 09:26 AM, Stefan Berger wrote: 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
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:
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
On 11/26/2014 09:26 AM, Stefan Berger wrote: 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
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:
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
On 11/26/2014 09:26 AM, Stefan Berger wrote: 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
participants (3)
-
Eric Blake -
Martin Kletzander -
Stefan Berger