[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Thu, Nov 26, 2009 at 06:27:19PM +0000, Daniel P. Berrange wrote:
> This introduces simple API for handling JSON data. There is
> an internal data structure 'virJSONValuePtr' which stores a
> arbitrary nested JSON value (number, string, array, object,
> nul, etc).  There are APIs for constructing/querying objects
> and APIs for parsing/formatting string formatted JSON data.
> 
> 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 ?

> ---
>  configure.in             |   55 +++
>  libvirt.spec.in          |   14 +
>  po/POTFILES.in           |    1 +
>  src/Makefile.am          |    8 +-
>  src/libvirt_private.syms |   47 ++
>  src/util/json.c          | 1043 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/json.h          |  131 ++++++

  I see that we are encapsulating it and exposing a completely distinct
API, that's fine then, we can switch or embbed our own if needed later.

>  7 files changed, 1296 insertions(+), 3 deletions(-)
>  create mode 100644 src/util/json.c
>  create mode 100644 src/util/json.h
> 
> diff --git a/configure.in b/configure.in
> index f735bba..41f50fc 100644
> --- a/configure.in
> +++ b/configure.in
> @@ -645,6 +645,56 @@ AC_SUBST([SASL_CFLAGS])
>  AC_SUBST([SASL_LIBS])
>  
>  
> +dnl YAJL JSON library http://lloyd.github.com/yajl/
> +AC_ARG_WITH([yajl],
> +  [  --with-yajl         use YAJL for JSON parsing/formatting],
> +  [],
> +  [with_yajl=check])
> +
> +YAJL_CFLAGS=
> +YAJL_LIBS=
> +if test "x$with_yajl" != "xno"; then
> +  if test "x$with_yajl" != "xyes" -a "x$with_yajl" != "xcheck"; then
> +    YAJL_CFLAGS="-I$with_yajl/include"
> +    YAJL_LIBS="-L$with_yajl/lib"
> +  fi
> +  fail=0
> +  old_cppflags="$CPPFLAGS"
> +  old_ldflags="$LDFLAGS"
> +  CPPFLAGS="$CPPFLAGS $YAJL_CFLAGS"
> +  LDFLAGS="$LDFLAGS $YAJL_LIBS"
> +  AC_CHECK_HEADER([yajl/yajl_common.h],[],[
> +    if test "x$with_yajl" != "xcheck" ; then
> +        with_yajl=no
> +    else
> +        fail=1
> +    fi])
> +  if test "x$with_yajl" != "xno" ; then
> +    AC_CHECK_LIB([yajl], [yajl_parse],[
> +      YAJL_LIBS="$YAJL_LIBS -lyajl"
> +      with_yajl=yes
> +    ],[
> +      if test "x$with_yajl" = "xcheck" ; then
> +        with_yajl=no
> +      else
> +        fail=1
> +      fi
> +    ])
> +  fi
> +  test $fail = 1 &&
> +    AC_MSG_ERROR([You must install the YAJL development package in order to compile libvirt])
> +  CPPFLAGS="$old_cppflags"
> +  LDFLAGS="$old_ldflags"
> +  if test "x$with_yajl" = "xyes" ; then
> +    AC_DEFINE_UNQUOTED([HAVE_YAJL], 1,
> +      [whether YAJL is available for JSON parsing/formatting])
> +  fi
> +fi
> +AM_CONDITIONAL([HAVE_YAJL], [test "x$with_yajl" = "xyes"])
> +AC_SUBST([YAJL_CFLAGS])
> +AC_SUBST([YAJL_LIBS])
> +
> +
>  dnl PolicyKit library
>  POLKIT_CFLAGS=
>  POLKIT_LIBS=
> @@ -1859,6 +1909,11 @@ AC_MSG_NOTICE([    sasl: $SASL_CFLAGS $SASL_LIBS])
>  else
>  AC_MSG_NOTICE([    sasl: no])
>  fi
> +if test "$with_yajl" != "no" ; then
> +AC_MSG_NOTICE([    yajl: $YAJL_CFLAGS $YAJL_LIBS])
> +else
> +AC_MSG_NOTICE([    yajl: no])
> +fi
>  if test "$with_avahi" = "yes" ; then
>  AC_MSG_NOTICE([   avahi: $AVAHI_CFLAGS $AVAHI_LIBS])
>  else
> 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 ?

>  # Force QEMU to run as non-root
>  %if 0%{?fedora} >= 12 || 0%{?rhel} >= 6
>  %define qemu_user  qemu
> @@ -257,6 +263,9 @@ BuildRequires: hal-devel
>  BuildRequires: libudev-devel >= 145
>  BuildRequires: libpciaccess-devel >= 0.10.9
>  %endif
> +%if %{with_yajl}
> +BuildRequires: yajl-devel
> +%endif
>  %if %{with_avahi}
>  BuildRequires: avahi-devel
>  %endif
> @@ -495,6 +504,10 @@ of recent versions of Linux (and other OSes).
>  %define _without_udev --without-udev
>  %endif
>  
> +%if ! %{with_yajl}
> +%define _without_yajl --without-yajl
> +%endif
> +
>  %configure %{?_without_xen} \
>             %{?_without_qemu} \
>             %{?_without_openvz} \
> @@ -522,6 +535,7 @@ of recent versions of Linux (and other OSes).
>             %{?_without_selinux} \
>             %{?_without_hal} \
>             %{?_without_udev} \
> +           %{?_without_yajl} \
>             --with-qemu-user=%{qemu_user} \
>             --with-qemu-group=%{qemu_group} \
>             --with-init-script=redhat \
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 116aa87..9864259 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -50,6 +50,7 @@ src/uml/uml_driver.c
>  src/util/bridge.c
>  src/util/conf.c
>  src/util/iptables.c
> +src/util/json.c
>  src/util/logging.c
>  src/util/pci.c
>  src/util/processinfo.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 9a3c9c8..b0775a8 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -52,6 +52,7 @@ UTIL_SOURCES =							\
>  		util/hash.c util/hash.h				\
>  		util/iptables.c util/iptables.h			\
>  		util/ebtables.c util/ebtables.h			\
> +		util/json.c util/json.h				\
>  		util/logging.c util/logging.h			\
>  		util/memory.c util/memory.h			\
>  		util/pci.c util/pci.h				\
> @@ -283,8 +284,8 @@ noinst_LTLIBRARIES = libvirt_util.la
>  libvirt_la_LIBADD = libvirt_util.la
>  libvirt_util_la_SOURCES =					\
>  		$(UTIL_SOURCES)
> -libvirt_util_la_CFLAGS =  $(CAPNG_CFLAGS)
> -libvirt_util_la_LDFLAGS =  $(CAPNG_LIBS)
> +libvirt_util_la_CFLAGS =  $(CAPNG_CFLAGS) $(YAJL_CFLAGS)
> +libvirt_util_la_LDFLAGS =  $(CAPNG_LIBS) $(YAJL_LIBS)
>  
>  
>  noinst_LTLIBRARIES += libvirt_conf.la
> @@ -828,12 +829,13 @@ libvirt_lxc_SOURCES =						\
>  		$(NODE_INFO_SOURCES)				\
>  		$(ENCRYPTION_CONF_SOURCES)			\
>  		$(DOMAIN_CONF_SOURCES)
> -libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) $(CAPNG_LIBS)
> +libvirt_lxc_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDCFLAGS) $(CAPNG_LIBS) $(YAJL_LIBS)
>  libvirt_lxc_LDADD = $(LIBXML_LIBS) $(NUMACTL_LIBS) ../gnulib/lib/libgnu.la
>  libvirt_lxc_CFLAGS =				\
>  		$(LIBPARTED_CFLAGS)		\
>  		$(NUMACTL_CFLAGS)		\
>  		$(CAPNG_CFLAGS)			\
> +		$(YAJL_CFLAGS)			\
>  		-I top_srcdir@/src/conf
>  endif
>  endif
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e880c2e..ee7f3ed 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -269,6 +269,53 @@ virRegisterDeviceMonitor;
>  virRegisterSecretDriver;
>  
>  
> +# json.h
> +virJSONValueFree;
> +virJSONValueNewString;
> +virJSONValueNewStringLen;
> +virJSONValueNewNumberInt;
> +virJSONValueNewNumberUint;
> +virJSONValueNewNumberLong;
> +virJSONValueNewNumberUlong;
> +virJSONValueNewNumberDouble;
> +virJSONValueNewBoolean;
> +virJSONValueNewNull;
> +virJSONValueNewArray;
> +virJSONValueNewObject;
> +virJSONValueObjectAppend;
> +virJSONValueObjectAppendString;
> +virJSONValueObjectAppendNumberInt;
> +virJSONValueObjectAppendNumberUint;
> +virJSONValueObjectAppendNumberLong;
> +virJSONValueObjectAppendNumberUlong;
> +virJSONValueObjectAppendNumberDouble;
> +virJSONValueObjectAppendBoolean;
> +virJSONValueObjectAppendNull;
> +virJSONValueArrayAppend;
> +virJSONValueObjectHasKey;
> +virJSONValueObjectGet;
> +virJSONValueArraySize;
> +virJSONValueArrayGet;
> +virJSONValueGetString;
> +virJSONValueGetNumberInt;
> +virJSONValueGetNumberUint;
> +virJSONValueGetNumberLong;
> +virJSONValueGetNumberUlong;
> +virJSONValueGetNumberDouble;
> +virJSONValueGetBoolean;
> +virJSONValueIsNull;
> +virJSONValueObjectGetString;
> +virJSONValueObjectGetNumberInt;
> +virJSONValueObjectGetNumberUint;
> +virJSONValueObjectGetNumberLong;
> +virJSONValueObjectGetNumberUlong;
> +virJSONValueObjectGetNumberDouble;
> +virJSONValueObjectGetBoolean;
> +virJSONValueObjectIsNull;
> +virJSONValueFromString;
> +virJSONValueToString;
> +
> +
>  # logging.h
>  virLogMessage;
>  virLogGetNbFilters;
> diff --git a/src/util/json.c b/src/util/json.c
> new file mode 100644
> index 0000000..f359b95
> --- /dev/null
> +++ b/src/util/json.c
> @@ -0,0 +1,1043 @@
> +/*
> + * json.h: JSON object parsing/formatting

  json.c

> + * Copyright (C) 2008 Daniel P. Berrange
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
> + *
> + */
> +
> +
> +#include <config.h>
> +
> +#include "json.h"
> +#include "memory.h"
> +#include "virterror_internal.h"
> +#include "logging.h"
> +#include "util.h"
> +
> +#include <yajl/yajl_gen.h>
> +#include <yajl/yajl_parse.h>
> +
> +/* XXX fixme */
> +#define VIR_FROM_THIS VIR_FROM_NONE

  hum, I would defined a new domain after all we have one for XML
parsing, but not urgent.

> +#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.

> +    return val;
> +}
[...]
> +
> +#if HAVE_YAJL
> +static int virJSONParserInsertValue(virJSONParserPtr parser,
> +                                    virJSONValuePtr value)

  okay so that's the YAJL specific part

[...]

> +static const yajl_callbacks parserCallbacks = {
> +    virJSONParserHandleNull,
> +    virJSONParserHandleBoolean,
> +    NULL,
> +    NULL,
> +    virJSONParserHandleNumber,
> +    virJSONParserHandleString,
> +    virJSONParserHandleStartMap,
> +    virJSONParserHandleMapKey,
> +    virJSONParserHandleEndMap,
> +    virJSONParserHandleStartArray,
> +    virJSONParserHandleEndArray
> +};

  and a driver block for it

> +
> +/* 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.

[...]
> +
> +#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,

  ACK

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]