[libvirt] [PATCH 01/14] Introduce a simple API for handling JSON data

[Daniel P. Berrange]

On Thu, Dec 03, 2009 at 01:11:19PM +0100, Daniel Veillard wrote:
> On Thu, Nov 26, 2009 at 06:27:19PM +0000, Daniel P. Berrange wrote:
> > 
> > This uses the YAJL library for parsing/formatting from
> > 
> >  http://lloyd.github.com/yajl/
> 
> > * src/util/json.h, src/util/json.c: Data structures and APIs
> >   for representing JSON data, and parsing/formatting it
> > * configure.in: Add check for yajl library
> > * libvirt.spec.in: Add build requires for yajl
> > * src/Makefile.am: Add json.c/h
> > * src/libvirt_private.syms: Export JSON symbols to drivers
> 
>   hum ... that becomes an external library, one more dependancy
> on the other hand we don't have to track bug fixes.... how widely used
> is yajl ?

It is very unclear - JSON doesn't really appear to be used much
from C at all - it is mostly something that python/perl/javascript
people use. There are six parsers listed on http://json.org/ and
I ended up picking yajl because it was a very nicely designed API
that integrated well with what I wanted.

> > diff --git a/libvirt.spec.in b/libvirt.spec.in
> > index dba14df..408ad05 100644
> > --- a/libvirt.spec.in
> > +++ b/libvirt.spec.in
> > @@ -60,6 +60,7 @@
> >  %define with_netcf         0%{!?_without_netcf:0}
> >  %define with_udev          0%{!?_without_udev:0}
> >  %define with_hal           0%{!?_without_hal:0}
> > +%define with_yajl          0%{!?_without_yajl:0}
> >  
> >  # Non-server/HV driver defaults which are always enabled
> >  %define with_python        0%{!?_without_python:1}
> > @@ -141,6 +142,11 @@
> >  %define with_hal       0%{!?_without_hal:%{server_drivers}}
> >  %endif
> >  
> > +# Enable yajl library for JSON mode with QEMU
> > +%if 0%{?fedora} >= 13 || 0%{?rhel} >= 6
> > +%define with_yajl     0%{!?_without_yajl:%{server_drivers}}
> > +%endif
> > +
> 
>   yajl is not in F12, did someone add it to rawhide already ?

No, its not in Fedora at all. If we go for this patch, then I will
have to prepare an yajl RPM for Fedora.

> > +#define ReportError(conn, code, fmt...)                                 \
> > +    virReportErrorHelper(conn, VIR_FROM_NONE, code, __FILE__,           \
> > +                         __FUNCTION__, __LINE__, fmt)
> > +
> > +
> [...]
> > +static virJSONValuePtr virJSONValueNewNumber(const char *data)
> > +{
> > +    virJSONValuePtr val;
> > +
> > +    if (VIR_ALLOC(val) < 0)
> > +        return NULL;
> > +
> > +    val->type = VIR_JSON_TYPE_NUMBER;
> > +    if (!(val->data.number = strdup(data))) {
> > +        VIR_FREE(val);
> > +        return NULL;
> > +    }
> 
>   so we keep numbers as their string value ... surprizing but
> maybe that's the most flexible for our use.

The reason for this is that at a low level JSON does not define 
whether a number is an int, floating point, etc, etc. Thus when
we parse numbers we just get a string.

Only the application (in this case libvirt's QEMU driver) knows
what number format is applicable for a particular context. So
when the libvirt QEMU monitor code wants to set/get a number,
it will use one of the wrapper methods like

  virJSONValueNewNumberInt(int value)
  virJSONValueNewNumberLong(long long value)
  virJSONValueNewNumberDouble(double value)

> > +
> > +/* XXX add an incremental streaming parser - yajl trivially supports it */
> 
>   what would be the point ? I doubt there is a memory problem, it's more
> the incremental aspect I suppose.

It is all about how we decent the message boundaries. The current
QEMU code will always terminate a JSON message with a '\r\n', so
we can easily detect boundaries without needing to parse the data.

If QEMU developers suddenly change their mind and remove the '\r\n',
we would need an incremental streaming parser, so we can detect 
where the message boundaries are.

Hopefully they won't change their minds :-)

> > +
> > +#else
> > +virJSONValuePtr virJSONValueFromString(const char *jsonstring ATTRIBUTE_UNUSED)
> > +{
> > +    ReprotError(NULL, VIR_ERR_INTERNAL_ERROR, "%s",
> > +                _("No JSON parser implementation is available"));
> > +    return NULL;
> > +}
> > +char *virJSONValueToString(virJSONValuePtr object)
> > +{
> > +    ReprotError(NULL, VIR_ERR_INTERNAL_ERROR, "%s",
> > +                _("No JSON parser implementation is available"));
> > +    return NULL;
> > +}
> 
>   and the fallback
> 
> 
>   ACK, it' weird there ain't any JSON C API yet in F12, there is one for
> nearly all high level language but noone seems to have imposed a good C
> one. so now we are betting on yajl, maybe another one will prevail but
> since we have a good decoupling, fine by me,

The only other option I see is to actually include the YAJL parsing/formatting
code in the libvirt source tree, but that is rather evil in itself :-)

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|