[libvirt] [PATCH 0/4] tests: Fix syntax-check failure, plus extras
Commit a03cbfe0fb introduced a syntax-check failure: patch 1/4 prepares for 2/4, where the failure is actually fixed. Patches 3/4 and 4/4 improve the naming consistency for mock libraries by shuffling files around a bit. Cheers. Andrea Bolognani (4): tests: Split off the mock part of the port allocator test tests: Allow use of close() in mock libraries tests: Don't use "lib" prefix for mock libraries tests: Rename virmockdbus -> virdbusmock for consistency cfg.mk | 2 +- tests/Makefile.am | 26 ++++---- tests/{virmockdbus.c => virdbusmock.c} | 2 +- tests/virfirewalltest.c | 2 +- tests/virpolkittest.c | 2 +- tests/virportallocatormock.c | 108 +++++++++++++++++++++++++++++++ tests/virportallocatortest.c | 112 +++------------------------------ tests/virsystemdtest.c | 2 +- 8 files changed, 134 insertions(+), 122 deletions(-) rename tests/{virmockdbus.c => virdbusmock.c} (97%) create mode 100644 tests/virportallocatormock.c -- 2.5.0
Instead of compiling either the mock or the non-mock part of the file based on a compiler flag, split the mock part off to its own file. --- tests/Makefile.am | 4 +- tests/virportallocatormock.c | 108 ++++++++++++++++++++++++++++++++++++++++++ tests/virportallocatortest.c | 110 +++---------------------------------------- 3 files changed, 117 insertions(+), 105 deletions(-) create mode 100644 tests/virportallocatormock.c diff --git a/tests/Makefile.am b/tests/Makefile.am index 5f5a561..1fe7d6b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1028,8 +1028,8 @@ virportallocatortest_SOURCES = \ virportallocatortest_LDADD = $(LDADDS) libvirportallocatormock_la_SOURCES = \ - virportallocatortest.c -libvirportallocatormock_la_CFLAGS = $(AM_CFLAGS) -DMOCK_HELPER=1 + virportallocatormock.c +libvirportallocatormock_la_CFLAGS = $(AM_CFLAGS) libvirportallocatormock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) libvirportallocatormock_la_LIBADD = $(MOCKLIBS_LIBS) diff --git a/tests/virportallocatormock.c b/tests/virportallocatormock.c new file mode 100644 index 0000000..e44191e --- /dev/null +++ b/tests/virportallocatormock.c @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2013-2014 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> +#include <stdlib.h> + +#if HAVE_DLFCN_H +# include <dlfcn.h> +#endif + +#if defined(RTLD_NEXT) +# include "internal.h" +# include <sys/socket.h> +# include <errno.h> +# include <arpa/inet.h> +# include <netinet/in.h> +# include <stdio.h> +# include <unistd.h> + +static bool host_has_ipv6; +static int (*realsocket)(int domain, int type, int protocol); + +static void init_syms(void) +{ + int fd; + + if (realsocket) + return; + + realsocket = dlsym(RTLD_NEXT, "socket"); + + if (!realsocket) { + fprintf(stderr, "Unable to find 'socket' symbol\n"); + abort(); + } + + fd = realsocket(AF_INET6, SOCK_STREAM, 0); + if (fd < 0) + return; + + host_has_ipv6 = true; + close(fd); +} + +int socket(int domain, + int type, + int protocol) +{ + init_syms(); + + if (getenv("LIBVIRT_TEST_IPV4ONLY") && domain == AF_INET6) { + errno = EAFNOSUPPORT; + return -1; + } + + return realsocket(domain, type, protocol); +} + +int bind(int sockfd ATTRIBUTE_UNUSED, + const struct sockaddr *addr, + socklen_t addrlen ATTRIBUTE_UNUSED) +{ + struct sockaddr_in saddr; + + memcpy(&saddr, addr, sizeof(saddr)); + + if (host_has_ipv6 && !getenv("LIBVIRT_TEST_IPV4ONLY")) { + if (saddr.sin_port == htons(5900) || + (saddr.sin_family == AF_INET && + saddr.sin_port == htons(5904)) || + (saddr.sin_family == AF_INET6 && + (saddr.sin_port == htons(5905) || + saddr.sin_port == htons(5906)))) { + errno = EADDRINUSE; + return -1; + } + return 0; + } + + if (saddr.sin_port == htons(5900) || + saddr.sin_port == htons(5904) || + saddr.sin_port == htons(5905) || + saddr.sin_port == htons(5906)) { + errno = EADDRINUSE; + return -1; + } + + return 0; +} + +#endif /* ! defined(RTLD_NEXT) */ diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c index 077aad8..8a8f413 100644 --- a/tests/virportallocatortest.c +++ b/tests/virportallocatortest.c @@ -22,101 +22,14 @@ #include <stdlib.h> #include "virfile.h" #include "testutils.h" +#include "virutil.h" +#include "virerror.h" +#include "viralloc.h" +#include "virlog.h" +#include "virportallocator.h" +#include "virstring.h" -#if HAVE_DLFCN_H -# include <dlfcn.h> -#endif - -#if defined(RTLD_NEXT) -# ifdef MOCK_HELPER -# include "internal.h" -# include <sys/socket.h> -# include <errno.h> -# include <arpa/inet.h> -# include <netinet/in.h> -# include <stdio.h> - -static bool host_has_ipv6; -static int (*realsocket)(int domain, int type, int protocol); - -static void init_syms(void) -{ - int fd; - - if (realsocket) - return; - - realsocket = dlsym(RTLD_NEXT, "socket"); - - if (!realsocket) { - fprintf(stderr, "Unable to find 'socket' symbol\n"); - abort(); - } - - fd = realsocket(AF_INET6, SOCK_STREAM, 0); - if (fd < 0) - return; - - host_has_ipv6 = true; - close(fd); -} - -int socket(int domain, - int type, - int protocol) -{ - init_syms(); - - if (getenv("LIBVIRT_TEST_IPV4ONLY") && domain == AF_INET6) { - errno = EAFNOSUPPORT; - return -1; - } - - return realsocket(domain, type, protocol); -} - -int bind(int sockfd ATTRIBUTE_UNUSED, - const struct sockaddr *addr, - socklen_t addrlen ATTRIBUTE_UNUSED) -{ - struct sockaddr_in saddr; - - memcpy(&saddr, addr, sizeof(saddr)); - - if (host_has_ipv6 && !getenv("LIBVIRT_TEST_IPV4ONLY")) { - if (saddr.sin_port == htons(5900) || - (saddr.sin_family == AF_INET && - saddr.sin_port == htons(5904)) || - (saddr.sin_family == AF_INET6 && - (saddr.sin_port == htons(5905) || - saddr.sin_port == htons(5906)))) { - errno = EADDRINUSE; - return -1; - } - return 0; - } - - if (saddr.sin_port == htons(5900) || - saddr.sin_port == htons(5904) || - saddr.sin_port == htons(5905) || - saddr.sin_port == htons(5906)) { - errno = EADDRINUSE; - return -1; - } - - return 0; -} - -# else - -# include "virutil.h" -# include "virerror.h" -# include "viralloc.h" -# include "virlog.h" -# include "virportallocator.h" -# include "virstring.h" - -# define VIR_FROM_THIS VIR_FROM_RPC +#define VIR_FROM_THIS VIR_FROM_RPC VIR_LOG_INIT("tests.portallocatortest"); @@ -255,12 +168,3 @@ mymain(void) } VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libvirportallocatormock.so") -# endif - -#else /* ! defined(RTLD_NEXT) */ -int -main(void) -{ - return EXIT_AM_SKIP; -} -#endif -- 2.5.0
On Thu, Feb 11, 2016 at 02:36:28PM +0100, Andrea Bolognani wrote:
Instead of compiling either the mock or the non-mock part of the file based on a compiler flag, split the mock part off to its own file. --- tests/Makefile.am | 4 +- tests/virportallocatormock.c | 108 ++++++++++++++++++++++++++++++++++++++++++ tests/virportallocatortest.c | 110 +++---------------------------------------- 3 files changed, 117 insertions(+), 105 deletions(-) create mode 100644 tests/virportallocatormock.c
@@ -255,12 +168,3 @@ mymain(void) }
VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libvirportallocatormock.so") -# endif - -#else /* ! defined(RTLD_NEXT) */ -int -main(void) -{ - return EXIT_AM_SKIP; -} -#endif
I'm afraid removing this will break the test on non-Linux. ACK if you leave the EXIT_AM_SKIP in. Jan
On Thu, 2016-02-11 at 15:45 +0100, Ján Tomko wrote:
On Thu, Feb 11, 2016 at 02:36:28PM +0100, Andrea Bolognani wrote:
Instead of compiling either the mock or the non-mock part of the file based on a compiler flag, split the mock part off to its own file. --- tests/Makefile.am | 4 +- tests/virportallocatormock.c | 108 ++++++++++++++++++++++++++++++++++++++++++ tests/virportallocatortest.c | 110 +++---------------------------------------- 3 files changed, 117 insertions(+), 105 deletions(-) create mode 100644 tests/virportallocatormock.c @@ -255,12 +168,3 @@ mymain(void) } VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libvirportallocatormock.so") -# endif - -#else /* ! defined(RTLD_NEXT) */ -int -main(void) -{ - return EXIT_AM_SKIP; -} -#endif I'm afraid removing this will break the test on non-Linux. ACK if you leave the EXIT_AM_SKIP in.
I'm reasonably confident it wouldn't make any difference, because we don't check for the availability of that symbol anywhere else in the code, not even in the dozen or so test cases that actually use dynamic symbol loading. We even include <dlfcn.h> without checking whether HAVE_DLFCN_H is defined in some cases. That said, better safe than sorry, so I've restored the check and pushed the whole series. Thanks for your review :) -- Andrea Bolognani Software Engineer - Virtualization Team
As mock libraries are not to be linked against libvirt, the sc_prohibit_close syntax-check rule does not apply. This fixes a syntax-check failure introduced by commit a03cbfe0fb. --- cfg.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfg.mk b/cfg.mk index 3f78842..5b864af 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1158,7 +1158,7 @@ exclude_file_name_regexp--sc_prohibit_strdup = \ ^(docs/|examples/|src/util/virstring\.c|tests/vir(netserverclient|cgroup)mock.c$$) exclude_file_name_regexp--sc_prohibit_close = \ - (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir(cgroup|pci)mock\.c)$$) + (\.p[yl]$$|\.spec\.in$$|^docs/|^(src/util/virfile\.c|src/libvirt-stream\.c|tests/vir.+mock\.c)$$) exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ (^tests/(qemuhelp|nodeinfo|virpcitest)data/|\.diff$$) -- 2.5.0
virportallocatormock was the only one using it, and has been changed accordingly. --- tests/Makefile.am | 10 +++++----- tests/virportallocatortest.c | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 1fe7d6b..359610f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -415,7 +415,7 @@ endif ! WITH_SECDRIVER_APPARMOR EXTRA_DIST += $(test_scripts) test_libraries = libshunload.la \ - libvirportallocatormock.la \ + virportallocatormock.la \ virnetserverclientmock.la \ vircgroupmock.la \ virpcimock.la \ @@ -1027,11 +1027,11 @@ virportallocatortest_SOURCES = \ virportallocatortest.c testutils.h testutils.c virportallocatortest_LDADD = $(LDADDS) -libvirportallocatormock_la_SOURCES = \ +virportallocatormock_la_SOURCES = \ virportallocatormock.c -libvirportallocatormock_la_CFLAGS = $(AM_CFLAGS) -libvirportallocatormock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) -libvirportallocatormock_la_LIBADD = $(MOCKLIBS_LIBS) +virportallocatormock_la_CFLAGS = $(AM_CFLAGS) +virportallocatormock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +virportallocatormock_la_LIBADD = $(MOCKLIBS_LIBS) vircgrouptest_SOURCES = \ vircgrouptest.c testutils.h testutils.c diff --git a/tests/virportallocatortest.c b/tests/virportallocatortest.c index 8a8f413..6f8ec65 100644 --- a/tests/virportallocatortest.c +++ b/tests/virportallocatortest.c @@ -167,4 +167,4 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libvirportallocatormock.so") +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virportallocatormock.so") -- 2.5.0
All mock libraries were called vir*mock except for this one; now the naming is consistent across the board. --- tests/Makefile.am | 14 +++++++------- tests/{virmockdbus.c => virdbusmock.c} | 2 +- tests/virfirewalltest.c | 2 +- tests/virpolkittest.c | 2 +- tests/virsystemdtest.c | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) rename tests/{virmockdbus.c => virdbusmock.c} (97%) diff --git a/tests/Makefile.am b/tests/Makefile.am index 359610f..90981dc 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -435,7 +435,7 @@ endif WITH_BHYVE if WITH_DBUS test_libraries += \ - virmockdbus.la + virdbusmock.la endif WITH_DBUS if WITH_LINUX @@ -1114,11 +1114,11 @@ virdbustest_SOURCES = \ virdbustest_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS) virdbustest_LDADD = $(LDADDS) $(DBUS_LIBS) -virmockdbus_la_SOURCES = \ - virmockdbus.c -virmockdbus_la_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS) -virmockdbus_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) -virmockdbus_la_LIBADD = $(MOCKLIBS_LIBS) +virdbusmock_la_SOURCES = \ + virdbusmock.c +virdbusmock_la_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS) +virdbusmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +virdbusmock_la_LIBADD = $(MOCKLIBS_LIBS) virpolkittest_SOURCES = \ virpolkittest.c testutils.h testutils.c @@ -1131,7 +1131,7 @@ virsystemdtest_CFLAGS = $(AM_CFLAGS) $(DBUS_CFLAGS) virsystemdtest_LDADD = $(LDADDS) $(DBUS_LIBS) else ! WITH_DBUS -EXTRA_DIST += virdbustest.c virmockdbus.c virsystemdtest.c +EXTRA_DIST += virdbustest.c virdbusmock.c virsystemdtest.c endif ! WITH_DBUS viruritest_SOURCES = \ diff --git a/tests/virmockdbus.c b/tests/virdbusmock.c similarity index 97% rename from tests/virmockdbus.c rename to tests/virdbusmock.c index 4261e6a..a62689e 100644 --- a/tests/virmockdbus.c +++ b/tests/virdbusmock.c @@ -1,5 +1,5 @@ /* - * virmockdbus.c: mocking of dbus message send/reply + * virdbusmock.c: mocking of dbus message send/reply * * Copyright (C) 2013-2014 Red Hat, Inc. * diff --git a/tests/virfirewalltest.c b/tests/virfirewalltest.c index 8f6fc9e..f1f29c6 100644 --- a/tests/virfirewalltest.c +++ b/tests/virfirewalltest.c @@ -1184,7 +1184,7 @@ mymain(void) } # if WITH_DBUS -VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmockdbus.so") +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virdbusmock.so") # else VIRT_TEST_MAIN(mymain) # endif diff --git a/tests/virpolkittest.c b/tests/virpolkittest.c index b39beed..1ef7635 100644 --- a/tests/virpolkittest.c +++ b/tests/virpolkittest.c @@ -349,7 +349,7 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmockdbus.so") +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virdbusmock.so") #else /* ! (WITH_DBUS && __linux__) */ int diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c index 101f5e0..5e72de4 100644 --- a/tests/virsystemdtest.c +++ b/tests/virsystemdtest.c @@ -602,7 +602,7 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmockdbus.so") +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virdbusmock.so") #else /* ! (WITH_DBUS && __linux__) */ int -- 2.5.0
On Thu, Feb 11, 2016 at 02:36:27PM +0100, Andrea Bolognani wrote:
Commit a03cbfe0fb introduced a syntax-check failure: patch 1/4 prepares for 2/4, where the failure is actually fixed.
Patches 3/4 and 4/4 improve the naming consistency for mock libraries by shuffling files around a bit.
Cheers.
Andrea Bolognani (4): tests: Split off the mock part of the port allocator test tests: Allow use of close() in mock libraries tests: Don't use "lib" prefix for mock libraries tests: Rename virmockdbus -> virdbusmock for consistency
cfg.mk | 2 +- tests/Makefile.am | 26 ++++---- tests/{virmockdbus.c => virdbusmock.c} | 2 +- tests/virfirewalltest.c | 2 +- tests/virpolkittest.c | 2 +- tests/virportallocatormock.c | 108 +++++++++++++++++++++++++++++++ tests/virportallocatortest.c | 112 +++------------------------------ tests/virsystemdtest.c | 2 +- 8 files changed, 134 insertions(+), 122 deletions(-) rename tests/{virmockdbus.c => virdbusmock.c} (97%) create mode 100644 tests/virportallocatormock.c
ACK to patches 2-4. Jan
participants (2)
-
Andrea Bolognani -
Ján Tomko