Re: [libvirt] [PATCH 4/4] Fix symbol exports remove duplicated libvirt_util.la linkage
On Thu, Oct 14, 2010 at 03:45:01PM +0100, Daniel P. Berrange wrote: On Thu, Oct 14, 2010 at 04:22:29PM +0200, Daniel Veillard wrote: On Tue, Oct 12, 2010 at 06:32:18PM +0100, Daniel P. Berrange wrote: From: Miloslav Trmač m...@redhat.com The libvirt_util.la library was mistakenly linked into libvirtd directly. Since libvirt_util.la is already linked to libvirt.so, this resulted in libvirtd getting two copies of the code and more critically 2 copies of static global variables. Testing in turn exposed a issue with loadable modules. The gnulib replacement functions are not exported to loadable modules. Rather than trying to figure out the name sof all gnulib functions export them, just linkage all loadable modules against libgnu.la statically. * daemon/Makefile.am: Remove linkage of libvirt_util.la and libvirt_driver.la * src/Makefile.am: Link driver modules against libgnu.la * src/libvirt.c: Don't try to load modules which were compiled out * src/libvirt_private.syms: Export all other internal symbols that are required by drivers Hum, weird, I tried to o a make rpm with that patch and got a linking error due to multiple definitions coming from gnulib: CCLD libvirt_lxc CCLD libvirt.la copying selected object files to avoid basename conflicts... ../gnulib/lib/.libs/libgnu.a(areadlink.o): In function `areadlink': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: multiple definition of `areadlink' ./.libs/libvirt_driver_phyp.a(areadlink.o):/u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: first defined here ../gnulib/lib/.libs/libgnu.a(base64.o): In function `base64_encode': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/base64.c:79: multiple definition of `base64_encode' So patches 1-3 look fine to me but that one seems to still have a problem, This was a simple mistake. One of my changes to the phyp driver link line was not properly protected by a 'if WITH_DRIVER_MODULES' Okay, ACK to a rebased patch set once this error is fixed, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] Fix symbol exports remove duplicated libvirt_util.la linkage
On Tue, Oct 12, 2010 at 06:32:18PM +0100, Daniel P. Berrange wrote: From: Miloslav Trmač m...@redhat.com The libvirt_util.la library was mistakenly linked into libvirtd directly. Since libvirt_util.la is already linked to libvirt.so, this resulted in libvirtd getting two copies of the code and more critically 2 copies of static global variables. Testing in turn exposed a issue with loadable modules. The gnulib replacement functions are not exported to loadable modules. Rather than trying to figure out the name sof all gnulib functions export them, just linkage all loadable modules against libgnu.la statically. * daemon/Makefile.am: Remove linkage of libvirt_util.la and libvirt_driver.la * src/Makefile.am: Link driver modules against libgnu.la * src/libvirt.c: Don't try to load modules which were compiled out * src/libvirt_private.syms: Export all other internal symbols that are required by drivers Hum, weird, I tried to o a make rpm with that patch and got a linking error due to multiple definitions coming from gnulib: CCLD libvirt_lxc CCLD libvirt.la copying selected object files to avoid basename conflicts... ../gnulib/lib/.libs/libgnu.a(areadlink.o): In function `areadlink': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: multiple definition of `areadlink' ./.libs/libvirt_driver_phyp.a(areadlink.o):/u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: first defined here ../gnulib/lib/.libs/libgnu.a(base64.o): In function `base64_encode': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/base64.c:79: multiple definition of `base64_encode' So patches 1-3 look fine to me but that one seems to still have a problem, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] Fix symbol exports remove duplicated libvirt_util.la linkage
On Thu, Oct 14, 2010 at 04:22:29PM +0200, Daniel Veillard wrote: On Tue, Oct 12, 2010 at 06:32:18PM +0100, Daniel P. Berrange wrote: From: Miloslav Trmač m...@redhat.com The libvirt_util.la library was mistakenly linked into libvirtd directly. Since libvirt_util.la is already linked to libvirt.so, this resulted in libvirtd getting two copies of the code and more critically 2 copies of static global variables. Testing in turn exposed a issue with loadable modules. The gnulib replacement functions are not exported to loadable modules. Rather than trying to figure out the name sof all gnulib functions export them, just linkage all loadable modules against libgnu.la statically. * daemon/Makefile.am: Remove linkage of libvirt_util.la and libvirt_driver.la * src/Makefile.am: Link driver modules against libgnu.la * src/libvirt.c: Don't try to load modules which were compiled out * src/libvirt_private.syms: Export all other internal symbols that are required by drivers Hum, weird, I tried to o a make rpm with that patch and got a linking error due to multiple definitions coming from gnulib: CCLD libvirt_lxc CCLD libvirt.la copying selected object files to avoid basename conflicts... ../gnulib/lib/.libs/libgnu.a(areadlink.o): In function `areadlink': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: multiple definition of `areadlink' ./.libs/libvirt_driver_phyp.a(areadlink.o):/u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/areadlink.c:58: first defined here ../gnulib/lib/.libs/libgnu.a(base64.o): In function `base64_encode': /u/veillard/rpms/BUILD/libvirt-0.8.4/gnulib/lib/base64.c:79: multiple definition of `base64_encode' So patches 1-3 look fine to me but that one seems to still have a problem, This was a simple mistake. One of my changes to the phyp driver link line was not properly protected by a 'if WITH_DRIVER_MODULES' 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] Fix symbol exports remove duplicated libvirt_util.la linkage
From: Miloslav Trmač m...@redhat.com The libvirt_util.la library was mistakenly linked into libvirtd directly. Since libvirt_util.la is already linked to libvirt.so, this resulted in libvirtd getting two copies of the code and more critically 2 copies of static global variables. Testing in turn exposed a issue with loadable modules. The gnulib replacement functions are not exported to loadable modules. Rather than trying to figure out the name sof all gnulib functions export them, just linkage all loadable modules against libgnu.la statically. * daemon/Makefile.am: Remove linkage of libvirt_util.la and libvirt_driver.la * src/Makefile.am: Link driver modules against libgnu.la * src/libvirt.c: Don't try to load modules which were compiled out * src/libvirt_private.syms: Export all other internal symbols that are required by drivers --- daemon/Makefile.am |6 ++ src/Makefile.am | 24 +++- src/libvirt.c| 14 ++ src/libvirt_private.syms | 45 + 4 files changed, 80 insertions(+), 9 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index b020b77..987133c 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -99,11 +99,9 @@ libvirtd_LDADD = \ $(SASL_LIBS)\ $(POLKIT_LIBS) -libvirtd_LDADD += ../src/libvirt_util.la ../src/libvirt-qemu.la +libvirtd_LDADD += ../src/libvirt-qemu.la -if WITH_DRIVER_MODULES - libvirtd_LDADD += ../src/libvirt_driver.la -else +if ! WITH_DRIVER_MODULES if WITH_QEMU libvirtd_LDADD += ../src/libvirt_driver_qemu.la endif diff --git a/src/Makefile.am b/src/Makefile.am index 8ec8230..521246c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -82,7 +82,7 @@ UTIL_SOURCES = \ util/uuid.c util/uuid.h \ util/util.c util/util.h \ util/xml.c util/xml.h \ - util/virtaudit.c util/virtaudit.h \ + util/virtaudit.c util/virtaudit.h \ util/virterror.c util/virterror_internal.h EXTRA_DIST += util/threads-pthread.c util/threads-win32.c @@ -566,6 +566,7 @@ libvirt_driver_xen_la_CFLAGS = \ libvirt_driver_xen_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_xen_la_LIBADD = $(XEN_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_xen_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_xen_la_LDFLAGS += -module -avoid-version endif libvirt_driver_xen_la_SOURCES = $(XEN_DRIVER_SOURCES) @@ -578,7 +579,8 @@ else noinst_LTLIBRARIES += libvirt_driver_phyp.la libvirt_la_BUILT_LIBADD += libvirt_driver_phyp.la endif -libvirt_driver_phyp_la_LIBADD = $(LIBSSH2_LIBS) +libvirt_driver_phyp_la_LIBADD = ../gnulib/lib/libgnu.la +libvirt_driver_phyp_la_LIBADD += $(LIBSSH2_LIBS) libvirt_driver_phyp_la_CFLAGS = $(LIBSSH2_CFLAGS) \ -...@top_srcdir@/src/conf $(AM_CFLAGS) libvirt_driver_phyp_la_SOURCES = $(PHYP_DRIVER_SOURCES) @@ -594,6 +596,7 @@ endif libvirt_driver_openvz_la_CFLAGS = \ -...@top_srcdir@/src/conf $(AM_CFLAGS) if WITH_DRIVER_MODULES +libvirt_driver_openvz_la_LIBADD = ../gnulib/lib/libgnu.la libvirt_driver_openvz_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) endif libvirt_driver_openvz_la_SOURCES = $(OPENVZ_DRIVER_SOURCES) @@ -608,10 +611,11 @@ libvirt_la_BUILT_LIBADD += libvirt_driver_vbox.la endif libvirt_driver_vbox_la_CFLAGS = \ -...@top_srcdir@/src/conf $(AM_CFLAGS) +libvirt_driver_vbox_la_LIBADD = $(DLOPEN_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_vbox_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_vbox_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) endif -libvirt_driver_vbox_la_LIBADD = $(DLOPEN_LIBS) libvirt_driver_vbox_la_SOURCES = $(VBOX_DRIVER_SOURCES) endif @@ -627,6 +631,7 @@ libvirt_driver_xenapi_la_CFLAGS = $(LIBXENSERVER_CFLAGS) $(LIBCURL_CFLAGS) \ libvirt_driver_xenapi_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_xenapi_la_LIBADD = $(LIBXENSERVER_LIBS) $(LIBCURL_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_xenapi_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_xenapi_la_LDFLAGS += -module -avoid-version endif libvirt_driver_xenapi_la_SOURCES = $(XENAPI_DRIVER_SOURCES) @@ -645,6 +650,7 @@ libvirt_driver_qemu_la_CFLAGS = $(NUMACTL_CFLAGS) \ libvirt_driver_qemu_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_qemu_la_LIBADD = $(NUMACTL_LIBS) if WITH_DRIVER_MODULES +libvirt_driver_qemu_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_qemu_la_LDFLAGS += -module -avoid-version endif libvirt_driver_qemu_la_SOURCES = $(QEMU_DRIVER_SOURCES) @@ -670,6 +676,7 @@ endif libvirt_driver_lxc_la_CFLAGS = \ -...@top_srcdir@/src/conf $(AM_CFLAGS) if WITH_DRIVER_MODULES
Re: [libvirt] [PATCH 4/4] Fix symbol exports remove duplicated libvirt_util.la linkage
On 10/12/2010 11:32 AM, Daniel P. Berrange wrote: @@ -708,6 +735,18 @@ virFileFindMountPoint; virFileWaitForDevices; virFileMatchesNameSuffix; virArgvToString; +virStrcpy; +virStrncpy; +virBuildPathInternal; +virFileStripSuffix; +virFileOperation; +virFork; +virRandom; +virRandomInitialize; +virDirCreate; +virIndexToDiskName; +virHexToBin; This is a duplicate with the patch I committed earlier today (I only noticed one function, but you obviously linked shard modules and found a lot more). Should we be running this through sort | uniq -c and detecting any duplicate lines as part of 'make syntax-check'? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
