Review needed: 3.2 release feature -- libvdsm

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/
-- 
Adam Litke <agl(a)us.ibm.com&gt;
IBM Linux Technology Center

[Dan Kenigsberg]

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/
>  --  
There is a couple of of patches below 8614 - I suppose they need review
too.

[Dan Kenigsberg]

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).

Regards,
Dan.

[Tony Asleson]

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?

Regards,
Tony

*Except size is represented as fixed length, zero padded text instead of
binary.

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.


-- 
Adam Litke <agl(a)us.ibm.com&gt;
IBM Linux Technology Center

[Dan Kenigsberg]

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.

On Tue, Nov 06, 2012 at 05:49:22PM +0200, 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).
>   
Thanks for bringing up this point.  I would like to make this protocol
compatible with existing clients.  Is there a standard for segmenting messages
over the channel?  I suppose it depends on the transport layer.


-- 
Adam Litke <agl(a)us.ibm.com&gt;
IBM Linux Technology Center