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

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



On Thu, Jun 23, 2011 at 09:02:16PM -0600, Eric Blake wrote:
> 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?

Yes it should be.

> 
> > +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)

Actually we only want O_CLOEXEC for this one.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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