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

[Eric Blake]

On 10/21/2011 06:55 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
> ---
>   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;
>   virNetMessageClear;
>   virNetMessageEncodeHeader;
>   virNetMessageEncodePayload;
> +virNetMessageEncodeNumFDs;
> +virNetMessageDecodeNumFDs;

Sorting?

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

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