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

On Tue, Jun 22, 2010 at 12:48:37PM +0200, Matthias Bolte wrote: > 2010/6/22 Daniel P. Berrange <berrange(a)redhat.com&gt;: > > 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.