[libvirt] [PATCH] tests: skip virfirewalltest on non-Linux systems
Currently firewalling is supported on Linux only, so skip the virfirewalltest on other platforms. --- tests/virfirewalltest.c | 58 +++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c index 805fa44..af74d57 100644 --- a/tests/virfirewalltest.c +++ b/tests/virfirewalltest.c @@ -24,23 +24,26 @@ #define __VIR_COMMAND_PRIV_H_ALLOW__ #include "testutils.h" -#include "virbuffer.h" -#include "vircommandpriv.h" -#include "virfirewallpriv.h" -#include "virmock.h" -#include "virdbuspriv.h" -#define VIR_FROM_THIS VIR_FROM_FIREWALL +#if defined(__linux__) -#if WITH_DBUS -# include <dbus/dbus.h> -#endif +# include "virbuffer.h" +# include "vircommandpriv.h" +# include "virfirewallpriv.h" +# include "virmock.h" +# include "virdbuspriv.h" + +# define VIR_FROM_THIS VIR_FROM_FIREWALL + +# if WITH_DBUS +# include <dbus/dbus.h> +# endif static bool fwDisabled = true; static virBufferPtr fwBuf; static bool fwError; -#define TEST_FILTER_TABLE_LIST \ +# define TEST_FILTER_TABLE_LIST \ "Chain INPUT (policy ACCEPT)\n" \ "target prot opt source destination\n" \ "\n" \ @@ -50,7 +53,7 @@ static bool fwError; "Chain OUTPUT (policy ACCEPT)\n" \ "target prot opt source destination\n" -#define TEST_NAT_TABLE_LIST \ +# define TEST_NAT_TABLE_LIST \ "Chain PREROUTING (policy ACCEPT)\n" \ "target prot opt source destination\n" \ "\n" \ @@ -63,7 +66,7 @@ static bool fwError; "Chain POSTROUTING (policy ACCEPT)\n" \ "target prot opt source destination\n" -#if WITH_DBUS +# if WITH_DBUS VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block, DBusMessage *, DBusConnection *, connection, @@ -186,7 +189,7 @@ VIR_MOCK_IMPL_RET_ARGS(dbus_connection_send_with_reply_and_block, goto cleanup; } -#endif +# endif struct testFirewallData { virFirewallBackend tryBackend; @@ -1126,7 +1129,7 @@ mymain(void) { int ret = 0; -#define RUN_TEST_DIRECT(name, method) \ +# define RUN_TEST_DIRECT(name, method) \ do { \ struct testFirewallData data; \ data.tryBackend = VIR_FIREWALL_BACKEND_AUTOMATIC; \ @@ -1141,8 +1144,8 @@ mymain(void) ret = -1; \ } while (0) -#if WITH_DBUS -# define RUN_TEST_FIREWALLD(name, method) \ +# if WITH_DBUS +# define RUN_TEST_FIREWALLD(name, method) \ do { \ struct testFirewallData data; \ data.tryBackend = VIR_FIREWALL_BACKEND_AUTOMATIC; \ @@ -1157,13 +1160,13 @@ mymain(void) ret = -1; \ } while (0) -# define RUN_TEST(name, method) \ +# define RUN_TEST(name, method) \ RUN_TEST_DIRECT(name, method); \ RUN_TEST_FIREWALLD(name, method) -#else /* ! WITH_DBUS */ -# define RUN_TEST(name, method) \ +# else /* ! WITH_DBUS */ +# define RUN_TEST(name, method) \ RUN_TEST_DIRECT(name, method) -#endif /* ! WITH_DBUS */ +# endif /* ! WITH_DBUS */ RUN_TEST("single group", testFirewallSingleGroup); RUN_TEST("remove rule", testFirewallRemoveRule); @@ -1179,8 +1182,17 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -#if WITH_DBUS +# if WITH_DBUS VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmockdbus.so") -#else +# else VIRT_TEST_MAIN(mymain) -#endif +# endif + +#else /* ! defined (__linux__) */ + +int main(void) +{ + return EXIT_AM_SKIP; +} + +#endif /* ! defined(__linux__) */ -- 1.9.0
On Thu, May 01, 2014 at 10:51:02PM +0400, Roman Bogorodskiy wrote:
Currently firewalling is supported on Linux only, so skip the virfirewalltest on other platforms. --- tests/virfirewalltest.c | 58 +++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 23 deletions(-)
This code is actually intended to be portable across all platforms. What actual problem are you seeing. 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 :|
Daniel P. Berrange wrote:
On Thu, May 01, 2014 at 10:51:02PM +0400, Roman Bogorodskiy wrote:
Currently firewalling is supported on Linux only, so skip the virfirewalltest on other platforms. --- tests/virfirewalltest.c | 58 +++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 23 deletions(-)
This code is actually intended to be portable across all platforms.
What actual problem are you seeing.
I get: TEST: virfirewalltest 1) single group auto direct ... libvirt: Firewall error : direct firewall backend requested, but /sbin/iptables is not available: No such file or directory FAILED All the other tests fail with the same error. Roman Bogorodskiy
On 05/02/14 11:37, Daniel P. Berrange wrote:
On Thu, May 01, 2014 at 10:51:02PM +0400, Roman Bogorodskiy wrote:
Currently firewalling is supported on Linux only, so skip the virfirewalltest on other platforms. --- tests/virfirewalltest.c | 58 +++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 23 deletions(-)
This code is actually intended to be portable across all platforms.
What actual problem are you seeing.
There were build failures with the new test, but according to our build systems they should be fixed now by a few patches from Pavel and Jan so there shouldn't be a need to skip the test now. Roman, could you please re-try running it with the current HEAD? (last patch went in just recently)
Regards, Daniel
Peter
Peter Krempa wrote:
On 05/02/14 11:37, Daniel P. Berrange wrote:
On Thu, May 01, 2014 at 10:51:02PM +0400, Roman Bogorodskiy wrote:
Currently firewalling is supported on Linux only, so skip the virfirewalltest on other platforms. --- tests/virfirewalltest.c | 58 +++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 23 deletions(-)
This code is actually intended to be portable across all platforms.
What actual problem are you seeing.
There were build failures with the new test, but according to our build systems they should be fixed now by a few patches from Pavel and Jan so there shouldn't be a need to skip the test now.
Roman, could you please re-try running it with the current HEAD? (last patch went in just recently)
The test still fails on current HEAD. Roman Bogorodskiy
On 05/02/2014 11:37 AM, Daniel P. Berrange wrote:
On Thu, May 01, 2014 at 10:51:02PM +0400, Roman Bogorodskiy wrote:
Currently firewalling is supported on Linux only, so skip the virfirewalltest on other platforms. --- tests/virfirewalltest.c | 58 +++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 23 deletions(-)
This code is actually intended to be portable across all platforms.
What actual problem are you seeing.
virFirewallValidateBackend checks for the presence of iptables and ebtables. This also breaks starting networks without ebtables installed, even if mac_filter is turned off. Would it be OK to remove this check and only fail when the commands are actually executed? (The test works for me on FreeBSD with the check removed) Jan
On Fri, May 02, 2014 at 11:52:28AM +0200, Ján Tomko wrote:
On 05/02/2014 11:37 AM, Daniel P. Berrange wrote:
On Thu, May 01, 2014 at 10:51:02PM +0400, Roman Bogorodskiy wrote:
Currently firewalling is supported on Linux only, so skip the virfirewalltest on other platforms. --- tests/virfirewalltest.c | 58 +++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 23 deletions(-)
This code is actually intended to be portable across all platforms.
What actual problem are you seeing.
virFirewallValidateBackend checks for the presence of iptables and ebtables. This also breaks starting networks without ebtables installed, even if mac_filter is turned off.
Would it be OK to remove this check and only fail when the commands are actually executed?
OH, I forgot about that check. It is actually intentional to check it upfront because it gives us better error reporting to have it done when the virFirewall instance is created Just disabling the test probably is the right answer until we add a backend that checks for BSD tools. 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 :|
On 01.05.2014 20:51, Roman Bogorodskiy wrote:
Currently firewalling is supported on Linux only, so skip the virfirewalltest on other platforms. --- tests/virfirewalltest.c | 58 +++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 23 deletions(-)
So from the discussion I feel like we have a consensus here, but nothing formal. So ACK. Michal
Michal Privoznik wrote:
On 01.05.2014 20:51, Roman Bogorodskiy wrote:
Currently firewalling is supported on Linux only, so skip the virfirewalltest on other platforms. --- tests/virfirewalltest.c | 58 +++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 23 deletions(-)
So from the discussion I feel like we have a consensus here, but nothing formal. So ACK.
Pushed, thanks! Roman Bogorodskiy
participants (5)
-
Daniel P. Berrange -
Ján Tomko -
Michal Privoznik -
Peter Krempa -
Roman Bogorodskiy