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

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

On 10/21/2011 06:55 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange 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
  src/libvirt_private.syms |    2 +
  src/rpc/virnetmessage.c  |  111 +++++++++++++++++++++++++++++++++++++++++++++-
  src/rpc/virnetmessage.h  |    9 ++++
  src/rpc/virnetprotocol.x |   23 +++++++++-
>   4 files changed, 142 insertions(+), 3 deletions(-)

Where's the edits to docs/internals/rpc.html.in that describes these two new types? In the interest of well-documented code, I won't ack without a v2 with docs.

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0648e49..f798c4a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1178,6 +1178,8 @@ virFileFdopen;


@@ -135,7 +152,11 @@ enum virNetMessageType {
      /* either direction. async notification */
      VIR_NET_MESSAGE = 2,
      /* either direction. stream data packet */
+    VIR_NET_STREAM = 3,
+    /* client ->  server. args from a method call, with passed FDs */
+    /* server ->  client. reply/error from a method call, with passed FDs */

Have you tested the case where client sends 4 but server does not understand it? Likewise, what if server sends 5 but client does not understand it? Are those graceful failures? Do we risk stranding a leaked fd into the side that wasn't aware of how to handle the new protocol?

Does this pass 'make check' with pdwtags (from the dwarves package) installed, or do you have to also tweak src/virnetprotocol-structs?

What you have looks mostly okay, but needs a v2 at least for docs.

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

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