Re: [libvirt] [PATCH 01/10] Provide a simple object for encoding/decoding RPC messages

18 Mar 2011


      On Tue, Mar 15, 2011 at 01:34:53PM -0600, Eric Blake wrote:
...
On 03/15/2011 11:51 AM, Daniel P. Berrange wrote:
...
This provides a new struct that contains a buffer for the RPC
message header+payload, as well as a decoded copy of the message
header. There is an API for applying a XDR encoding & decoding
of the message headers and payloads. There are also APIs for
maintaining a simple FIFO queue of message instances.
Expected usage scenarios are:
To send a message
msg = virNetMessageNew()
...fill in msg->header fields..
   virNetMessageEncodeHeader(msg)
   ...loook at msg->header fields to determine payload filter
   virNetMessageEncodePayload(msg, xdrfilter, data)
   ...send msg->bufferLength worth of data from buffer
To receive a message
msg = virNetMessageNew()
   ...read VIR_NET_MESSAGE_LEN_MAX of data into buffer
   virNetMessageDecodeLength(msg)
   ...read msg->bufferLength-msg->bufferOffset of data into buffer
   virNetMessageDecodeHeader(msg)
   ...look at msg->header fields to determine payload filter
   virNetMessageDecodePayload(msg, xdrfilter, data)
   ...run payload processor
...
+++ b/src/Makefile.am
@@ -1202,6 +1202,7 @@ EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
 noinst_LTLIBRARIES += libvirt-net-rpc.la
libvirt_net_rpc_la_SOURCES = \
+	rpc/virnetmessage.h rpc/virnetmessage.c \
  rpc/virnetprotocol.h rpc/virnetprotocol.c
Huh? There's no src/rpc in current libvirt.git.  This looks like 2/15 in
the v1 posting.  Did you forget to submit the original 1/15 as a
prerequisite patch?
http://www.redhat.com/archives/libvir-list/2010-December/msg00617.html
Yes, picked the wrong hash when sending the series.
...
...
+
+struct _virNetMessage {
+    char buffer[VIR_NET_MESSAGE_MAX + VIR_NET_MESSAGE_LEN_MAX];
Is it worth a comment warning that this struct should never be
stack-allocated?
Added
...
...
+void virNetMessageFree(virNetMessagePtr msg);
cfg.mk should list this as a free-like function.
Added, and several more in later patches
...
...
+
+virNetMessagePtr virNetMessageQueueServe(virNetMessagePtr *queue);
It's worth adding attributes:
ATTRIBUTE_NONNULL(1)
...
+void virNetMessageQueuePush(virNetMessagePtr *queue,
+                            virNetMessagePtr msg);
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
...
+
+int virNetMessageEncodeHeader(virNetMessagePtr msg);
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK
...
+int virNetMessageDecodeLength(virNetMessagePtr msg);
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK
...
+int virNetMessageDecodeHeader(virNetMessagePtr msg);
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK
...
+
+int virNetMessageEncodePayload(virNetMessagePtr msg,
+                               xdrproc_t filter,
+                               void *data);
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK
...
+int virNetMessageDecodePayload(virNetMessagePtr msg,
+                               xdrproc_t filter,
+                               void *data);
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK
...
+
+int virNetMessageEncodePayloadRaw(virNetMessagePtr msg,
+                                  const char *buf,
+                                  size_t len);
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK
...
+
+void virNetMessageSaveError(virNetMessageErrorPtr rerr);
ATTRIBUTE_NONNULL(1)
Added all those too.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

    