[libvirt] [PATCH 01/15] Defines the basics of a generic RPC protocol in XDR

[Eric Blake]

On 12/16/2010 04:21 AM, Daniel P. Berrange wrote:
> This patch defines the basics of a generic RPC protocol in XDR.
> This is wire ABI compatible with the original remote_protocol.x.
> diff --git a/.x-sc_preprocessor_indentation b/.x-sc_preprocessor_indentation
> new file mode 100644
> index 0000000..30273f8
> --- /dev/null
> +++ b/.x-sc_preprocessor_indentation
> @@ -0,0 +1 @@
> +src/rpc/virnetprotocol.h
> \ No newline at end of file

This file should have had a newline, if it is even needed.  However, I
think that you are better off changing preprocessor_exempt in cfg.mk
instead of adding this file.

> diff --git a/Makefile.am b/Makefile.am
> index c525e65..a400f59 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -21,6 +21,7 @@ syntax_check_exceptions = \
>    .x-sc_bindtextdomain \
>    .x-sc_m4_quote_check \
>    .x-sc_po_check \
> +  .x-sc_preprocessor_indentation \

Which means you also don't need this change.

> diff --git a/src/rpc/virnetprotocol.x b/src/rpc/virnetprotocol.x
> new file mode 100644
> index 0000000..10ece6f
> --- /dev/null
> +++ b/src/rpc/virnetprotocol.x
> +
> +/*----- Data types. -----*/
> +
> +/* Maximum total message size (serialised). */
> +const VIR_NET_MESSAGE_MAX = 262144;
> +
> +/* Size of struct virNetMessageHeader (serialized)*/

Love the comment copy-and-paste inconsistency from remote_protocol.x
(-sed vs. -zed).

> +const VIR_NET_MESSAGE_HEADER_MAX = 24;
> +
> +/* Size of message payload */
> +const VIR_NET_MESSAGE_PAYLOAD_MAX = 262120;

Would it be better to write VIR_NET_MESSAGE_PAYLOAD_MAX =
VIR_NET_MESSAGE_MAX - VIR_NET_MESSAGE_HEADER_MAX?  But that's just cosmetic.

> +
> +/* Size of message length field. Not counted in VIR_NET_MESSAGE_MAX */
> +const VIR_NET_MESSAGE_LEN_MAX = 4;
> +
> +const VIR_NET_MESSAGE_STRING_MAX = 65536;

You lost a useful comment from remote_protocol.h:

/* Length of long, but not unbounded, strings.
 * This is an arbitrary limit designed to stop the decoder from trying
 * to allocate unbounded amounts of memory when fed with a bad message.
 */
	

> + * and the 'status' field varies according to:
> + *
> + *  - type == VIR_NET_CALL
> + *     * VIR_NET_OK always
> + *
> + *  - type == VIR_NET_REPLY
> + *     * VIR_NET_OK if RPC finished successfully
> + *     * VIR_NET_ERROR if something failed
> + *
> + *  - type == VIR_NET_MESSAGE
> + *     * VIR_NET_OK always
> + *
> + *  - type == VIR_NET_STREAM
> + *     * VIR_NET_CONTINUE if more data is following
> + *     * VIR_NET_OK if stream is complete
> + *     * VIR_NET_ERROR if stream had an error
> + *
> + * Payload varies according to type and status:
> + *
> + *  - type == VIR_NET_CALL
> + *          XXX_args  for procedure
> + *
> + *  - type == VIR_NET_REPLY
> + *     * status == VIR_NET_OK
> + *          XXX_ret         for procedure
> + *     * status == VIR_NET_ERROR
> + *          remote_error    Error information
> + *
> + *  - type == VIR_NET_MESSAGE
> + *     * status == VIR_NET_OK
> + *          XXX_args        for procedure
> + *     * status == VIR_NET_ERROR
> + *          remote_error    Error information

This is pure copy-and-paste from remote_protocol.x, but it doesnt' make
sense to me.  Earlier, you said that VIR_NET_MESSAGE implies status is
VIR_NET_OK always, and that messages are asynchronous, rather than in
reply to a message.  Either this should be:

- type == VIR_NET_MESSAGE
    XXX_msg      according to proc

or you need to allow for VIR_NET_MESSAGE to pass status==VIR_NET_ERROR.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20101216/bfa92f71/attachment-0001.sig>