[libvirt] [PATCH] tests: link virpcimock with utils

Pavel Hrdina posted 1 patch 7 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/326015dc836ecd21647ffcba3bd9be6a8b514732.1491810741.git.phrdina@redhat.com
tests/Makefile.am | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[libvirt] [PATCH] tests: link virpcimock with utils
Posted by Pavel Hrdina 7 years ago
In virpcimock we use some functions from utils so we should link
utils library to provide required symbols.

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

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 279e9b7da8..8b9bba31ce 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1131,7 +1131,8 @@ virpcimock_la_SOURCES = \
 	virpcimock.c
 virpcimock_la_CFLAGS = $(AM_CFLAGS)
 virpcimock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
-virpcimock_la_LIBADD = $(MOCKLIBS_LIBS)
+virpcimock_la_LIBADD = $(MOCKLIBS_LIBS) \
+	../src/libvirt_util.la
 
 virrandommock_la_SOURCES = \
 	virrandommock.c
-- 
2.12.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: link virpcimock with utils
Posted by Michal Privoznik 7 years ago
On 04/10/2017 09:52 AM, Pavel Hrdina wrote:
> In virpcimock we use some functions from utils so we should link
> utils library to provide required symbols.
>
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  tests/Makefile.am | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 279e9b7da8..8b9bba31ce 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -1131,7 +1131,8 @@ virpcimock_la_SOURCES = \
>  	virpcimock.c
>  virpcimock_la_CFLAGS = $(AM_CFLAGS)
>  virpcimock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
> -virpcimock_la_LIBADD = $(MOCKLIBS_LIBS)
> +virpcimock_la_LIBADD = $(MOCKLIBS_LIBS) \
> +	../src/libvirt_util.la

You forgot to include $(PROBES_O). But even with that added in, this 
will not fly. While you make it work under libvirt it fails without it 
because 'stat' is undefined but called. Look into virtestmock.c for 
explanation.

I'm not sure how to address this issue. I know about it for quite some 
time now but all my previous attempts to fix it (including this one) 
failed. I've tried to mimic what virtestmock.c does, but without any luck.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: link virpcimock with utils
Posted by Daniel P. Berrange 7 years ago
On Mon, Apr 10, 2017 at 01:00:58PM +0200, Michal Privoznik wrote:
> On 04/10/2017 09:52 AM, Pavel Hrdina wrote:
> > In virpcimock we use some functions from utils so we should link
> > utils library to provide required symbols.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  tests/Makefile.am | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index 279e9b7da8..8b9bba31ce 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -1131,7 +1131,8 @@ virpcimock_la_SOURCES = \
> >  	virpcimock.c
> >  virpcimock_la_CFLAGS = $(AM_CFLAGS)
> >  virpcimock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
> > -virpcimock_la_LIBADD = $(MOCKLIBS_LIBS)
> > +virpcimock_la_LIBADD = $(MOCKLIBS_LIBS) \
> > +	../src/libvirt_util.la
> 
> You forgot to include $(PROBES_O). But even with that added in, this will
> not fly. While you make it work under libvirt it fails without it because
> 'stat' is undefined but called. Look into virtestmock.c for explanation.
> 
> I'm not sure how to address this issue. I know about it for quite some time
> now but all my previous attempts to fix it (including this one) failed. I've
> tried to mimic what virtestmock.c does, but without any luck.

What is the actual build problem with the virpcimock ? It seems to work
fine for me right now.

FWIW, the original intenion was that the mock impls should not use any
libvirt APIs, just pure POSIX / Linux APIs, so as to avoid dependancies
which could end up being circular.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: link virpcimock with utils
Posted by Michal Privoznik 7 years ago
On 04/10/2017 01:04 PM, Daniel P. Berrange wrote:
> On Mon, Apr 10, 2017 at 01:00:58PM +0200, Michal Privoznik wrote:
>> On 04/10/2017 09:52 AM, Pavel Hrdina wrote:
>>> In virpcimock we use some functions from utils so we should link
>>> utils library to provide required symbols.
>>>
>>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>>> ---
>>>  tests/Makefile.am | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>>> index 279e9b7da8..8b9bba31ce 100644
>>> --- a/tests/Makefile.am
>>> +++ b/tests/Makefile.am
>>> @@ -1131,7 +1131,8 @@ virpcimock_la_SOURCES = \
>>>  	virpcimock.c
>>>  virpcimock_la_CFLAGS = $(AM_CFLAGS)
>>>  virpcimock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
>>> -virpcimock_la_LIBADD = $(MOCKLIBS_LIBS)
>>> +virpcimock_la_LIBADD = $(MOCKLIBS_LIBS) \
>>> +	../src/libvirt_util.la
>>
>> You forgot to include $(PROBES_O). But even with that added in, this will
>> not fly. While you make it work under libvirt it fails without it because
>> 'stat' is undefined but called. Look into virtestmock.c for explanation.
>>
>> I'm not sure how to address this issue. I know about it for quite some time
>> now but all my previous attempts to fix it (including this one) failed. I've
>> tried to mimic what virtestmock.c does, but without any luck.
> 
> What is the actual build problem with the virpcimock ? It seems to work
> fine for me right now.

It does build fine. You want to run it under valgrind:

libvirt.git/tests $ ../run valgrind --trace-children=yes ./virpcitest
==30672== Memcheck, a memory error detector
==30672== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==30672== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==30672== Command: libvirt.git/tests/.libs/virpcitest
==30672== 
valgrind: symbol lookup error: libvirt.git/tests/.libs/virpcimock.so: undefined symbol: virFree



> 
> FWIW, the original intenion was that the mock impls should not use any
> libvirt APIs, just pure POSIX / Linux APIs, so as to avoid dependancies
> which could end up being circular.

Yeah, I was just about to suggests this. However, in general would it be a problem if a mock library would link with say libvirt_utils statically? That way we shouldn't have any dependency problems, should we?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: link virpcimock with utils
Posted by Daniel P. Berrange 7 years ago
On Mon, Apr 10, 2017 at 01:18:05PM +0200, Michal Privoznik wrote:
> On 04/10/2017 01:04 PM, Daniel P. Berrange wrote:
> > On Mon, Apr 10, 2017 at 01:00:58PM +0200, Michal Privoznik wrote:
> >> On 04/10/2017 09:52 AM, Pavel Hrdina wrote:
> >>> In virpcimock we use some functions from utils so we should link
> >>> utils library to provide required symbols.
> >>>
> >>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> >>> ---
> >>>  tests/Makefile.am | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tests/Makefile.am b/tests/Makefile.am
> >>> index 279e9b7da8..8b9bba31ce 100644
> >>> --- a/tests/Makefile.am
> >>> +++ b/tests/Makefile.am
> >>> @@ -1131,7 +1131,8 @@ virpcimock_la_SOURCES = \
> >>>  	virpcimock.c
> >>>  virpcimock_la_CFLAGS = $(AM_CFLAGS)
> >>>  virpcimock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
> >>> -virpcimock_la_LIBADD = $(MOCKLIBS_LIBS)
> >>> +virpcimock_la_LIBADD = $(MOCKLIBS_LIBS) \
> >>> +	../src/libvirt_util.la
> >>
> >> You forgot to include $(PROBES_O). But even with that added in, this will
> >> not fly. While you make it work under libvirt it fails without it because
> >> 'stat' is undefined but called. Look into virtestmock.c for explanation.
> >>
> >> I'm not sure how to address this issue. I know about it for quite some time
> >> now but all my previous attempts to fix it (including this one) failed. I've
> >> tried to mimic what virtestmock.c does, but without any luck.
> > 
> > What is the actual build problem with the virpcimock ? It seems to work
> > fine for me right now.
> 
> It does build fine. You want to run it under valgrind:
> 
> libvirt.git/tests $ ../run valgrind --trace-children=yes ./virpcitest
> ==30672== Memcheck, a memory error detector
> ==30672== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
> ==30672== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
> ==30672== Command: libvirt.git/tests/.libs/virpcitest
> ==30672== 
> valgrind: symbol lookup error: libvirt.git/tests/.libs/virpcimock.so: undefined symbol: virFree
> 
> 
> 
> > 
> > FWIW, the original intenion was that the mock impls should not use any
> > libvirt APIs, just pure POSIX / Linux APIs, so as to avoid dependancies
> > which could end up being circular.
> 
> Yeah, I was just about to suggests this. However, in general would
> it be a problem if a mock library would link with say libvirt_utils
> statically? That way we shouldn't have any dependency problems,
> should we?

Yep, static linking makes you immune from LD_PRELOADs.

The one caveat is about global data & any 3rd party library initialization
though. eg You'd now have 2 copies of any global variables, and if you
trigger code that initializes 3rd party libraries that'd get run twice.

Probably ok if we're only using very simple things like virfile & virstring
but if we get into the more advanced modules it could cause trouble.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: link virpcimock with utils
Posted by Martin Kletzander 7 years ago
On Mon, Apr 10, 2017 at 12:26:40PM +0100, Daniel P. Berrange wrote:
>On Mon, Apr 10, 2017 at 01:18:05PM +0200, Michal Privoznik wrote:
>> On 04/10/2017 01:04 PM, Daniel P. Berrange wrote:
>> > On Mon, Apr 10, 2017 at 01:00:58PM +0200, Michal Privoznik wrote:
>> >> On 04/10/2017 09:52 AM, Pavel Hrdina wrote:
>> >>> In virpcimock we use some functions from utils so we should link
>> >>> utils library to provide required symbols.
>> >>>
>> >>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>> >>> ---
>> >>>  tests/Makefile.am | 3 ++-
>> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> >>> index 279e9b7da8..8b9bba31ce 100644
>> >>> --- a/tests/Makefile.am
>> >>> +++ b/tests/Makefile.am
>> >>> @@ -1131,7 +1131,8 @@ virpcimock_la_SOURCES = \
>> >>>  	virpcimock.c
>> >>>  virpcimock_la_CFLAGS = $(AM_CFLAGS)
>> >>>  virpcimock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
>> >>> -virpcimock_la_LIBADD = $(MOCKLIBS_LIBS)
>> >>> +virpcimock_la_LIBADD = $(MOCKLIBS_LIBS) \
>> >>> +	../src/libvirt_util.la
>> >>
>> >> You forgot to include $(PROBES_O). But even with that added in, this will
>> >> not fly. While you make it work under libvirt it fails without it because
>> >> 'stat' is undefined but called. Look into virtestmock.c for explanation.
>> >>
>> >> I'm not sure how to address this issue. I know about it for quite some time
>> >> now but all my previous attempts to fix it (including this one) failed. I've
>> >> tried to mimic what virtestmock.c does, but without any luck.
>> >
>> > What is the actual build problem with the virpcimock ? It seems to work
>> > fine for me right now.
>>
>> It does build fine. You want to run it under valgrind:
>>
>> libvirt.git/tests $ ../run valgrind --trace-children=yes ./virpcitest
>> ==30672== Memcheck, a memory error detector
>> ==30672== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
>> ==30672== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
>> ==30672== Command: libvirt.git/tests/.libs/virpcitest
>> ==30672==
>> valgrind: symbol lookup error: libvirt.git/tests/.libs/virpcimock.so: undefined symbol: virFree
>>
>>
>>
>> >
>> > FWIW, the original intenion was that the mock impls should not use any
>> > libvirt APIs, just pure POSIX / Linux APIs, so as to avoid dependancies
>> > which could end up being circular.
>>
>> Yeah, I was just about to suggests this. However, in general would
>> it be a problem if a mock library would link with say libvirt_utils
>> statically? That way we shouldn't have any dependency problems,
>> should we?
>
>Yep, static linking makes you immune from LD_PRELOADs.
>
>The one caveat is about global data & any 3rd party library initialization
>though. eg You'd now have 2 copies of any global variables, and if you
>trigger code that initializes 3rd party libraries that'd get run twice.
>
>Probably ok if we're only using very simple things like virfile & virstring
>but if we get into the more advanced modules it could cause trouble.
>

Can't we instead make all mocked functions just weak aliases to our real
functions?  That way mocks wouldn't need to be preloaded and it could
'just work'.  At least that what I think why virfilemock.c works.
Despite its name it's not actually a mock.  See vircaps2xmltest.c and
Makefile in patch 04/12:

  https://www.redhat.com/archives/libvir-list/2017-April/msg00233.html

And usage in 05/12:

  https://www.redhat.com/archives/libvir-list/2017-April/msg00234.html

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