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

[Daniel P. Berrange]

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 :|