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"<berrange(a)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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org