[vdsm] Review needed: 3.2 release feature -- libvdsm

Dan Kenigsberg danken at redhat.com
Thu Nov 8 12:27:07 UTC 2012


On Thu, Nov 08, 2012 at 03:22:46AM -0600, Adam Litke wrote:
> On Tue, Nov 06, 2012 at 11:40:53AM -0600, Tony Asleson wrote:
> > On 11/06/2012 09:49 AM, Dan Kenigsberg wrote:
> > > On Mon, Oct 29, 2012 at 10:20:04AM -0500, Adam Litke wrote:
> > >> Hi everyone,
> > >>
> > >> libvdsm is listed as a release feature for 3.2 (preview only)[1][2].  There is a
> > >> set of patches up in gerrit that could use a wide review from the community.
> > >> The plan is to merge the new json-rpc server[3] first so if you could
> > >> concentrate your reviews there it would yield the greatest benefit.  Thanks!
> > >>
> > >> [1] http://wiki.ovirt.org/wiki/OVirt_3.2_release-management
> > >> [2] http://wiki.ovirt.org/wiki/Features/libvdsm
> > >> [3] http://gerrit.ovirt.org/#/c/8614/
> > > 
> > > [3] defines the format of each message as
> > > 
> > >     <size><json-data>
> > > 
> > > where <size> is a binary value, used to split a (tcp) stream into
> > > messages. I would like to consider another splitting scheme, which I
> > > find better suited to the textual nature of jsonrpc: terminate each
> > > message with the newline character. It makes the protocol easier to
> > > sniff and debug (in case you've missed part of a message).
> > > 
> > > The down size is that we would need to require clients to
> > > escape literal newlines, and unescape them in responses (both are done
> > > by python's json module, and the latter is part of the json standard).
> > 
> > I use json-rpc for IPC in libStoragemgmt (out of process plug-ins) with
> > unix domain sockets.  I adopted the <size><json-data> model as well*.
> > 
> > I chose this because it allows the use of non-stream capable json
> > parsers.  I wanted to ensure that the transport and protocol would be
> > language and parser agnostic.
> > 
> > You could achieve the message separation with new lines as you suggest,
> > but then you may have to parse the message stream twice.  Once to find
> > the message delimiter and once again to parse the json itself, depending
> > on json parser.  Having the size at the beginning of the message is
> > incredibly convenient from a coding efficiency standpoint.
> > 
> > As for debug, I just log the message payload if needed.  I haven't had
> > the need to use a packet trace, but I'm not sure having a single newline
> > separating messages would be obvious in a single frame capture?
> > 
> > Would it be possible to compromise and leave the length and add the
> > newline as the end?  So <size><payload><new line>?  You could then pass
> > the message payload to the parser with without having to escape the
> > newlines?
> 
> Thanks for weighing in on this!  If you use the size and newline, how do you
> account for the newline char in the <size> value?  It seems unnecessary to
> include this character to me since you can use a combination of logfile analysis
> and scanning the data stream for 'id': to find method calls and responses.

If you know where one message starts (let alone if you have the begining
of the stream), a newline is unnecessary, or you could rely on a
heuristic such as the scanning you suggest.

I was talking more about the cleanliness of the protocol: in the
payload, we do not send integers we send ascii decimal representation of
them, but for segmentation we use these binary words.

I do not have a really strong opinion here and did not think much about
parser efficience, but I do know that
http://git.savannah.gnu.org/cgit/qemu.git/tree/QMP/qmp-spec.txt has

2.1.1 All interactions transmitted by the Server are json-objects, always
      terminating with CRLF

Dan.


More information about the vdsm-devel mailing list