[libvirt] [PATCH] src: Move DLOPEN_LIBS to libraries introducing the dependency

Michal Privoznik posted 1 patch 5 years, 8 months ago
Failed in applying to current master (apply log)
src/Makefile.am          | 2 ++
src/util/Makefile.inc.am | 1 +
tools/Makefile.am        | 4 ++--
3 files changed, 5 insertions(+), 2 deletions(-)
[libvirt] [PATCH] src: Move DLOPEN_LIBS to libraries introducing the dependency
Posted by Michal Privoznik 5 years, 8 months ago
There are few places where dlopen() is called. This call means we
have to link with DLOPEN_LIBS. However, instead of having each
final, installable library linking with it, move the directive to
the source that introduced the dependency.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/Makefile.am          | 2 ++
 src/util/Makefile.inc.am | 1 +
 tools/Makefile.am        | 4 ++--
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index a4f213480e..61876cf382 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -752,6 +752,7 @@ libvirt_setuid_rpc_client_la_CFLAGS = \
 		$(AM_CFLAGS) \
 		$(SECDRIVER_CFLAGS) \
 		$(XDR_CFLAGS) \
+		$(DLOPEN_LIBS) \
 		$(NULL)
 endif WITH_SETUID_RPC_CLIENT
 
@@ -1000,6 +1001,7 @@ libvirt_nss_la_CFLAGS = \
 		$(NULL)
 libvirt_nss_la_LDFLAGS = \
 		$(AM_LDFLAGS) \
+		$(DLOPEN_LIBS) \
 		$(NULL)
 endif WITH_NSS
 
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index 8ef9ee1dfa..c5c50f1844 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -278,6 +278,7 @@ libvirt_util_la_LIBADD = \
 	$(NUMACTL_LIBS) \
 	$(ACL_LIBS) \
 	$(GNUTLS_LIBS) \
+	$(DLOPEN_LIBS) \
 	$(NULL)
 
 
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 26c887649e..3e129c04c4 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -528,7 +528,7 @@ nss_libnss_libvirt_impl_la_CFLAGS = \
 nss_libnss_libvirt_impl_la_LIBADD = \
 	../gnulib/lib/libgnu.la \
 	../src/libvirt-nss.la \
-	$(DLOPEN_LIBS)
+	$(NULL)
 
 nss_libnss_libvirt_la_SOURCES =
 nss_libnss_libvirt_la_LDFLAGS = \
@@ -556,7 +556,7 @@ nss_libnss_libvirt_guest_impl_la_CFLAGS = \
 nss_libnss_libvirt_guest_impl_la_LIBADD = \
 	../gnulib/lib/libgnu.la \
 	../src/libvirt-nss.la \
-	$(DLOPEN_LIBS)
+	$(NULL)
 
 nss_libnss_libvirt_guest_la_SOURCES =
 nss_libnss_libvirt_guest_la_LDFLAGS = \
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] src: Move DLOPEN_LIBS to libraries introducing the dependency
Posted by Jim Fehlig 5 years, 8 months ago
On 08/09/2018 11:45 PM, Michal Privoznik wrote:
> There are few places where dlopen() is called. This call means we
> have to link with DLOPEN_LIBS. However, instead of having each
> final, installable library linking with it, move the directive to
> the source that introduced the dependency.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Jim Fehlig <jfehlig@suse.com>

> ---
>   src/Makefile.am          | 2 ++
>   src/util/Makefile.inc.am | 1 +
>   tools/Makefile.am        | 4 ++--
>   3 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index a4f213480e..61876cf382 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -752,6 +752,7 @@ libvirt_setuid_rpc_client_la_CFLAGS = \
>   		$(AM_CFLAGS) \
>   		$(SECDRIVER_CFLAGS) \
>   		$(XDR_CFLAGS) \
> +		$(DLOPEN_LIBS) \
>   		$(NULL)
>   endif WITH_SETUID_RPC_CLIENT
>   
> @@ -1000,6 +1001,7 @@ libvirt_nss_la_CFLAGS = \
>   		$(NULL)
>   libvirt_nss_la_LDFLAGS = \
>   		$(AM_LDFLAGS) \
> +		$(DLOPEN_LIBS) \
>   		$(NULL)
>   endif WITH_NSS
>   
> diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
> index 8ef9ee1dfa..c5c50f1844 100644
> --- a/src/util/Makefile.inc.am
> +++ b/src/util/Makefile.inc.am
> @@ -278,6 +278,7 @@ libvirt_util_la_LIBADD = \
>   	$(NUMACTL_LIBS) \
>   	$(ACL_LIBS) \
>   	$(GNUTLS_LIBS) \
> +	$(DLOPEN_LIBS) \
>   	$(NULL)
>   
>   
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 26c887649e..3e129c04c4 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -528,7 +528,7 @@ nss_libnss_libvirt_impl_la_CFLAGS = \
>   nss_libnss_libvirt_impl_la_LIBADD = \
>   	../gnulib/lib/libgnu.la \
>   	../src/libvirt-nss.la \
> -	$(DLOPEN_LIBS)
> +	$(NULL)
>   
>   nss_libnss_libvirt_la_SOURCES =
>   nss_libnss_libvirt_la_LDFLAGS = \
> @@ -556,7 +556,7 @@ nss_libnss_libvirt_guest_impl_la_CFLAGS = \
>   nss_libnss_libvirt_guest_impl_la_LIBADD = \
>   	../gnulib/lib/libgnu.la \
>   	../src/libvirt-nss.la \
> -	$(DLOPEN_LIBS)
> +	$(NULL)
>   
>   nss_libnss_libvirt_guest_la_SOURCES =
>   nss_libnss_libvirt_guest_la_LDFLAGS = \
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] src: Move DLOPEN_LIBS to libraries introducing the dependency
Posted by Julio Faracco 5 years, 8 months ago
Hi guys,

Em sex, 10 de ago de 2018 às 11:23, Jim Fehlig <jfehlig@suse.com> escreveu:
>
> On 08/09/2018 11:45 PM, Michal Privoznik wrote:
> > There are few places where dlopen() is called. This call means we
> > have to link with DLOPEN_LIBS. However, instead of having each
> > final, installable library linking with it, move the directive to
> > the source that introduced the dependency.
> >
> > Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>
> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
>
> > ---
> >   src/Makefile.am          | 2 ++
> >   src/util/Makefile.inc.am | 1 +
> >   tools/Makefile.am        | 4 ++--
> >   3 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index a4f213480e..61876cf382 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -752,6 +752,7 @@ libvirt_setuid_rpc_client_la_CFLAGS = \
> >               $(AM_CFLAGS) \
> >               $(SECDRIVER_CFLAGS) \
> >               $(XDR_CFLAGS) \
> > +             $(DLOPEN_LIBS) \
> >               $(NULL)
> >   endif WITH_SETUID_RPC_CLIENT

I think that DLOPEN_LIBS is not needed for setuid_rpc_client.
See the error when I enable RPC libraries:
clang: error: -ldl: 'linker' input unused
[-Werror,-Wunused-command-line-argument]
Makefile:9014: recipe for target
'util/libvirt_setuid_rpc_client_la-viralloc.lo' failed

> >
> > @@ -1000,6 +1001,7 @@ libvirt_nss_la_CFLAGS = \
> >               $(NULL)
> >   libvirt_nss_la_LDFLAGS = \
> >               $(AM_LDFLAGS) \
> > +             $(DLOPEN_LIBS) \
> >               $(NULL)
> >   endif WITH_NSS
> >
> > diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
> > index 8ef9ee1dfa..c5c50f1844 100644
> > --- a/src/util/Makefile.inc.am
> > +++ b/src/util/Makefile.inc.am
> > @@ -278,6 +278,7 @@ libvirt_util_la_LIBADD = \
> >       $(NUMACTL_LIBS) \
> >       $(ACL_LIBS) \
> >       $(GNUTLS_LIBS) \
> > +     $(DLOPEN_LIBS) \
> >       $(NULL)
> >
> >
> > diff --git a/tools/Makefile.am b/tools/Makefile.am
> > index 26c887649e..3e129c04c4 100644
> > --- a/tools/Makefile.am
> > +++ b/tools/Makefile.am
> > @@ -528,7 +528,7 @@ nss_libnss_libvirt_impl_la_CFLAGS = \
> >   nss_libnss_libvirt_impl_la_LIBADD = \
> >       ../gnulib/lib/libgnu.la \
> >       ../src/libvirt-nss.la \
> > -     $(DLOPEN_LIBS)
> > +     $(NULL)
> >
> >   nss_libnss_libvirt_la_SOURCES =
> >   nss_libnss_libvirt_la_LDFLAGS = \
> > @@ -556,7 +556,7 @@ nss_libnss_libvirt_guest_impl_la_CFLAGS = \
> >   nss_libnss_libvirt_guest_impl_la_LIBADD = \
> >       ../gnulib/lib/libgnu.la \
> >       ../src/libvirt-nss.la \
> > -     $(DLOPEN_LIBS)
> > +     $(NULL)
> >
> >   nss_libnss_libvirt_guest_la_SOURCES =
> >   nss_libnss_libvirt_guest_la_LDFLAGS = \
> >
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] src: Move DLOPEN_LIBS to libraries introducing the dependency
Posted by Michal Prívozník 5 years, 8 months ago
On 08/12/2018 11:46 PM, Julio Faracco wrote:
> Hi guys,
> 
> Em sex, 10 de ago de 2018 às 11:23, Jim Fehlig <jfehlig@suse.com> escreveu:
>>
>> On 08/09/2018 11:45 PM, Michal Privoznik wrote:
>>> There are few places where dlopen() is called. This call means we
>>> have to link with DLOPEN_LIBS. However, instead of having each
>>> final, installable library linking with it, move the directive to
>>> the source that introduced the dependency.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>
>> Reviewed-by: Jim Fehlig <jfehlig@suse.com>
>>
>>> ---
>>>   src/Makefile.am          | 2 ++
>>>   src/util/Makefile.inc.am | 1 +
>>>   tools/Makefile.am        | 4 ++--
>>>   3 files changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/Makefile.am b/src/Makefile.am
>>> index a4f213480e..61876cf382 100644
>>> --- a/src/Makefile.am
>>> +++ b/src/Makefile.am
>>> @@ -752,6 +752,7 @@ libvirt_setuid_rpc_client_la_CFLAGS = \
>>>               $(AM_CFLAGS) \
>>>               $(SECDRIVER_CFLAGS) \
>>>               $(XDR_CFLAGS) \
>>> +             $(DLOPEN_LIBS) \
>>>               $(NULL)
>>>   endif WITH_SETUID_RPC_CLIENT
> 
> I think that DLOPEN_LIBS is not needed for setuid_rpc_client.
> See the error when I enable RPC libraries:
> clang: error: -ldl: 'linker' input unused
> [-Werror,-Wunused-command-line-argument]
> Makefile:9014: recipe for target
> 'util/libvirt_setuid_rpc_client_la-viralloc.lo' failed


Ah, so the library compiles virjsoncompact.c in which I used as an
indication where to put DLOPEN_LIBS. But now that I look at it closer it
doesn't make sense for the lib to compile in the file because in
config-post.h we clearly disable JANSSON support.

Sigh, this jansson has brought more problems than it solved.

Anyway, I think this is the fix:

diff --git i/src/Makefile.am w/src/Makefile.am
index 61876cf382..d9bce1e8ab 100644
--- i/src/Makefile.am
+++ w/src/Makefile.am
@@ -689,7 +689,6 @@ libvirt_setuid_rpc_client_la_SOURCES = \
                util/virhashcode.c \
                util/virhostcpu.c \
                util/virjson.c \
-               util/virjsoncompat.c \
                util/virlog.c \
                util/virobject.c \
                util/virpidfile.c \
@@ -752,7 +751,6 @@ libvirt_setuid_rpc_client_la_CFLAGS = \
                $(AM_CFLAGS) \
                $(SECDRIVER_CFLAGS) \
                $(XDR_CFLAGS) \
-               $(DLOPEN_LIBS) \
                $(NULL)
 endif WITH_SETUID_RPC_CLIENT


Can you test it please? I'm no longer confident around jansson.

Michal

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