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

On 10/25/2011 10:03 AM, Daniel P. Berrange wrote: >From: "Daniel P. Berrange&quot;&lt;berrange(a)redhat.com&gt; > >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?