[libvirt] [PATCH v3] nwfilter: fix loadable module support

[Daniel P. Berrange]

On Tue, Jun 22, 2010 at 12:48:37PM +0200, Matthias Bolte wrote:
> 2010/6/22 Daniel P. Berrange <berrange at redhat.com>:
> > On Mon, Jun 21, 2010 at 02:27:36PM -0400, Stefan Berger wrote:
> >> diff --git a/src/Makefile.am b/src/Makefile.am
> >> index 5109302..e553f35 100644
> >> --- a/src/Makefile.am
> >> +++ b/src/Makefile.am
> >> @@ -1001,7 +1001,7 @@ libvirt_la_LDFLAGS =
> >> $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMB
> >>                     $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS)
> >>  libvirt_la_BUILT_LIBADD += ../gnulib/lib/libgnu.la
> >>  libvirt_la_LIBADD += $(LIBXML_LIBS) \
> >> -                   $(LIBPCAP_LIBS) $(LIBNL_LIBS) \
> >> +                   $(LIBPCAP_LIBS) $(LIBNL_LIBS) $(GNUTLS_LIBS) \
> >>                     $(DRIVER_MODULE_LIBS) \
> >>                     $(CYGWIN_EXTRA_LIBADD)
> >>  libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT
> >
> > I think that one needs to be against libvirt_driver_la_CFLAGS instead.
> > since that's where the source file using gnutls is.
> >
> > Regards,
> > Daniel
> >
> 
> I think the problem Stefan tries to fix here is this one while linking
> virsh. The calls to gcry_check_version and gcry_control are in
> libvirt.c itself.

The libvirt.c file is compiled into libvirt_driver.la, which in
turn links to libvirt.la. The libraries should always be linked
directly to the module which uses them. There are no .c files 
listed directly against libvirt.la, hence there should never be
any need for libraries to link directly against libvirt.la. The
final libvirt.la will inherit linkage for all the sub-libraries
The only real exceptions to this should be LIBXML since that is 
used in pretty much every module and its a waste of time to have
it duplicated it in all places, and the cygwin bits.
 
>   CCLD   virsh
> ../src/.libs/libvirt.so: undefined reference to `gcry_check_version'
> ../src/.libs/libvirt.so: undefined reference to `gcry_control'
> 
> Therefore, this patch adds GnuTLS to libvirt_la_{LIBADD|CFLAGS}. It
> also adds a missing dependency for the statstest.

statstest links to libvirt_test.la which links to libvirt_driver.la
so it will inherit the link to gnutls still. 

We should not need to add any libraries directly to libvirt_la_LIBADD,
always to one of the other modules first.

> diff --git a/src/Makefile.am b/src/Makefile.am
> index 5109302..eb93727 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -1001,10 +1002,10 @@ libvirt_la_LDFLAGS =
> $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \
>                     $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS)
>  libvirt_la_BUILT_LIBADD += ../gnulib/lib/libgnu.la
>  libvirt_la_LIBADD += $(LIBXML_LIBS) \
> -                   $(LIBPCAP_LIBS) $(LIBNL_LIBS) \
> +                   $(LIBPCAP_LIBS) $(LIBNL_LIBS) $(GNUTLS_LIBS) \

These two previous additions of LIBPCAP and LIBNL are both wrong
too. They should be linking to libvirt_driver_nwfilter.la instead.

>                     $(DRIVER_MODULE_LIBS) \
>                     $(CYGWIN_EXTRA_LIBADD)
> -libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT
> +libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) $(GNUTLS_CFLAGS) -DIN_LIBVIRT

This should also be against libvirt_driver_la_CFLAGS

>  # Because we specify libvirt_la_DEPENDENCIES for $(LIBVIRT_SYMBOL_FILE), we
>  # lose automake's automatic dependencies on an appropriate subset of
>  # $(libvirt_la_LIBADD).  But we were careful to create
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index a3661f6..574d0cf 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -329,7 +329,7 @@ nodeinfotest_LDADD = $(LDADDS)
> 
>  statstest_SOURCES = \
>         statstest.c testutils.h testutils.c
> -statstest_LDADD = $(LDADDS)
> +statstest_LDADD = ../src/libvirt_driver_xen.la $(LDADDS)

This bit is fine, since statstest has a direct dependancy on Xen.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|