[prev in list] [next in list] [prev in thread] [next in thread] 

List:       libvir-list
Subject:    Re: [libvirt] [PATCH 1/2] Make libvirt.so include the RPC server code
From:       Eric Blake <eblake () redhat ! com>
Date:       2011-09-30 21:00:02
Message-ID: 4E862DD2.7000902 () redhat ! com
[Download RAW message or body]

On 09/30/2011 07:54 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"<berrange@redhat.com>
> 
> To avoid static linking libvirtd to the RPC server code, which
> then prevents sane introduction of DTrace probes, put it all
> in the libvirt.so, and export it
> 
> * daemon/Makefile.am: Don't link to RPC libraries
> * src/Makefile.am: Link all RPC libraries to libvirt.so
> * src/libvirt_private.syms: Export all RPC functions
> ---
> daemon/Makefile.am       |    2 -
> src/Makefile.am          |    2 +-
> src/libvirt_private.syms |   76 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 77 insertions(+), 3 deletions(-)
> 

> +++ b/src/Makefile.am
> @@ -612,7 +612,7 @@ libvirt_driver_remote_la_CFLAGS =				\
> 		-I@top_srcdir@/src/rpc				\
> 		$(AM_CFLAGS)
> libvirt_driver_remote_la_LDFLAGS = $(AM_LDFLAGS)
> -libvirt_driver_remote_la_LIBADD = $(GNUTLS_LIBS) libvirt-net-rpc-client.la \
> libvirt-net-rpc.la +libvirt_driver_remote_la_LIBADD = $(GNUTLS_LIBS) \
> libvirt-net-rpc-client.la libvirt-net-rpc-server.la libvirt-net-rpc.la

Use \-newline wrapping to make this not be quite so long.


> if WITH_DRIVER_MODULES
> libvirt_driver_remote_la_LIBADD += ../gnulib/lib/libgnu.la
> libvirt_driver_remote_la_LDFLAGS += -module -avoid-version
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c2a3fab..e086253 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1162,6 +1162,82 @@ virFileDirectFdNew;
> virFileFclose;
> virFileFdopen;
> 
> +# rpc

Hmm, that's not really the .h where they are declared, although it is 
the right directory.  As is, sticking 'rpc' between 'virfile.h' and 
'virpidfile.h' doesn't sort very well.  It shouldn't be too much more 
effort to break this into multiple sections, one for each relevant .h...

virnetmessage.h:

> +virNetMessageClear;
> +virNetMessageEncodeHeader;
> +virNetMessageEncodePayload;
> +virNetMessageFree;
> +virNetMessageNew;
> +virNetMessageQueuePush;
> +virNetMessageQueueServe;
> +virNetMessageSaveError;

virnetsaslcontext.h

> +virNetSASLContextCheckIdentity;
> +virNetSASLContextNewServer;
> +virNetSASLSessionExtKeySize;
> +virNetSASLSessionFree;
> +virNetSASLSessionGetIdentity;
> +virNetSASLSessionGetKeySize;
> +virNetSASLSessionListMechanisms;
> +virNetSASLSessionNewServer;
> +virNetSASLSessionSecProps;
> +virNetSASLSessionServerStart;
> +virNetSASLSessionServerStep;

virnetserver.h

> +virNetServerAddProgram;
> +virNetServerAddService;
> +virNetServerAddSignalHandler;
> +virNetServerAutoShutdown;

virnetserverclient.h

> +virNetServerClientAddFilter;
> +virNetServerClientClose;
> +virNetServerClientDelayedClose;
> +virNetServerClientFree;
> +virNetServerClientGetAuth;
> +virNetServerClientGetFD;
> +virNetServerClientGetLocalIdentity;
> +virNetServerClientGetPrivateData;
> +virNetServerClientGetReadonly;
> +virNetServerClientGetTLSKeySize;
> +virNetServerClientHasTLSSession;
> +virNetServerClientImmediateClose;
> +virNetServerClientIsSecure;
> +virNetServerClientLocalAddrString;
> +virNetServerClientRef;
> +virNetServerClientRemoteAddrString;
> +virNetServerClientRemoveFilter;
> +virNetServerClientSendMessage;
> +virNetServerClientSetCloseHook;
> +virNetServerClientSetIdentity;
> +virNetServerClientSetPrivateData;
> +virNetServerClientSetSASLSession;

move these to be with the rest of virnetserver.h

> +virNetServerClose;
> +virNetServerFree;
> +virNetServerIsPrivileged;
> +virNetServerNew;

virnetserverprogram.h

> +virNetServerProgramFree;
> +virNetServerProgramGetID;
> +virNetServerProgramGetVersion;
> +virNetServerProgramMatches;
> +virNetServerProgramNew;
> +virNetServerProgramRef;
> +virNetServerProgramSendReplyError;
> +virNetServerProgramSendStreamData;
> +virNetServerProgramSendStreamError;
> +virNetServerQuit;
> +virNetServerRef;
> +virNetServerRun;

virnetserverservice.h

> +virNetServerServiceFree;
> +virNetServerServiceNewTCP;
> +virNetServerServiceNewUNIX;
> +virNetServerUpdateServices;

virnetsocket.h

> +virNetSocketDupFD;
> +virNetSocketFree;
> +virNetSocketGetFD;
> +virNetSocketListen;
> +virNetSocketNewConnectTCP;
> +virNetSocketNewListenUNIX;

virnettlscontext.h

> +virNetTLSContextFree;
> +virNetTLSContextNewServer;
> +virNetTLSContextNewServerPath;
> +
> 
> # virpidfile.h
> virPidFileAcquire;


But all of those are just layout nits, not technical objections, so I'm 
okay giving:

ACK with changes

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[prev in list] [next in list] [prev in thread] [next in thread]