[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 06/10] Qemu Monitor API entry point.



On Tue, Apr 27, 2010 at 04:16:15PM -0400, Chris Lalancette wrote:
> On 04/22/2010 08:27 AM, Daniel P. Berrange wrote:
> >> --- a/src/Makefile.am
> >> +++ b/src/Makefile.am
> >> @@ -32,7 +32,7 @@ if WITH_NETWORK
> >>  UUID=$(shell uuidgen 2>/dev/null)
> >>  endif
> >>  
> >> -lib_LTLIBRARIES = libvirt.la
> >> +lib_LTLIBRARIES = libvirt.la libvirt-qemu.la
> >>  
> >>  moddir = $(libdir)/libvirt/drivers
> >>  mod_LTLIBRARIES =
> >> @@ -947,6 +947,12 @@ libvirt_test_la_LIBADD = $(libvirt_la_LIBADD)
> >>  libvirt_test_la_LDFLAGS = $(test_LDFLAGS)
> >>  libvirt_test_la_CFLAGS = $(COVERAGE_CFLAGS)
> >>  
> >> +libvirt_qemu_la_SOURCES = libvirt-qemu.c
> >> +libvirt_qemu_la_LDFLAGS = $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS)
> >> +libvirt_qemu_la_CFLAGS = $(COVERAGE_CFLAGS)
> >> +libvirt_qemu_la_LIBADD = libvirt_util.la libvirt_driver_qemu.la \
> >> +			libvirt_driver_remote.la $(CYGWIN_EXTRA_LIBADD) \
> >> +			../gnulib/lib/libgnu.la
> > 
> > This is still going to cause duplicate copies of the code to be
> > statically linked in. If you just mention 'libvirt.la', then it
> > should dynamically link to the libvirt.so with no duplication.
> > You may need to export some more private symbols but that's not
> > a big issue.
> 
> I actually tried to do this, but I can't get it to work.  It certainly
> might be my unfamiliarity with the auto* tools, but doing something like:
> 
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 3994f00..04aa203 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -989,11 +989,11 @@ libvirt_test_la_LDFLAGS = $(test_LDFLAGS)
>  libvirt_test_la_CFLAGS = $(COVERAGE_CFLAGS)
>  
>  libvirt_qemu_la_SOURCES = libvirt-qemu.c
> -libvirt_qemu_la_LDFLAGS = $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS)
> +libvirt_qemu_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \
> +                          -version-info $(LIBVIRT_VERSION_INFO) \
> +                          $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS)
>  libvirt_qemu_la_CFLAGS = $(COVERAGE_CFLAGS)
> -libvirt_qemu_la_LIBADD = libvirt_util.la libvirt_driver_qemu.la \
> -                       libvirt_driver_remote.la $(CYGWIN_EXTRA_LIBADD) \
> -                       ../gnulib/lib/libgnu.la
> +libvirt_qemu_la_LIBADD = libvirt.la
>  
>  libexec_PROGRAMS =
>  
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 746327d..4b88a74 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -706,6 +706,7 @@ virReportSystemErrorFull;
>  virReportOOMErrorFull;
>  virStrerror;
>  virSetError;
> +virDispatchError;
>  
>  
>  # xml.h
> @@ -723,3 +724,11 @@ virXPathLongLong;
>  virXPathULongLong;
>  virXPathLongHex;
>  virXPathULongHex;
> +
> +
> +# remote_driver.h
> +remoteQemuMonitorCommand;
> +
> +
> +# qemu_driver.h
> +qemuMonitorCommand;
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index b4db904..22d53f9 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -393,5 +393,8 @@ LIBVIRT_0.8.0 {
>          virDomainSnapshotFree;
>  } LIBVIRT_0.7.7;
>  
> -
> +LIBVIRT_0.8.1 {
> +    global:
> +        virDomainQemuMonitorCommand;
> +} LIBVIRT_0.8.0;
> 
> Causes the build to fail when linking virsh:
> 
>   CCLD   virsh
> ../src/.libs/libvirt-qemu.so: undefined reference to `qemuMonitorCommand'
> collect2: ld returned 1 exit status
> 
> Indeed, if I nm src/.libs/libvirt-qemu.so:
> 
> [clalance localhost libvirt (qemu_monitor)]$ nm src/.libs/libvirt-qemu.so | grep Monitor
>          U qemuMonitorCommand
>          U remoteQemuMonitorCommand@@LIBVIRT_PRIVATE_0.8.0
> 000010b0 T virDomainQemuMonitorCommand
> 
> Any suggestions about what I'm doing wrong?

Have you re-ordered your patches ? There is a qemuMonitorCommand  defined
in src/qemu/qemu_monitor_text.c but that is static. The qemuMonitorCommand
you actually want is not added till your next patch. So perhaps your old
code was pullin in the wrong definition ?

It'd be good to rename this to reflect the public API name and avoid the
clash

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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]