1. Patchew
  2. Libvirt
  3. [PATCH 0/4] Final changes to fix test suite on macOS
  4. View patch

[PATCH 2/4] tests: Fix mock chaining on macOS

Roman Bolshakov posted 4 patches 4 years ago
There is a newer version of this series
[PATCH 2/4] tests: Fix mock chaining on macOS
Posted by Roman Bolshakov 4 years ago 
Some tests in qemuxml2argvtest need opendir() from virpcimock, others
need opendir() from virfilewrapper.

But as of now, only opendir() from virpcimock has an effect.
real_opendir in virpcimock has a pointer to opendir$INODE64 in
libsystem_kernel.dylib instead of pointing to opendir$INODE64 in
qemuxml2argvtest (from virfilewrapper). And because the second one is
never used, tests that rely on prefixes added by virFileWrapperAddPrefix
fail.

That can be fixed if dlsym(3) is asked explicitly to search symbols in
main executable with RTLD_MAIN_ONLY before going to other dylibs.
Existing RTLD_NEXT handle results into libsystem_kernel.dylib being
searched before main executable.

Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 tests/virmock.h    | 37 +++++++++++++++++++++++++++++++++++--
 tests/virpcimock.c |  1 +
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/tests/virmock.h b/tests/virmock.h
index dea5feb80f..db051f3636 100644
--- a/tests/virmock.h
+++ b/tests/virmock.h
@@ -284,8 +284,19 @@
     static void (*real_##name)(void); \
     void wrap_##name(void)
 
+#ifdef __APPLE__
+# define VIR_MOCK_REAL_INIT_MAIN(name, alias) \
+    do { \
+        if (real_##name == NULL) { \
+            real_##name = dlsym(RTLD_MAIN_ONLY, alias); \
+        } \
+    } while (0)
+#else
+# define VIR_MOCK_REAL_INIT_MAIN(name, alias) \
+    do {} while (0)
+#endif
 
-#define VIR_MOCK_REAL_INIT(name) \
+#define VIR_MOCK_REAL_INIT_NEXT(name) \
     do { \
         if (real_##name == NULL && \
             !(real_##name = dlsym(RTLD_NEXT, \
@@ -295,7 +306,7 @@
         } \
     } while (0)
 
-#define VIR_MOCK_REAL_INIT_ALIASED(name, alias) \
+#define VIR_MOCK_REAL_INIT_ALIASED_NEXT(name, alias) \
     do { \
         if (real_##name == NULL && \
             !(real_##name = dlsym(RTLD_NEXT, \
@@ -304,3 +315,25 @@
             abort(); \
         } \
     } while (0)
+
+#ifdef VIR_MOCK_LOOKUP_MAIN
+# define VIR_MOCK_REAL_INIT(name) \
+    do { \
+        VIR_MOCK_REAL_INIT_MAIN(name, #name); \
+        VIR_MOCK_REAL_INIT_NEXT(name); \
+    } while (0)
+# define VIR_MOCK_REAL_INIT_ALIASED(name, alias) \
+    do { \
+        VIR_MOCK_REAL_INIT_MAIN(name, alias); \
+        VIR_MOCK_REAL_INIT_ALIASED_NEXT(name, alias); \
+    } while (0)
+#else
+# define VIR_MOCK_REAL_INIT(name) \
+    do { \
+        VIR_MOCK_REAL_INIT_NEXT(name); \
+    } while (0)
+# define VIR_MOCK_REAL_INIT_ALIASED(name, alias) \
+    do { \
+        VIR_MOCK_REAL_INIT_ALIASED_NEXT(name, alias); \
+    } while (0)
+#endif
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 686f894e99..4aa96cae08 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -19,6 +19,7 @@
 #include <config.h>
 
 #if defined(__linux__) || defined(__FreeBSD__) || defined(__APPLE__)
+# define VIR_MOCK_LOOKUP_MAIN
 # include "virmock.h"
 # include <unistd.h>
 # include <fcntl.h>
-- 
2.29.2
Re: [PATCH 2/4] tests: Fix mock chaining on macOS
Posted by Michal Privoznik 4 years ago 
On 11/8/20 10:24 PM, Roman Bolshakov wrote:
> Some tests in qemuxml2argvtest need opendir() from virpcimock, others
> need opendir() from virfilewrapper.
> 
> But as of now, only opendir() from virpcimock has an effect.
> real_opendir in virpcimock has a pointer to opendir$INODE64 in
> libsystem_kernel.dylib instead of pointing to opendir$INODE64 in
> qemuxml2argvtest (from virfilewrapper). And because the second one is
> never used, tests that rely on prefixes added by virFileWrapperAddPrefix
> fail.
> 
> That can be fixed if dlsym(3) is asked explicitly to search symbols in
> main executable with RTLD_MAIN_ONLY before going to other dylibs.
> Existing RTLD_NEXT handle results into libsystem_kernel.dylib being
> searched before main executable.
> 
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
>   tests/virmock.h    | 37 +++++++++++++++++++++++++++++++++++--
>   tests/virpcimock.c |  1 +
>   2 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/virmock.h b/tests/virmock.h
> index dea5feb80f..db051f3636 100644
> --- a/tests/virmock.h
> +++ b/tests/virmock.h
> @@ -284,8 +284,19 @@
>       static void (*real_##name)(void); \
>       void wrap_##name(void)
>   
> +#ifdef __APPLE__
> +# define VIR_MOCK_REAL_INIT_MAIN(name, alias) \
> +    do { \
> +        if (real_##name == NULL) { \
> +            real_##name = dlsym(RTLD_MAIN_ONLY, alias); \
> +        } \
> +    } while (0)
> +#else
> +# define VIR_MOCK_REAL_INIT_MAIN(name, alias) \
> +    do {} while (0)
> +#endif
>   
> -#define VIR_MOCK_REAL_INIT(name) \
> +#define VIR_MOCK_REAL_INIT_NEXT(name) \
>       do { \
>           if (real_##name == NULL && \
>               !(real_##name = dlsym(RTLD_NEXT, \
> @@ -295,7 +306,7 @@
>           } \
>       } while (0)
>   
> -#define VIR_MOCK_REAL_INIT_ALIASED(name, alias) \
> +#define VIR_MOCK_REAL_INIT_ALIASED_NEXT(name, alias) \
>       do { \
>           if (real_##name == NULL && \
>               !(real_##name = dlsym(RTLD_NEXT, \
> @@ -304,3 +315,25 @@
>               abort(); \
>           } \
>       } while (0)
> +
> +#ifdef VIR_MOCK_LOOKUP_MAIN

I think we can do this even without this macro. If the 
VIR_MOCK_LOOKUP_MAIN macro is not defined then we are still guarded by 
VIR_MOCK_REAL_INIT_MAIN() being a NOP on !__APPLE__. Also ..

> +# define VIR_MOCK_REAL_INIT(name) \
> +    do { \
> +        VIR_MOCK_REAL_INIT_MAIN(name, #name); \
> +        VIR_MOCK_REAL_INIT_NEXT(name); \
> +    } while (0)
> +# define VIR_MOCK_REAL_INIT_ALIASED(name, alias) \
> +    do { \
> +        VIR_MOCK_REAL_INIT_MAIN(name, alias); \
> +        VIR_MOCK_REAL_INIT_ALIASED_NEXT(name, alias); \
> +    } while (0)
> +#else
> +# define VIR_MOCK_REAL_INIT(name) \
> +    do { \
> +        VIR_MOCK_REAL_INIT_NEXT(name); \
> +    } while (0)
> +# define VIR_MOCK_REAL_INIT_ALIASED(name, alias) \
> +    do { \
> +        VIR_MOCK_REAL_INIT_ALIASED_NEXT(name, alias); \
> +    } while (0)
> +#endif
> diff --git a/tests/virpcimock.c b/tests/virpcimock.c
> index 686f894e99..4aa96cae08 100644
> --- a/tests/virpcimock.c
> +++ b/tests/virpcimock.c
> @@ -19,6 +19,7 @@
>   #include <config.h>
>   
>   #if defined(__linux__) || defined(__FreeBSD__) || defined(__APPLE__)
> +# define VIR_MOCK_LOOKUP_MAIN

.. it feels a bit odd that this is defined for virpcimock only.

Michal
Re: [PATCH 2/4] tests: Fix mock chaining on macOS
Posted by Roman Bolshakov 4 years ago 
On Fri, Nov 13, 2020 at 04:58:39PM +0100, Michal Privoznik wrote:
> On 11/8/20 10:24 PM, Roman Bolshakov wrote:
> > Some tests in qemuxml2argvtest need opendir() from virpcimock, others
> > need opendir() from virfilewrapper.
> > 
> > But as of now, only opendir() from virpcimock has an effect.
> > real_opendir in virpcimock has a pointer to opendir$INODE64 in
> > libsystem_kernel.dylib instead of pointing to opendir$INODE64 in
> > qemuxml2argvtest (from virfilewrapper). And because the second one is
> > never used, tests that rely on prefixes added by virFileWrapperAddPrefix
> > fail.
> > 
> > That can be fixed if dlsym(3) is asked explicitly to search symbols in
> > main executable with RTLD_MAIN_ONLY before going to other dylibs.
> > Existing RTLD_NEXT handle results into libsystem_kernel.dylib being
> > searched before main executable.
> > 
> > Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> > ---
> >   tests/virmock.h    | 37 +++++++++++++++++++++++++++++++++++--
> >   tests/virpcimock.c |  1 +
> >   2 files changed, 36 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/virmock.h b/tests/virmock.h
> > index dea5feb80f..db051f3636 100644
> > --- a/tests/virmock.h
> > +++ b/tests/virmock.h
> > @@ -284,8 +284,19 @@
> >       static void (*real_##name)(void); \
> >       void wrap_##name(void)
> > +#ifdef __APPLE__
> > +# define VIR_MOCK_REAL_INIT_MAIN(name, alias) \
> > +    do { \
> > +        if (real_##name == NULL) { \
> > +            real_##name = dlsym(RTLD_MAIN_ONLY, alias); \
> > +        } \
> > +    } while (0)
> > +#else
> > +# define VIR_MOCK_REAL_INIT_MAIN(name, alias) \
> > +    do {} while (0)
> > +#endif
> > -#define VIR_MOCK_REAL_INIT(name) \
> > +#define VIR_MOCK_REAL_INIT_NEXT(name) \
> >       do { \
> >           if (real_##name == NULL && \
> >               !(real_##name = dlsym(RTLD_NEXT, \
> > @@ -295,7 +306,7 @@
> >           } \
> >       } while (0)
> > -#define VIR_MOCK_REAL_INIT_ALIASED(name, alias) \
> > +#define VIR_MOCK_REAL_INIT_ALIASED_NEXT(name, alias) \
> >       do { \
> >           if (real_##name == NULL && \
> >               !(real_##name = dlsym(RTLD_NEXT, \
> > @@ -304,3 +315,25 @@
> >               abort(); \
> >           } \
> >       } while (0)
> > +
> > +#ifdef VIR_MOCK_LOOKUP_MAIN
> 
> I think we can do this even without this macro. If the VIR_MOCK_LOOKUP_MAIN
> macro is not defined then we are still guarded by VIR_MOCK_REAL_INIT_MAIN()
> being a NOP on !__APPLE__. Also ..


Yeah, that works for me. I thought of it after I sent the patch :)
I'll improve it, thanks!

> 
> > +# define VIR_MOCK_REAL_INIT(name) \
> > +    do { \
> > +        VIR_MOCK_REAL_INIT_MAIN(name, #name); \
> > +        VIR_MOCK_REAL_INIT_NEXT(name); \
> > +    } while (0)
> > +# define VIR_MOCK_REAL_INIT_ALIASED(name, alias) \
> > +    do { \
> > +        VIR_MOCK_REAL_INIT_MAIN(name, alias); \
> > +        VIR_MOCK_REAL_INIT_ALIASED_NEXT(name, alias); \
> > +    } while (0)
> > +#else
> > +# define VIR_MOCK_REAL_INIT(name) \
> > +    do { \
> > +        VIR_MOCK_REAL_INIT_NEXT(name); \
> > +    } while (0)
> > +# define VIR_MOCK_REAL_INIT_ALIASED(name, alias) \
> > +    do { \
> > +        VIR_MOCK_REAL_INIT_ALIASED_NEXT(name, alias); \
> > +    } while (0)
> > +#endif
> > diff --git a/tests/virpcimock.c b/tests/virpcimock.c
> > index 686f894e99..4aa96cae08 100644
> > --- a/tests/virpcimock.c
> > +++ b/tests/virpcimock.c
> > @@ -19,6 +19,7 @@
> >   #include <config.h>
> >   #if defined(__linux__) || defined(__FreeBSD__) || defined(__APPLE__)
> > +# define VIR_MOCK_LOOKUP_MAIN
> 
> .. it feels a bit odd that this is defined for virpcimock only.
> 

That's the only place where exact lookup order is needed.

Regards,
Roman

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.