[libvirt] [PATCH 5/8] Extend RPC protocol to allow FD passing

[Eric Blake]

On 10/25/2011 10:03 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"<berrange at redhat.com>
>
> Define two new RPC message types VIR_NET_CALL_WITH_FDS and
> VIR_NET_REPLY_WITH_FDS. These message types are equivalent
> to VIR_NET_CALL and VIR_NET_REPLY, except that between the
> message header, and payload there is a 32-bit integer field
> specifying how many file descriptors have been passed.
>
> The actual file descriptors are sent/recv'd out of band.
>
> * src/rpc/virnetmessage.c, src/rpc/virnetmessage.h,
>    src/libvirt_private.syms: Add support for handling
>    passed file descriptors
> * src/rpc/virnetprotocol.x: Extend protocol for FD
>    passing
> ---
>   docs/internals/rpc.html.in |   33 +++++++++++++

Thanks; this fixes my biggest concern from the last round.

>       <p>
> +      With the two packet types that support passing file descriptors, in
> +      between the header and the payload there will be a 4-byte integer
> +      specifying the number of file descriptors which are being sent.
> +      The actual file handles are sent after the payload has been sent.
> +      Each file handle has a single dummy byte transmitted as a carrier
> +      for the out of band file descriptor.

gnulib recvfd() ignores this byte value.  While sendfd() sends NUL, it 
is conceivable that a 3rd-party implementation of the wire protocol 
might pick a different value (or that gnulib might change in the 
future).  Should we add any documentation along the lines of 'sender 
should send 0' and 'receiver must ignore the dummy byte, even if it is 
not 0', just for robustness sake?

> +
> +
> +int virNetMessageDupFD(virNetMessagePtr msg,
> +                       size_t slot)
> +{
> +    int fd;
> +
> +    if (slot>= msg->nfds) {
> +        virNetError(VIR_ERR_INTERNAL_ERROR,
> +                    _("No FD available at slot %zu"), slot);
> +        return -1;
> +    }
> +
> +    if ((fd = dup(msg->fds[slot]))<  0) {
> +        virReportSystemError(errno,
> +                             _("Unable to duplicate FD %d"),
> +                             msg->fds[slot]);
> +        return -1;
> +    }

Do we want to be using gnulib's dup_cloexec ("cloexec.h") or fcntl(fd, 
F_DUPFD_CLOEXEC, 0) here, so that our dup doesn't leak into a 
third-party child if we are linked into some larger multithreaded app?

ACK with that resolved (and I'm okay if the resolution is saving the 
conversion of dup to dup_cloexec for another day, especially since I 
didn't raise the issue on the last round of reviews, but want this to go 
in before code freeze).

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