[libvirt] [PATCH 5/8] Introduce generic RPC server objects

[Eric Blake]

On 12/01/2010 10:26 AM, Daniel P. Berrange wrote:
> To facilitate creation of new daemons providing XDR RPC services,
> pull alot of the libvirtd daemon code into a set of reusable
> objects.
> 
>  * virNetServer: A server contains one or more services which
>    accept incoming clients. It maintains the list of active
>    clients. It has a list of RPC programs which can be used
>    by clients. When clients produce a complete RPC message,
>    the server passes this onto the corresponding program for
>    handling, and queues any response back with the client.
> 
>  * virNetServerClient: Encapsulates a single client connection.
>    All I/O for the client is handled, reading & writing RPC
>    messages. Also contains the SASL/TLS code, but this will
>    eventually move into the virNetSocket object
> 
>  * virNetServerProgram: Handles processing and dispatch of
>    RPC method calls for a single RPC (program,version).
>    Multiple programs can be registered with the server.
> 
>  * virNetServerService: Encapsulates socket(s) listening for
>    new connections. Each service listens on a single host/port,
>    but may have multiple sockets if on a dual IPv4/6 host.
> 
> Each new daemon now merely has to define the list of RPC procedures
> & their handlers. It does not need to deal with any network related
> functionality at all.

> @@ -1138,6 +1138,22 @@ libvirt_net_rpc_la_LDFLAGS = \
>  libvirt_net_rpc_la_LIBADD = \
>  			$(CYGWIN_EXTRA_LIBADD)
>  
> +libvirt_net_server_la_SOURCES = \
> +	rpc/virnetservermessage.h \
> +	rpc/virnetserverprogram.h rpc/virnetserverprogram.c \
> +	rpc/virnetserverservice.h rpc/virnetserverservice.c \
> +	rpc/virnetserverclient.h rpc/virnetserverclient.c \
> +	rpc/virnetserver.h rpc/virnetserver.c
> +libvirt_net_server_la_CFLAGS = \
> +			$(AM_CFLAGS)
> +libvirt_net_server_la_LDFLAGS = \
> +			$(AM_LDFLAGS) \
> +			$(CYGWIN_EXTRA_LDFLAGS) \
> +			$(MINGW_EXTRA_LDFLAGS)l

s/l$//

> +
> +struct _virNetServer {
> +    int refs;

size_t?

> +static void virNetServerHandleJob(void *jobOpaque, void *opaque)
> +{
> +    virNetServerPtr srv = opaque;
> +    virNetServerJobPtr job = jobOpaque;
> +    virNetServerProgramPtr prog = NULL;
> +    int i;

s/int/size_t/

> +
> +    if (virNetServerProgramDispatch(prog,
> +                                    srv,
> +                                    job->client,
> +                                    job->msg) < 0) {
> +        job->msg = NULL;
> +        goto error;
> +    }

Should this call be run while the virNetServerLock is still held, or
should we drop and reacquire the lock to allow other virNetServer
threads to make progress during the arbitrarily long dispatch?

> +static int virNetServerDispatchNewClient(virNetServerServicePtr svc ATTRIBUTE_UNUSED,
> +                                         virNetServerClientPtr client,
> +                                         void *opaque)
> +{
> +    virNetServerPtr srv = opaque;
> +
> +    virNetServerLock(srv);
> +
> +    if (srv->nclients >= srv->nclients_max) {
> +        virNetError(VIR_ERR_RPC,
> +                    _("Too many active clients (%d), dropping connection from %s"),
> +                    (int)srv->nclients_max, virNetServerClientAddrString(client));

You can safely use %zu to avoid the case of size_t srv->nclients_max.

> +
> +#if 0
> +    reprocess:
> +        for (i = 0 ; i < srv->nclients ; i++) {
> +            int inactive;

Any reason to keep this block of code, since you commented it out?

> +static void
> +virNetServerProgramFormatError(virNetServerProgramPtr prog,
> +                               void *rerr,
> +                               int code,
> +                               const char *fmt,
> +                               ...)
> +{
> +    va_list args;
> +    char msgbuf[1024];
> +    char *msg = msgbuf;
> +
> +    va_start(args, fmt);
> +    vsnprintf(msgbuf, sizeof msgbuf, fmt, args);

Should we do something special if the message got truncated at
sizeof(msgbuf)?
> +
> +virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
> +                                                 const char *service,
> +                                                 int auth,
> +                                                 virNetTLSContextPtr tls)
> +{
> +    virNetServerServicePtr svc;
> +    int i;

s/int/size_t/

Mostly looks reasonable (probably because it's mostly copying from
existing working code, and just renaming into the new API names).

-- 
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/20101209/37fa2805/attachment-0001.sig>