[libvirt] [PATCH v3 7/9] tests: Build virpcimock on non-Linux

Andrea Bolognani posted 9 patches 7 years ago
[libvirt] [PATCH v3 7/9] tests: Build virpcimock on non-Linux
Posted by Andrea Bolognani 7 years ago
There are only a couple issues preventing it from working on
other platform such as FreeBSD. Let's fix them.

With the mocking in place, qemumemlocktest and qemuxml2xmltest
can finally succeed on FreeBSD.

This commit is best viewed with 'git show -w'.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 tests/virpcimock.c | 57 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 24 deletions(-)

diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 7e4ac191d0..09cb26567c 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -20,19 +20,18 @@
 
 #include <config.h>
 
-#ifdef __linux__
-# include "virmock.h"
-# include <stdio.h>
-# include <stdlib.h>
-# include <unistd.h>
-# include <fcntl.h>
-# include <sys/stat.h>
-# include <stdarg.h>
-# include <dirent.h>
-# include "viralloc.h"
-# include "virstring.h"
-# include "virfile.h"
-# include "dirname.h"
+#include "virmock.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <stdarg.h>
+#include <dirent.h>
+#include "viralloc.h"
+#include "virstring.h"
+#include "virfile.h"
+#include "dirname.h"
 
 static int (*real_access)(const char *path, int mode);
 static int (*real_lstat)(const char *path, struct stat *sb);
@@ -51,20 +50,20 @@ static char *(*real_virFileCanonicalizePath)(const char *path);
 char *fakerootdir;
 char *fakesysfspcidir;
 
-# define SYSFS_PCI_PREFIX "/sys/bus/pci/"
+#define SYSFS_PCI_PREFIX "/sys/bus/pci/"
 
-# define STDERR(...) \
+#define STDERR(...) \
     fprintf(stderr, "%s %zu: ", __FUNCTION__, (size_t) __LINE__); \
     fprintf(stderr, __VA_ARGS__); \
     fprintf(stderr, "\n"); \
 
-# define ABORT(...) \
+#define ABORT(...) \
     do { \
         STDERR(__VA_ARGS__); \
         abort(); \
     } while (0)
 
-# define ABORT_OOM() \
+#define ABORT_OOM() \
     ABORT("Out of memory")
 /*
  * The plan:
@@ -341,6 +340,7 @@ pci_device_new_from_stub(const struct pciDevice *data)
     char *configSrc;
     char tmp[256];
     struct stat sb;
+    bool configSrcExists = false;
 
     if (VIR_STRDUP_QUIET(id, data->id) < 0)
         ABORT_OOM();
@@ -368,10 +368,18 @@ pci_device_new_from_stub(const struct pciDevice *data)
     if (virFileMakePath(devpath) < 0)
         ABORT("Unable to create: %s", devpath);
 
+    if (real_stat && real_stat(configSrc, &sb) == 0)
+        configSrcExists = true;
+
+#ifdef HAVE___XSTAT
+    if (!configSrcExists &&
+        real___xstat && real___xstat(_STAT_VER, configSrc, &sb) == 0)
+        configSrcExists = true;
+#endif
+
     /* If there is a config file for the device within virpcitestdata dir,
      * symlink it. Otherwise create a dummy config file. */
-    if ((real_stat && real_stat(configSrc, &sb) == 0) ||
-        (real___xstat && real___xstat(_STAT_VER, configSrc, &sb) == 0)) {
+    if (configSrcExists) {
         /* On success, copy @configSrc into the destination (a copy,
          * rather than a symlink, is required since we write into the
          * file, and parallel VPATH builds must not stomp on the
@@ -834,7 +842,7 @@ init_env(void)
 
     make_file(fakesysfspcidir, "drivers_probe", NULL, -1);
 
-# define MAKE_PCI_DRIVER(name, ...) \
+#define MAKE_PCI_DRIVER(name, ...) \
     pci_driver_new(name, 0, __VA_ARGS__, -1, -1)
 
     MAKE_PCI_DRIVER("iwlwifi", 0x8086, 0x0044);
@@ -842,7 +850,7 @@ init_env(void)
     MAKE_PCI_DRIVER("pci-stub", -1, -1);
     pci_driver_new("vfio-pci", PCI_ACTION_BIND, -1, -1);
 
-# define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \
+#define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \
     do { \
         struct pciDevice dev = {.id = (char *)Id, .vendor = Vendor, \
                                 .device = Device, __VA_ARGS__}; \
@@ -891,6 +899,7 @@ access(const char *path, int mode)
     return ret;
 }
 
+#ifdef HAVE___LXSTAT
 int
 __lxstat(int ver, const char *path, struct stat *sb)
 {
@@ -909,6 +918,7 @@ __lxstat(int ver, const char *path, struct stat *sb)
     }
     return ret;
 }
+#endif /* HAVE___LXSTAT */
 
 int
 lstat(const char *path, struct stat *sb)
@@ -929,6 +939,7 @@ lstat(const char *path, struct stat *sb)
     return ret;
 }
 
+#ifdef HAVE___XSTAT
 int
 __xstat(int ver, const char *path, struct stat *sb)
 {
@@ -947,6 +958,7 @@ __xstat(int ver, const char *path, struct stat *sb)
     }
     return ret;
 }
+#endif /* HAVE___XSTAT */
 
 int
 stat(const char *path, struct stat *sb)
@@ -1049,6 +1061,3 @@ virFileCanonicalizePath(const char *path)
 
     return ret;
 }
-#else
-/* Nothing to override on non-__linux__ platforms */
-#endif
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 7/9] tests: Build virpcimock on non-Linux
Posted by Daniel P. Berrangé 7 years ago
On Thu, May 03, 2018 at 12:54:21PM +0200, Andrea Bolognani wrote:
> There are only a couple issues preventing it from working on
> other platform such as FreeBSD. Let's fix them.
> 
> With the mocking in place, qemumemlocktest and qemuxml2xmltest
> can finally succeed on FreeBSD.
> 
> This commit is best viewed with 'git show -w'.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  tests/virpcimock.c | 57 +++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 33 insertions(+), 24 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

(on basis that your followup fix is squashed in)


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 7/9] tests: Build virpcimock on non-Linux
Posted by Andrea Bolognani 7 years ago
On Thu, 2018-05-03 at 12:54 +0200, Andrea Bolognani wrote:
> There are only a couple issues preventing it from working on
> other platform such as FreeBSD. Let's fix them.
> 
> With the mocking in place, qemumemlocktest and qemuxml2xmltest
> can finally succeed on FreeBSD.

Too bad it also happens to break compilation on macOS

  duplicate symbol _rpl_lstat in:
      .libs/virpcimock.o
      ../gnulib/lib/.libs/libgnu.a(lstat.o)
  duplicate symbol _rpl_open in:
      .libs/virpcimock.o
      ../gnulib/lib/.libs/libgnu.a(open.o)
  duplicate symbol _rpl_stat in:
      .libs/virpcimock.o
      ../gnulib/lib/.libs/libgnu.a(stat.o)
  ld: 3 duplicate symbols for architecture x86_64
  clang: error: linker command failed with exit code 1 (use -v to see invocation)
  make[2]: *** [virpcimock.la] Error 1

and on MinGW

  ../../tests/virpcimock.c: In function 'make_symlink':
  ../../tests/virpcimock.c:203:9: error: implicit declaration of function 'symlink'; did you mean 'unlink'? [-Werror=implicit-function-declaration]
       if (symlink(target, filepath) < 0)
           ^~~~~~~
           unlink
  ../../tests/virpcimock.c:203:9: error: nested extern declaration of 'symlink' [-Werror=nested-externs]
  ../../tests/virpcimock.c: In function 'pci_read_file':
  ../../tests/virpcimock.c:228:5: error: implicit declaration of function 'bzero' [-Werror=implicit-function-declaration]
       bzero(buf, buf_size);
       ^~~~~
  ../../tests/virpcimock.c:228:5: error: incompatible implicit declaration of built-in function 'bzero' [-Werror]
  In file included from ../gnulib/lib/fcntl.h:58:0,
                   from ../../tests/virpcimock.c:27:
  ../../tests/virpcimock.c: At top level:
  ../../tests/virpcimock.c:964:1: error: redefinition of 'rpl_stat'
   stat(const char *path, struct stat *sb)
   ^
  ../../tests/virpcimock.c:924:1: note: previous definition of 'rpl_stat' was here
   lstat(const char *path, struct stat *sb)
   ^
  cc1: all warnings being treated as errors
  gmake[2]: *** [Makefile:5505: virpcimock.lo] Error 1

I'll look into solving the latter, but I have no intention of
spending time on the former because I don't have access to macOS
machines and debugging this kind of failure through Travis would
be just too painful and time consuming.

In the meantime, and for the purpose of reviewing the series,
consider the attached patch, which merely adds FreeBSD to the list
of platforms virpcimock should be compiled on, rather than getting
rid of such a list altogether, squashed in.

-- 
Andrea Bolognani / Red Hat / Virtualization--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 7/9] tests: Build virpcimock on non-Linux
Posted by Daniel P. Berrangé 7 years ago
On Thu, May 03, 2018 at 02:28:48PM +0200, Andrea Bolognani wrote:
> On Thu, 2018-05-03 at 12:54 +0200, Andrea Bolognani wrote:
> > There are only a couple issues preventing it from working on
> > other platform such as FreeBSD. Let's fix them.
> > 
> > With the mocking in place, qemumemlocktest and qemuxml2xmltest
> > can finally succeed on FreeBSD.
> 
> Too bad it also happens to break compilation on macOS
> 
>   duplicate symbol _rpl_lstat in:
>       .libs/virpcimock.o
>       ../gnulib/lib/.libs/libgnu.a(lstat.o)
>   duplicate symbol _rpl_open in:
>       .libs/virpcimock.o
>       ../gnulib/lib/.libs/libgnu.a(open.o)
>   duplicate symbol _rpl_stat in:
>       .libs/virpcimock.o
>       ../gnulib/lib/.libs/libgnu.a(stat.o)
>   ld: 3 duplicate symbols for architecture x86_64
>   clang: error: linker command failed with exit code 1 (use -v to see invocation)
>   make[2]: *** [virpcimock.la] Error 1
> 
> and on MinGW
> 
>   ../../tests/virpcimock.c: In function 'make_symlink':
>   ../../tests/virpcimock.c:203:9: error: implicit declaration of function 'symlink'; did you mean 'unlink'? [-Werror=implicit-function-declaration]
>        if (symlink(target, filepath) < 0)
>            ^~~~~~~
>            unlink
>   ../../tests/virpcimock.c:203:9: error: nested extern declaration of 'symlink' [-Werror=nested-externs]
>   ../../tests/virpcimock.c: In function 'pci_read_file':
>   ../../tests/virpcimock.c:228:5: error: implicit declaration of function 'bzero' [-Werror=implicit-function-declaration]
>        bzero(buf, buf_size);
>        ^~~~~
>   ../../tests/virpcimock.c:228:5: error: incompatible implicit declaration of built-in function 'bzero' [-Werror]
>   In file included from ../gnulib/lib/fcntl.h:58:0,
>                    from ../../tests/virpcimock.c:27:
>   ../../tests/virpcimock.c: At top level:
>   ../../tests/virpcimock.c:964:1: error: redefinition of 'rpl_stat'
>    stat(const char *path, struct stat *sb)
>    ^
>   ../../tests/virpcimock.c:924:1: note: previous definition of 'rpl_stat' was here
>    lstat(const char *path, struct stat *sb)
>    ^
>   cc1: all warnings being treated as errors
>   gmake[2]: *** [Makefile:5505: virpcimock.lo] Error 1
> 
> I'll look into solving the latter, but I have no intention of
> spending time on the former because I don't have access to macOS
> machines and debugging this kind of failure through Travis would
> be just too painful and time consuming.

That's ok - FreeBSD testing is already giving us better confidence
in our macOS support as there's some overlapping heritage. 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list