[libvirt] [PATCH] tests: Link mocks with libvirt.so

Michal Privoznik posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/2971d2b724bf2ef5a2728fa46ec62471c9563049.1526384717.git.mprivozn@redhat.com
Test syntax-check passed
tests/Makefile.am | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[libvirt] [PATCH] tests: Link mocks with libvirt.so
Posted by Michal Privoznik 5 years, 10 months ago
In a lot of our mocks (if not all of them) we use our internal
APIs (e.g. VIR_ALLOC). So far, we're relying on test binary that
links with the mock to drag in libvirt.so. Well, this works only
partially. Firstly, whatever binary we execute from tests will
fail (e.g. as Martin reported on the list ./qemucapsprobe fails
to execute qemu). Secondly, if there's a program that tries to
validate linking (like valgrind is doing) it fails because of
unresolved symbols.

Because of that we have to link our mocks with libvirt.so.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tests/Makefile.am | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 621480dd0c..ac92190845 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -81,7 +81,8 @@ LDADDS = \
 	../src/libvirt.la
 
 MOCKLIBS_LIBS = \
-	$(GNULIB_LIBS)
+	$(GNULIB_LIBS) \
+	../src/libvirt.la
 
 EXTRA_DIST = \
 	.valgrind.supp \
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Link mocks with libvirt.so
Posted by Daniel P. Berrangé 5 years, 10 months ago
On Tue, May 15, 2018 at 01:45:33PM +0200, Michal Privoznik wrote:
> In a lot of our mocks (if not all of them) we use our internal
> APIs (e.g. VIR_ALLOC). So far, we're relying on test binary that
> links with the mock to drag in libvirt.so. Well, this works only
> partially. Firstly, whatever binary we execute from tests will
> fail (e.g. as Martin reported on the list ./qemucapsprobe fails
> to execute qemu). Secondly, if there's a program that tries to
> validate linking (like valgrind is doing) it fails because of
> unresolved symbols.

Hmm, that first issue suggests we are not unsetting LD_PRELOAD
before executing programs. Indeed looking at git only the
qemuargv2xmltest and qemuxml2argvtest.c are unsetting LD_PRELOAD.

Seems we should make virTestMain unset LD_PRELOAD globally in
some way.

NB, I'm not objecting to this patch, just saying we shoudl fix
the LD_PRELOAD regardless.

> Because of that we have to link our mocks with libvirt.so.

It feels rather recursive to me but if it works...

> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  tests/Makefile.am | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 621480dd0c..ac92190845 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -81,7 +81,8 @@ LDADDS = \
>  	../src/libvirt.la
>  
>  MOCKLIBS_LIBS = \
> -	$(GNULIB_LIBS)
> +	$(GNULIB_LIBS) \
> +	../src/libvirt.la
>  
>  EXTRA_DIST = \
>  	.valgrind.supp \

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



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] tests: Link mocks with libvirt.so
Posted by Michal Privoznik 5 years, 10 months ago
On 05/15/2018 01:57 PM, Daniel P. Berrangé wrote:
> On Tue, May 15, 2018 at 01:45:33PM +0200, Michal Privoznik wrote:
>> In a lot of our mocks (if not all of them) we use our internal
>> APIs (e.g. VIR_ALLOC). So far, we're relying on test binary that
>> links with the mock to drag in libvirt.so. Well, this works only
>> partially. Firstly, whatever binary we execute from tests will
>> fail (e.g. as Martin reported on the list ./qemucapsprobe fails
>> to execute qemu). Secondly, if there's a program that tries to
>> validate linking (like valgrind is doing) it fails because of
>> unresolved symbols.
> 
> Hmm, that first issue suggests we are not unsetting LD_PRELOAD
> before executing programs. Indeed looking at git only the
> qemuargv2xmltest and qemuxml2argvtest.c are unsetting LD_PRELOAD.
> 
> Seems we should make virTestMain unset LD_PRELOAD globally in
> some way.

Yeah, that's what I though of too. But I'm not quite sure how to do that.

> 
> NB, I'm not objecting to this patch, just saying we shoudl fix
> the LD_PRELOAD regardless.
> 
>> Because of that we have to link our mocks with libvirt.so.
> 
> It feels rather recursive to me but if it works...

Well, yes and no. For instance if I'd have a small binary that say
attaches/detaches PCI devices on the host, I can reuse our virpcimock to
test it instead of having the binary link with libvirt.so (assume it
doesn't call any libvirt API).

> 
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  tests/Makefile.am | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index 621480dd0c..ac92190845 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -81,7 +81,8 @@ LDADDS = \
>>  	../src/libvirt.la
>>  
>>  MOCKLIBS_LIBS = \
>> -	$(GNULIB_LIBS)
>> +	$(GNULIB_LIBS) \
>> +	../src/libvirt.la
>>  
>>  EXTRA_DIST = \
>>  	.valgrind.supp \
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Pushed, thanks.

Michal

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