[libvirt] [PATCH 6/8] Add client side support for FD passing

[Eric Blake]

On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"<berrange at redhat.com>
>
> Extend the RPC client code to allow file descriptors to be sent
> to the server with calls, and received back with replies.
>
> * src/remote/remote_driver.c: Stub extra args
> * src/libvirt_private.syms, src/rpc/virnetclient.c,
>    src/rpc/virnetclient.h, src/rpc/virnetclientprogram.c,
>    src/rpc/virnetclientprogram.h: Extend APIs to allow
>    FD passing
> ---
>   src/libvirt_private.syms      |    4 +++
>   src/remote/remote_driver.c    |    1 +
>   src/rpc/virnetclient.c        |   25 +++++++++++++++++++++++
>   src/rpc/virnetclient.h        |    2 +
>   src/rpc/virnetclientprogram.c |   44 +++++++++++++++++++++++++++++++++++++++-
>   src/rpc/virnetclientprogram.h |    4 +++
>   6 files changed, 78 insertions(+), 2 deletions(-)

>
> +bool virNetClientHasPassFD(virNetClientPtr client)
> +{
> +    int hasPassFD;

s/int/bool/

> +    virNetClientLock(client);
> +    hasPassFD = virNetSocketHasPassFD(client->sock);
> +    virNetClientUnlock(client);
> +    return hasPassFD;

so that your return value matches the signature off the bat.

> @@ -697,6 +708,15 @@ virNetClientCallDispatch(virNetClientPtr client)
>       case VIR_NET_REPLY: /* Normal RPC replies */
>           return virNetClientCallDispatchReply(client);
>
> +    case VIR_NET_REPLY_WITH_FDS: /* Normal RPC replies with FDs */
> +        if (virNetMessageDecodeNumFDs(&client->msg)<  0)
> +            return -1;
> +        for (i = 0 ; i<  client->msg.nfds ; i++) {
> +            if ((client->msg.fds[i] = virNetSocketRecvFD(client->sock))<  0)

You do realize that gnulib's sendfd/recvfd pass a single byte alongside 
each out-of-band fd (since passing fds with 0-byte messages isn't 
portable).  It looks like you were careful to ensure that fds are only 
sent and received in between complete messages; so hopefully we don't 
ever run into any problems where the extra byte payloads gets 
interleaved with real rpc traffic, since that could cause confusion on 
the current state of bytes going between endpoints.  Is encryption ever 
used on UNIX sockets, or is that only for TCP connections?

> @@ -278,13 +285,30 @@ int virNetClientProgramCall(virNetClientProgramPtr prog,
>       msg->header.prog = prog->program;
>       msg->header.vers = prog->version;
>       msg->header.status = VIR_NET_OK;
> -    msg->header.type = VIR_NET_CALL;
> +    msg->header.type = noutfds ? VIR_NET_CALL_WITH_FDS : VIR_NET_CALL;
>       msg->header.serial = serial;
>       msg->header.proc = proc;
> +    msg->nfds = noutfds;
> +    if (VIR_ALLOC_N(msg->fds, msg->nfds)<  0) {
> +        virReportOOMError();
> +        goto error;
> +    }
> +    for (i = 0 ; i<  msg->nfds ; i++) {
> +        if ((msg->fds[i] = dup(outfds[i]))<  0) {

Should we be using fcntl(outfds[i], F_DUP_CLOEXEC, 0) here, so that the 
dups don't leak out of this process?

ACK with nits fixed.

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