[libvirt] [PATCH v3] nwfilter: fix loadable module support
Following Daniel Berrange's multiple helpful suggestions for improving this patch and introducing another driver interface, I now wrote the below patch where the nwfilter driver registers the functions to instantiate and teardown the nwfilters with a function in conf/domain_nwfilter.c called virDomainConfNWFilterRegister. Previous helper functions that were called from qemu_driver.c and qemu_conf.c were move into conf/domain_nwfilter.h with slight renaming done for consistency. Those functions now call the function expored by domain_nwfilter.c, which in turn call the functions of the new driver interface, if available. V3: no more inline functions Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- src/Makefile.am | 3 + src/conf/domain_nwfilter.c | 61 +++++++++++++++++++++++++++++++++ src/conf/domain_nwfilter.h | 43 +++++++++++++++++++++++ src/libvirt_private.syms | 5 ++ src/nwfilter/nwfilter_driver.c | 23 ++++++++++++ src/nwfilter/nwfilter_gentech_driver.h | 17 --------- src/qemu/qemu_conf.c | 17 ++++----- src/qemu/qemu_driver.c | 10 ++--- 8 files changed, 148 insertions(+), 31 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h @@ -67,21 +67,4 @@ void virNWFilterDomainFWUpdateCB(void *p const char *name ATTRIBUTE_UNUSED, void *data); - -/* tear down an interface's filter before tearing down the interface */ -static inline void -virNWFilterTearNWFilter(virDomainNetDefPtr net) { - if ((net->filter) && (net->ifname)) - virNWFilterTeardownFilter(net); -} - - -static inline void -virNWFilterTearVMNWFilters(virDomainObjPtr vm) { - int i; - - for (i = 0; i < vm->def->nnets; i++) - virNWFilterTearNWFilter(vm->def->nets[i]); -} - #endif Index: libvirt-acl/src/qemu/qemu_conf.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_conf.c +++ libvirt-acl/src/qemu/qemu_conf.c @@ -54,7 +54,7 @@ #include "network.h" #include "macvtap.h" #include "cpu/cpu.h" -#include "nwfilter/nwfilter_gentech_driver.h" +#include "domain_nwfilter.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -1514,9 +1514,10 @@ int qemudExtractVersion(struct qemud_dri /** * qemudPhysIfaceConnect: * @conn: pointer to virConnect object + * @driver: pointer to the qemud_driver * @net: pointer to he VM's interface description with direct device type - * @linkdev: The name of the physical interface to link the macvtap to - * @brmode: The mode to put the macvtap device into + * @qemuCmdFlags: flags for qemu + * @vmuuid: The UUID of the VM (needed by 802.1Qbh) * * Returns a filedescriptor on success or -1 in case of error. */ @@ -1555,7 +1556,7 @@ qemudPhysIfaceConnect(virConnectPtr conn if (rc >= 0) { if ((net->filter) && (net->ifname)) { - err = virNWFilterInstantiateFilter(conn, net); + err = virDomainConfNWFilterInstantiate(conn, net); if (err) { close(rc); rc = -1; @@ -1688,7 +1689,7 @@ qemudNetworkIfaceConnect(virConnectPtr c if (tapfd >= 0) { if ((net->filter) && (net->ifname)) { - err = virNWFilterInstantiateFilter(conn, net); + err = virDomainConfNWFilterInstantiate(conn, net); if (err) { close(tapfd); tapfd = -1; @@ -4207,7 +4208,7 @@ int qemudBuildCommandLine(virConnectPtr goto error; if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { - virNWFilterTearNWFilter(net); + virDomainConfNWFilterTeardown(net); close(tapfd); goto no_memory; } @@ -4226,7 +4227,7 @@ int qemudBuildCommandLine(virConnectPtr goto error; if (VIR_REALLOC_N(*vmfds, (*nvmfds)+1) < 0) { - virNWFilterTearNWFilter(net); + virDomainConfNWFilterTeardown(net); close(tapfd); goto no_memory; } @@ -4766,7 +4767,7 @@ int qemudBuildCommandLine(virConnectPtr virReportOOMError(); error: for (i = 0; i <= last_good_net; i++) - virNWFilterTearNWFilter(def->nets[i]); + virDomainConfNWFilterTeardown(def->nets[i]); if (vmfds && *vmfds) { for (i = 0; i < *nvmfds; i++) Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -81,7 +81,7 @@ #include "xml.h" #include "cpu/cpu.h" #include "macvtap.h" -#include "nwfilter/nwfilter_gentech_driver.h" +#include "domain_nwfilter.h" #include "hooks.h" #include "storage_file.h" @@ -3576,7 +3576,7 @@ static int qemudStartVMDaemon(virConnect VIR_FREE(progenv); if (ret == -1) /* The VM failed to start; tear filters before taps */ - virNWFilterTearVMNWFilters(vm); + virDomainConfVMNWFilterTeardown(vm); if (vmfds) { for (i = 0 ; i < nvmfds ; i++) { @@ -3668,7 +3668,7 @@ static void qemudShutdownVMDaemon(struct * reporting so we don't squash a legit error. */ orig_err = virSaveLastError(); - virNWFilterTearVMNWFilters(vm); + virDomainConfVMNWFilterTeardown(vm); if (driver->macFilter) { def = vm->def; @@ -7640,7 +7640,7 @@ cleanup: VIR_WARN0("Unable to release PCI address on NIC"); if (ret != 0) - virNWFilterTearNWFilter(net); + virDomainConfNWFilterTeardown(net); VIR_FREE(nicstr); VIR_FREE(netstr); @@ -8609,7 +8609,7 @@ qemudDomainDetachNetDevice(struct qemud_ } qemuDomainObjExitMonitorWithDriver(driver, vm); - virNWFilterTearNWFilter(detach); + virDomainConfNWFilterTeardown(detach); #if WITH_MACVTAP if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) { Index: libvirt-acl/src/Makefile.am =================================================================== --- libvirt-acl.orig/src/Makefile.am +++ libvirt-acl/src/Makefile.am @@ -97,7 +97,8 @@ DRIVER_SOURCES = \ # Domain driver generic impl APIs DOMAIN_CONF_SOURCES = \ conf/capabilities.c conf/capabilities.h \ - conf/domain_conf.c conf/domain_conf.h + conf/domain_conf.c conf/domain_conf.h \ + conf/domain_nwfilter.c conf/domain_nwfilter.h DOMAIN_EVENT_SOURCES = \ conf/domain_event.c conf/domain_event.h Index: libvirt-acl/src/conf/domain_nwfilter.h =================================================================== --- /dev/null +++ libvirt-acl/src/conf/domain_nwfilter.h @@ -0,0 +1,43 @@ +/* + * domain_nwfilter.h: + * + * Copyright (C) 2010 IBM Corporation + * Copyright (C) 2010 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Stefan Berger <stefanb@us.ibm.com> + */ +#ifndef DOMAIN_NWFILTER_H +# define DOMAIN_NWFILTER_H + +typedef int (*virDomainConfInstantiateNWFilter)(virConnectPtr conn, + virDomainNetDefPtr net); +typedef void (*virDomainConfTeardownNWFilter)(virDomainNetDefPtr net); + +typedef struct { + virDomainConfInstantiateNWFilter instantiateFilter; + virDomainConfTeardownNWFilter teardownFilter; +} virDomainConfNWFilterDriver; +typedef virDomainConfNWFilterDriver *virDomainConfNWFilterDriverPtr; + +void virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver); + +int virDomainConfNWFilterInstantiate(virConnectPtr conn, + virDomainNetDefPtr net); +void virDomainConfNWFilterTeardown(virDomainNetDefPtr net); +void virDomainConfVMNWFilterTeardown(virDomainObjPtr vm); + +#endif /* DOMAIN_NWFILTER_H */ Index: libvirt-acl/src/conf/domain_nwfilter.c =================================================================== --- /dev/null +++ libvirt-acl/src/conf/domain_nwfilter.c @@ -0,0 +1,61 @@ +/* + * domain_nwfilter.c: + * + * Copyright (C) 2010 IBM Corporation + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Stefan Berger <stefanb@us.ibm.com> + */ + +#include <config.h> + +#include "internal.h" + +#include "datatypes.h" +#include "domain_conf.h" +#include "domain_nwfilter.h" + +static virDomainConfNWFilterDriverPtr nwfilterDriver; + +void +virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver) { + nwfilterDriver = driver; +} + +int +virDomainConfNWFilterInstantiate(virConnectPtr conn, + virDomainNetDefPtr net) { + if (nwfilterDriver != NULL) + return nwfilterDriver->instantiateFilter(conn, net); + /* driver module not available -- don't indicate failure */ + return 0; +} + +void +virDomainConfNWFilterTeardown(virDomainNetDefPtr net) { + if (nwfilterDriver != NULL) + nwfilterDriver->teardownFilter(net); +} + +void +virDomainConfVMNWFilterTeardown(virDomainObjPtr vm) { + int i; + + if (nwfilterDriver != NULL) { + for (i = 0; i < vm->def->nnets; i++) + virDomainConfNWFilterTeardown(vm->def->nets[i]); + } +} Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -264,6 +264,11 @@ virDomainEventDispatchDefaultFunc; virDomainEventDispatch; virDomainEventQueueDispatch; +# domain_nwfilter.h +virDomainConfNWFilterRegister; +virDomainConfNWFilterInstantiate; +virDomainConfNWFilterTeardown; +virDomainConfVMNWFilterTeardown; # ebtables.h ebtablesAddForwardAllowIn; Index: libvirt-acl/src/nwfilter/nwfilter_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_driver.c @@ -33,6 +33,7 @@ #include "datatypes.h" #include "memory.h" #include "domain_conf.h" +#include "domain_nwfilter.h" #include "nwfilter_driver.h" #include "nwfilter_gentech_driver.h" @@ -410,6 +411,20 @@ cleanup: } +static int +nwfilterInstantiateFilter(virConnectPtr conn, + virDomainNetDefPtr net) { + return virNWFilterInstantiateFilter(conn, net); +} + + +static void +nwfilterTeardownFilter(virDomainNetDefPtr net) { + if ((net->ifname) && (net->filter)) + virNWFilterTeardownFilter(net); +} + + static virNWFilterDriver nwfilterDriver = { .name = "nwfilter", .open = nwfilterOpen, @@ -432,8 +447,16 @@ static virStateDriver stateDriver = { .active = nwfilterDriverActive, }; + +static virDomainConfNWFilterDriver domainNWFilterDriver = { + .instantiateFilter = nwfilterInstantiateFilter, + .teardownFilter = nwfilterTeardownFilter, +}; + + int nwfilterRegister(void) { virRegisterNWFilterDriver(&nwfilterDriver); virRegisterStateDriver(&stateDriver); + virDomainConfNWFilterRegister(&domainNWFilterDriver); return 0; }
On Wed, Jun 16, 2010 at 11:33:48AM -0400, Stefan Berger wrote:
Following Daniel Berrange's multiple helpful suggestions for improving this patch and introducing another driver interface, I now wrote the below patch where the nwfilter driver registers the functions to instantiate and teardown the nwfilters with a function in conf/domain_nwfilter.c called virDomainConfNWFilterRegister. Previous helper functions that were called from qemu_driver.c and qemu_conf.c were move into conf/domain_nwfilter.h with slight renaming done for consistency. Those functions now call the function expored by domain_nwfilter.c, which in turn call the functions of the new driver interface, if available.
V3: no more inline functions
Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
--- src/Makefile.am | 3 + src/conf/domain_nwfilter.c | 61 +++++++++++++++++++++++++++++++++ src/conf/domain_nwfilter.h | 43 +++++++++++++++++++++++ src/libvirt_private.syms | 5 ++ src/nwfilter/nwfilter_driver.c | 23 ++++++++++++ src/nwfilter/nwfilter_gentech_driver.h | 17 --------- src/qemu/qemu_conf.c | 17 ++++----- src/qemu/qemu_driver.c | 10 ++--- 8 files changed, 148 insertions(+), 31 deletions(-)
ACK, this looks good now 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 :|
On 06/21/2010 11:33 AM, Daniel P. Berrange wrote:
On Wed, Jun 16, 2010 at 11:33:48AM -0400, Stefan Berger wrote:
Following Daniel Berrange's multiple helpful suggestions for improving this patch and introducing another driver interface, I now wrote the below patch where the nwfilter driver registers the functions to instantiate and teardown the nwfilters with a function in conf/domain_nwfilter.c called virDomainConfNWFilterRegister. Previous helper functions that were called from qemu_driver.c and qemu_conf.c were move into conf/domain_nwfilter.h with slight renaming done for consistency. Those functions now call the function expored by domain_nwfilter.c, which in turn call the functions of the new driver interface, if available.
V3: no more inline functions
Signed-off-by: Stefan Berger<stefanb@us.ibm.com>
--- src/Makefile.am | 3 + src/conf/domain_nwfilter.c | 61 +++++++++++++++++++++++++++++++++ src/conf/domain_nwfilter.h | 43 +++++++++++++++++++++++ src/libvirt_private.syms | 5 ++ src/nwfilter/nwfilter_driver.c | 23 ++++++++++++ src/nwfilter/nwfilter_gentech_driver.h | 17 --------- src/qemu/qemu_conf.c | 17 ++++----- src/qemu/qemu_driver.c | 10 ++--- 8 files changed, 148 insertions(+), 31 deletions(-)
ACK, this looks good now
Thanks. Pushed it now. The only patch that I need now to compile with --with-driver-modules (using --without-xen) is this here: 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 Regards, Stefan
On Mon, Jun 21, 2010 at 02:27:36PM -0400, Stefan Berger wrote:
On 06/21/2010 11:33 AM, Daniel P. Berrange wrote:
On Wed, Jun 16, 2010 at 11:33:48AM -0400, Stefan Berger wrote:
Following Daniel Berrange's multiple helpful suggestions for improving this patch and introducing another driver interface, I now wrote the below patch where the nwfilter driver registers the functions to instantiate and teardown the nwfilters with a function in conf/domain_nwfilter.c called virDomainConfNWFilterRegister. Previous helper functions that were called from qemu_driver.c and qemu_conf.c were move into conf/domain_nwfilter.h with slight renaming done for consistency. Those functions now call the function expored by domain_nwfilter.c, which in turn call the functions of the new driver interface, if available.
V3: no more inline functions
Signed-off-by: Stefan Berger<stefanb@us.ibm.com>
--- src/Makefile.am | 3 + src/conf/domain_nwfilter.c | 61 +++++++++++++++++++++++++++++++++ src/conf/domain_nwfilter.h | 43 +++++++++++++++++++++++ src/libvirt_private.syms | 5 ++ src/nwfilter/nwfilter_driver.c | 23 ++++++++++++ src/nwfilter/nwfilter_gentech_driver.h | 17 --------- src/qemu/qemu_conf.c | 17 ++++----- src/qemu/qemu_driver.c | 10 ++--- 8 files changed, 148 insertions(+), 31 deletions(-)
ACK, this looks good now
Thanks. Pushed it now.
The only patch that I need now to compile with --with-driver-modules (using --without-xen) is this here:
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 -- |: 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 :|
2010/6/22 Daniel P. Berrange <berrange@redhat.com>:
On Mon, Jun 21, 2010 at 02:27:36PM -0400, Stefan Berger wrote:
On 06/21/2010 11:33 AM, Daniel P. Berrange wrote:
On Wed, Jun 16, 2010 at 11:33:48AM -0400, Stefan Berger wrote:
Following Daniel Berrange's multiple helpful suggestions for improving this patch and introducing another driver interface, I now wrote the below patch where the nwfilter driver registers the functions to instantiate and teardown the nwfilters with a function in conf/domain_nwfilter.c called virDomainConfNWFilterRegister. Previous helper functions that were called from qemu_driver.c and qemu_conf.c were move into conf/domain_nwfilter.h with slight renaming done for consistency. Those functions now call the function expored by domain_nwfilter.c, which in turn call the functions of the new driver interface, if available.
V3: no more inline functions
Signed-off-by: Stefan Berger<stefanb@us.ibm.com>
--- src/Makefile.am | 3 + src/conf/domain_nwfilter.c | 61 +++++++++++++++++++++++++++++++++ src/conf/domain_nwfilter.h | 43 +++++++++++++++++++++++ src/libvirt_private.syms | 5 ++ src/nwfilter/nwfilter_driver.c | 23 ++++++++++++ src/nwfilter/nwfilter_gentech_driver.h | 17 --------- src/qemu/qemu_conf.c | 17 ++++----- src/qemu/qemu_driver.c | 10 ++--- 8 files changed, 148 insertions(+), 31 deletions(-)
ACK, this looks good now
Thanks. Pushed it now.
The only patch that I need now to compile with --with-driver-modules (using --without-xen) is this here:
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. 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. 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) \ $(DRIVER_MODULE_LIBS) \ $(CYGWIN_EXTRA_LIBADD) -libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT +libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) $(GNUTLS_CFLAGS) -DIN_LIBVIRT # 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) if WITH_SECDRIVER_SELINUX seclabeltest_SOURCES = \ This fixes compilation, but there are still runtime errors about undefined symbols. I'll post a patch for this soon. Matthias
On Tue, Jun 22, 2010 at 12:48:37PM +0200, Matthias Bolte wrote:
2010/6/22 Daniel P. Berrange <berrange@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 :|
2010/6/22 Daniel P. Berrange <berrange@redhat.com>:
On Tue, Jun 22, 2010 at 12:48:37PM +0200, Matthias Bolte wrote:
2010/6/22 Daniel P. Berrange <berrange@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.
Ah, okay. My fault, I didn't look it up properly.
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.
Okay. I'll fix that too. Matthias
On 06/22/2010 07:11 AM, Daniel P. Berrange wrote:
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.
LIBNL is an addition due to macvtap.c rather than nwfilter. Stefan
On Tue, Jun 22, 2010 at 08:00:14AM -0400, Stefan Berger wrote:
On 06/22/2010 07:11 AM, Daniel P. Berrange wrote:
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.
LIBNL is an addition due to macvtap.c rather than nwfilter.
Ah, ok that one should be against libvirt_util_la_LIBADD then 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 :|
participants (3)
-
Daniel P. Berrange -
Matthias Bolte -
Stefan Berger