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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/25/2011 10:03 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange@xxxxxxxxxx>

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@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]