[libvirt] [PATCH 09/10] Introduce generic RPC client objects

[Eric Blake]

On 06/22/2011 09:33 AM, Daniel P. Berrange wrote:
> To facilitate creation of new clients using XDR RPC services,
> pull alot of the remote driver code into a set of reusable

s/alot/a lot/

> objects.
> 
>  - virNetClient: Encapsulates a socket connection to a
>    remote RPC server. Handles all the network I/O for
>    reading/writing RPC messages. Delegates RPC encoding
>    and decoding to the registered programs
> 
>  - virNetClientProgram: Handles processing and dispatch
>    of RPC messages for a single RPC (program,version).
>    A program can register to receive async events
>    from a client
> 
>  - virNetClientStream: Handles generic I/O stream
>    integration to RPC layer
> 
> Each new client program now merely needs to define the list of
> RPC procedures & events it wants and their handlers. It does
> not need to deal with any of the network I/O functionality at
> all.

> +++ b/src/Makefile.am
> @@ -1187,7 +1187,7 @@ else
>  EXTRA_DIST += $(LOCK_DRIVER_SANLOCK_SOURCES)
>  endif
>  
> -noinst_LTLIBRARIES += libvirt-net-rpc.la libvirt-net-rpc-server.la
> +noinst_LTLIBRARIES += libvirt-net-rpc.la libvirt-net-rpc-server.la libvirt-net-rpc-client.la
>  
>  libvirt_net_rpc_la_SOURCES = \
>  	rpc/virnetmessage.h rpc/virnetmessage.c \
> @@ -1237,6 +1237,18 @@ libvirt_net_rpc_server_la_LDFLAGS = \
>  libvirt_net_rpc_server_la_LIBADD = \
>  			$(CYGWIN_EXTRA_LIBADD)
>  
> +libvirt_net_rpc_client_la_SOURCES = \
> +	rpc/virnetclientprogram.h rpc/virnetclientprogram.c \
> +	rpc/virnetclientstream.h rpc/virnetclientstream.c \
> +	rpc/virnetclient.h rpc/virnetclient.c
> +libvirt_net_rpc_client_la_CFLAGS = \
> +			$(AM_CFLAGS)
> +libvirt_net_rpc_client_la_LDFLAGS = \
> +			$(AM_LDFLAGS) \
> +			$(CYGWIN_EXTRA_LDFLAGS) \
> +			$(MINGW_EXTRA_LDFLAGS)l

There's that 'l' again.  Copy and paste strikes hard.  :)

> +
> +struct _virNetClientCall {
> +    int mode;
> +
> +    virNetMessagePtr msg;
> +    int expectReply;

bool?

> +static virNetClientPtr virNetClientNew(virNetSocketPtr sock,
> +                                       const char *hostname)
> +{
> +    virNetClientPtr client;
> +    int wakeupFD[2] = { -1, -1 };
> +
> +    if (pipe(wakeupFD) < 0) {

pipe2(wakeupFD, O_CLOEXEC|O_NONBLOCK)

> +++ b/src/rpc/virnetclient.h
> @@ -0,0 +1,86 @@

> +
> +#ifndef __VIR_NET_CLIENT_H__
> +# define __VIR_NET_CLIENT_H__
> +
> +# include <stdbool.h>

Another <stdbool.h>

> +static void
> +virNetClientStreamEventTimer(int timer ATTRIBUTE_UNUSED, void *opaque)
> +{
> +    virNetClientStreamPtr st = opaque;
> +    int events = 0;
> +
> +    /* XXX we need a mutex on  'st' to protect this callback */

A few places throughout this series, I've noticed double-spaces
mid-sentence in comments.  Not fatal, but might be worth a cleanup pass
later on.

Looks like a reasonable refactoring overall; most of the code seems to
be copied from elsewhere already existing (so I'm assuming later patches
clean up existing code to use these new objects).

ACK with nits fixed.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110623/c95deded/attachment-0001.sig>