1:51 p.m.
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
4:37 a.m.
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 :|
4:47 a.m.
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
4:48 a.m.
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
4:52 a.m.
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
4:52 a.m.
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
4:58 a.m.
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 :|
7:12 a.m.
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
7:52 a.m.
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
3857
days inactive
3858
days old
8 comments
5 participants
participants (5)
-
Daniel P. Berrange -
Ján Tomko -
Michal Privoznik -
Peter Krempa -
Roman Bogorodskiy