Check for the presence of Jansson library and prefer it to yajl
if possible.
The minimum required version is 2.7.
Internally, virJSONValue still stores numbers as strings even
though Jansson uses numeric variables for them.
The configure script is particularly hideous, but will hopefully
go away after we stop aiming to support compiling on CentOS 6.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
configure.ac | 1 +
m4/virt-json.m4 | 55 +++++++++++---
src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 264 insertions(+), 11 deletions(-)
diff --git a/configure.ac b/configure.ac
index bef10c0fb..fe93209d7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -987,6 +987,7 @@ LIBVIRT_RESULT_FUSE
LIBVIRT_RESULT_GLUSTER
LIBVIRT_RESULT_GNUTLS
LIBVIRT_RESULT_HAL
+LIBVIRT_RESULT_JANSSON
LIBVIRT_RESULT_LIBNL
LIBVIRT_RESULT_LIBPCAP
LIBVIRT_RESULT_LIBSSH
diff --git a/m4/virt-json.m4 b/m4/virt-json.m4
index 5e4bcc7c9..a5ae3edcd 100644
--- a/m4/virt-json.m4
+++ b/m4/virt-json.m4
@@ -18,12 +18,24 @@ dnl <http://www.gnu.org/licenses/>.
dnl
AC_DEFUN([LIBVIRT_ARG_JSON],[
+ LIBVIRT_ARG_WITH_FEATURE([JANSSON], [jansson], [check])
LIBVIRT_ARG_WITH_FEATURE([YAJL], [yajl], [check])
])
AC_DEFUN([LIBVIRT_CHECK_JSON],[
+ if test "$with_yajl:$with_jansson" = "yes:yes"; then
+ AC_MSG_ERROR("Compiling with both jansson and yajl is unsupported")
+ fi
+
+ if test "$with_yajl" = "yes"; then
+ with_jansson=no
+ elif test "$with_jansson" = "yes"; then
+ with_yajl=no
+ fi
+
need_json=no
- if test "$with_qemu:$with_yajl" = yes:check; then
+ if test "$with_qemu:$with_yajl" = yes:check or
+ test "$with_qemu:$with_jansson" = yes:check; then
dnl Some versions of qemu require the use of JSON; try to detect them
dnl here, although we do not require qemu to exist in order to compile.
dnl This check mirrors src/qemu/qemu_capabilities.c
@@ -44,24 +56,45 @@ AC_DEFUN([LIBVIRT_CHECK_JSON],[
fi
fi
- dnl YAJL JSON library http://lloyd.github.com/yajl/
- LIBVIRT_CHECK_LIB_ALT([YAJL], [yajl],
- [yajl_parse_complete], [yajl/yajl_common.h],
- [YAJL2], [yajl],
- [yajl_tree_parse], [yajl/yajl_common.h])
+ dnl Jansson http://www.digip.org/jansson/
+ LIBVIRT_CHECK_PKG([JANSSON], [jansson], [2.7])
+
+ if test "$with_jansson" = "no"; then
+ dnl YAJL JSON library http://lloyd.github.com/yajl/
+ LIBVIRT_CHECK_LIB_ALT([YAJL], [yajl],
+ [yajl_parse_complete], [yajl/yajl_common.h],
+ [YAJL2], [yajl],
+ [yajl_tree_parse], [yajl/yajl_common.h])
+ else
+ AM_CONDITIONAL([WITH_YAJL], 0)
+ AM_CONDITIONAL([WITH_YAJL2], 0)
+ fi
AM_CONDITIONAL([WITH_JSON],
- [test "$with_yajl" = "yes"])
- if test "$with_yajl" = "yes"; then
+ [test "$with_yajl" = "yes" || test "$with_jansson" = "yes"])
+ if test "$with_yajl" = "yes" || test "$with_jansson" = "yes"; then
AC_DEFINE([WITH_JSON], [1], [whether a JSON library is available])
elif "$need_json" = "yes"; then
AC_MSG_ERROR([QEMU needs JSON but no library is available])
fi
- AC_SUBST([JSON_CFLAGS], [$YAJL_CFLAGS])
- AC_SUBST([JSON_LIBS], [$YAJL_LIBS])
+
+ if test "$with_jansson" = "yes"; then
+ AC_SUBST([JSON_CFLAGS], [$JANSSON_CFLAGS])
+ AC_SUBST([JSON_LIBS], [$JANSSON_LIBS])
+ else
+ AC_SUBST([JSON_CFLAGS], [$YAJL_CFLAGS])
+ AC_SUBST([JSON_LIBS], [$YAJL_LIBS])
+ fi
])
AC_DEFUN([LIBVIRT_RESULT_JSON],[
- LIBVIRT_RESULT_LIB([YAJL])
+ if test "$with_jansson" = "yes"; then
+ msg = "Jansson"
+ elif test "$with_yajl" = "yes"; then
+ msg = "yajl"
+ else
+ msg = "none"
+ fi
+ LIBVIRT_RESULT([JSON library:], [$msg])
])
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 6a02ddf0c..86cbd6eef 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1951,6 +1951,225 @@ virJSONValueToString(virJSONValuePtr object,
}
+#elif WITH_JANSSON
+# include <jansson.h>
+
+static virJSONValuePtr
+virJSONValueFromJansson(json_t *json)
+{
+ virJSONValuePtr ret = NULL;
+ const char *key;
+ json_t *cur;
+ size_t i;
+
+ switch (json_typeof(json)) {
+ case JSON_OBJECT:
+ ret = virJSONValueNewObject();
+ if (!ret)
+ goto error;
+
+ json_object_foreach(json, key, cur) {
+ virJSONValuePtr val = virJSONValueFromJansson(cur);
+ if (!val)
+ goto error;
+
+ if (virJSONValueObjectAppend(ret, key, val) < 0)
+ goto error;
+ }
+
+ break;
+
+ case JSON_ARRAY:
+ ret = virJSONValueNewArray();
+ if (!ret)
+ goto error;
+
+ json_array_foreach(json, i, cur) {
+ virJSONValuePtr val = virJSONValueFromJansson(cur);
+ if (!val)
+ goto error;
+
+ if (virJSONValueArrayAppend(ret, val) < 0)
+ goto error;
+ }
+ break;
+
+ case JSON_STRING:
+ ret = virJSONValueNewStringLen(json_string_value(json),
+ json_string_length(json));
+ break;
+
+ case JSON_INTEGER:
+ ret = virJSONValueNewNumberLong(json_integer_value(json));
+ break;
+
+ case JSON_REAL:
+ ret = virJSONValueNewNumberDouble(json_real_value(json));
+ break;
+
+ case JSON_TRUE:
+ case JSON_FALSE:
+ ret = virJSONValueNewBoolean(json_boolean_value(json));
+ break;
+
+ case JSON_NULL:
+ ret = virJSONValueNewNull();
+ break;
+ }
+
+ return ret;
+
+ error:
+ virJSONValueFree(ret);
+ return NULL;
+}
+
+virJSONValuePtr
+virJSONValueFromString(const char *jsonstring)
+{
+ virJSONValuePtr ret = NULL;
+ json_t *json;
+ json_error_t error;
+ size_t flags = JSON_REJECT_DUPLICATES |
+ JSON_DECODE_ANY;
+
+ if (!(json = json_loads(jsonstring, flags, &error))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to parse JSON %d:%d: %s"),
+ error.line, error.column, error.text);
+ return NULL;
+ }
+
+ ret = virJSONValueFromJansson(json);
+ json_decref(json);
+ return ret;
+}
+
+
+static json_t *
+virJSONValueToJansson(virJSONValuePtr object)
+{
+ json_error_t error;
+ json_t *ret = NULL;
+ size_t i;
+
+ switch (object->type) {
+ case VIR_JSON_TYPE_OBJECT:
+ ret = json_object();
+ if (!ret) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to create JSON object: %s"),
+ error.text);
+ goto error;
+ }
+ for (i = 0; i < object->data.object.npairs; i++) {
+ virJSONObjectPairPtr cur = object->data.object.pairs + i;
+ json_t *val = virJSONValueToJansson(cur->value);
+
+ if (!val)
+ goto error;
+ if (json_object_set_new(ret, cur->key, val) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to set JSON object: %s"),
+ error.text);
+ goto error;
+ }
+ }
+ break;
+
+ case VIR_JSON_TYPE_ARRAY:
+ ret = json_array();
+ if (!ret) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to create JSON array: %s"),
+ error.text);
+ goto error;
+ }
+ for (i = 0; i < object->data.array.nvalues; i++) {
+ virJSONValuePtr cur = object->data.array.values[i];
+ json_t *val = virJSONValueToJansson(cur);
+
+ if (!val)
+ goto error;
+ if (json_array_append_new(ret, val) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to append array value: %s"),
+ error.text);
+ goto error;
+ }
+ }
+ break;
+
+ case VIR_JSON_TYPE_STRING:
+ ret = json_string(object->data.string);
+ break;
+
+ case VIR_JSON_TYPE_NUMBER: {
+ long long ll_val;
+ double d_val;
+ if (virStrToLong_ll(object->data.number, NULL, 10, &ll_val) < 0) {
+ if (virStrToDouble(object->data.number, NULL, &d_val) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("that don't seem like no kind of number to me"));
+ return NULL;
+ }
+ ret = json_real(d_val);
+ } else {
+ ret = json_integer(ll_val);
+ }
+ }
+ break;
+
+ case VIR_JSON_TYPE_BOOLEAN:
+ ret = json_boolean(object->data.boolean);
+ break;
+
+ case VIR_JSON_TYPE_NULL:
+ ret = json_null();
+ break;
+ }
+ if (!ret) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("error creating JSON value: %s"),
+ error.text);
+ }
+ return ret;
+
+ error:
+ json_decref(ret);
+ return NULL;
+}
+
+
+char *
+virJSONValueToString(virJSONValuePtr object,
+ bool pretty)
+{
+ size_t flags = JSON_ENCODE_ANY;
+ json_t *json;
+ json_error_t error;
+ char *str = NULL;
+
+ if (pretty)
+ flags |= JSON_INDENT(2);
+ else
+ flags |= JSON_COMPACT;
+
+ json = virJSONValueToJansson(object);
+ if (!json)
+ return NULL;
+
+ str = json_dumps(json, flags);
+ if (!str) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to format JSON %d:%d: %s"),
+ error.line, error.column, error.text);
+ }
+ json_decref(json);
+ return str;
+}
+
+
#else
virJSONValuePtr
virJSONValueFromString(const char *jsonstring ATTRIBUTE_UNUSED)
--
2.16.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-listOn Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote:
> Check for the presence of Jansson library and prefer it to yajl
> if possible.
>
> The minimum required version is 2.7.
>
> Internally, virJSONValue still stores numbers as strings even
> though Jansson uses numeric variables for them.
>
> The configure script is particularly hideous, but will hopefully
> go away after we stop aiming to support compiling on CentOS 6.
>
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
> configure.ac | 1 +
> m4/virt-json.m4 | 55 +++++++++++---
> src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 264 insertions(+), 11 deletions(-)
> diff --git a/m4/virt-json.m4 b/m4/virt-json.m4
> index 5e4bcc7c9..a5ae3edcd 100644
> --- a/m4/virt-json.m4
> +++ b/m4/virt-json.m4
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index 6a02ddf0c..86cbd6eef 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -1951,6 +1951,225 @@ virJSONValueToString(virJSONValuePtr object,
> }
>
>
> +#elif WITH_JANSSON
> +# include <jansson.h>
> +
> +static virJSONValuePtr
> +virJSONValueFromJansson(json_t *json)
> +{
> + virJSONValuePtr ret = NULL;
> + const char *key;
> + json_t *cur;
> + size_t i;
> +
> + switch (json_typeof(json)) {
> + case JSON_OBJECT:
> + ret = virJSONValueNewObject();
> + if (!ret)
> + goto error;
> +
> + json_object_foreach(json, key, cur) {
> + virJSONValuePtr val = virJSONValueFromJansson(cur);
> + if (!val)
> + goto error;
> +
> + if (virJSONValueObjectAppend(ret, key, val) < 0)
> + goto error;
'val' will be leaked on failure
> + }
> +
> + break;
> +
> + case JSON_ARRAY:
> + ret = virJSONValueNewArray();
> + if (!ret)
> + goto error;
> +
> + json_array_foreach(json, i, cur) {
> + virJSONValuePtr val = virJSONValueFromJansson(cur);
> + if (!val)
> + goto error;
> +
> + if (virJSONValueArrayAppend(ret, val) < 0)
> + goto error;
here too
> + }
> + break;
> +
> + case JSON_STRING:
> + ret = virJSONValueNewStringLen(json_string_value(json),
> + json_string_length(json));
> + break;
> +
> + case JSON_INTEGER:
> + ret = virJSONValueNewNumberLong(json_integer_value(json));
> + break;
> +
> + case JSON_REAL:
> + ret = virJSONValueNewNumberDouble(json_real_value(json));
> + break;
After mi privatization of struct _virJSONValue it should be simple
enough to add the same differetiation to our code.
Not sure whether that's worth though.
> +
> + case JSON_TRUE:
> + case JSON_FALSE:
> + ret = virJSONValueNewBoolean(json_boolean_value(json));
> + break;
> +
> + case JSON_NULL:
> + ret = virJSONValueNewNull();
> + break;
> + }
> +
> + return ret;
> +
> + error:
> + virJSONValueFree(ret);
> + return NULL;
> +}
> +
> +virJSONValuePtr
> +virJSONValueFromString(const char *jsonstring)
> +{
> + virJSONValuePtr ret = NULL;
> + json_t *json;
> + json_error_t error;
> + size_t flags = JSON_REJECT_DUPLICATES |
> + JSON_DECODE_ANY;
> +
> + if (!(json = json_loads(jsonstring, flags, &error))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("failed to parse JSON %d:%d: %s"),
> + error.line, error.column, error.text);
> + return NULL;
> + }
> +
> + ret = virJSONValueFromJansson(json);
> + json_decref(json);
> + return ret;
> +}
> +
> +
> +static json_t *
> +virJSONValueToJansson(virJSONValuePtr object)
> +{
> + json_error_t error;
> + json_t *ret = NULL;
> + size_t i;
> +
> + switch (object->type) {
> + case VIR_JSON_TYPE_OBJECT:
> + ret = json_object();
> + if (!ret) {
So this would be the second copy of a similar function. I propose that
we replace the formatter which in this case copies everything from our
structs into structs of janson to format it with a formatter which
directly uses virBuffer to do so.
https://www.redhat.com/archives/libvir-list/2018-March/msg01935.html
This will allow us to have a single copy of the formatter and
additionally it will not depend on the library.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Sat, Mar 31, 2018 at 11:13:13AM +0200, Peter Krempa wrote:
> On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote:
> > Check for the presence of Jansson library and prefer it to yajl
> > if possible.
> >
> > The minimum required version is 2.7.
> >
> > Internally, virJSONValue still stores numbers as strings even
> > though Jansson uses numeric variables for them.
> >
> > The configure script is particularly hideous, but will hopefully
> > go away after we stop aiming to support compiling on CentOS 6.
> >
> > Signed-off-by: Ján Tomko <jtomko@redhat.com>
> > ---
> > configure.ac | 1 +
> > m4/virt-json.m4 | 55 +++++++++++---
> > src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 264 insertions(+), 11 deletions(-)
>
> > diff --git a/m4/virt-json.m4 b/m4/virt-json.m4
> > index 5e4bcc7c9..a5ae3edcd 100644
> > --- a/m4/virt-json.m4
> > +++ b/m4/virt-json.m4
>
>
> > diff --git a/src/util/virjson.c b/src/util/virjson.c
> > index 6a02ddf0c..86cbd6eef 100644
> > --- a/src/util/virjson.c
> > +++ b/src/util/virjson.c
> > @@ -1951,6 +1951,225 @@ virJSONValueToString(virJSONValuePtr object,
> > }
> >
> >
> > +#elif WITH_JANSSON
> > +# include <jansson.h>
> > +
> > +static virJSONValuePtr
> > +virJSONValueFromJansson(json_t *json)
> > +{
> > + virJSONValuePtr ret = NULL;
> > + const char *key;
> > + json_t *cur;
> > + size_t i;
> > +
> > + switch (json_typeof(json)) {
> > + case JSON_OBJECT:
> > + ret = virJSONValueNewObject();
> > + if (!ret)
> > + goto error;
> > +
> > + json_object_foreach(json, key, cur) {
> > + virJSONValuePtr val = virJSONValueFromJansson(cur);
> > + if (!val)
> > + goto error;
> > +
> > + if (virJSONValueObjectAppend(ret, key, val) < 0)
> > + goto error;
>
> 'val' will be leaked on failure
>
> > + }
> > +
> > + break;
> > +
> > + case JSON_ARRAY:
> > + ret = virJSONValueNewArray();
> > + if (!ret)
> > + goto error;
> > +
> > + json_array_foreach(json, i, cur) {
> > + virJSONValuePtr val = virJSONValueFromJansson(cur);
> > + if (!val)
> > + goto error;
> > +
> > + if (virJSONValueArrayAppend(ret, val) < 0)
> > + goto error;
>
> here too
>
> > + }
> > + break;
> > +
> > + case JSON_STRING:
> > + ret = virJSONValueNewStringLen(json_string_value(json),
> > + json_string_length(json));
> > + break;
> > +
> > + case JSON_INTEGER:
> > + ret = virJSONValueNewNumberLong(json_integer_value(json));
> > + break;
> > +
> > + case JSON_REAL:
> > + ret = virJSONValueNewNumberDouble(json_real_value(json));
> > + break;
>
> After mi privatization of struct _virJSONValue it should be simple
> enough to add the same differetiation to our code.
>
> Not sure whether that's worth though.
>
> > +
> > + case JSON_TRUE:
> > + case JSON_FALSE:
> > + ret = virJSONValueNewBoolean(json_boolean_value(json));
> > + break;
> > +
> > + case JSON_NULL:
> > + ret = virJSONValueNewNull();
> > + break;
> > + }
> > +
> > + return ret;
> > +
> > + error:
> > + virJSONValueFree(ret);
> > + return NULL;
> > +}
> > +
> > +virJSONValuePtr
> > +virJSONValueFromString(const char *jsonstring)
> > +{
> > + virJSONValuePtr ret = NULL;
> > + json_t *json;
> > + json_error_t error;
> > + size_t flags = JSON_REJECT_DUPLICATES |
> > + JSON_DECODE_ANY;
> > +
> > + if (!(json = json_loads(jsonstring, flags, &error))) {
> > + virReportError(VIR_ERR_INTERNAL_ERROR,
> > + _("failed to parse JSON %d:%d: %s"),
> > + error.line, error.column, error.text);
> > + return NULL;
> > + }
> > +
> > + ret = virJSONValueFromJansson(json);
> > + json_decref(json);
> > + return ret;
> > +}
> > +
> > +
> > +static json_t *
> > +virJSONValueToJansson(virJSONValuePtr object)
> > +{
> > + json_error_t error;
> > + json_t *ret = NULL;
> > + size_t i;
> > +
> > + switch (object->type) {
> > + case VIR_JSON_TYPE_OBJECT:
> > + ret = json_object();
> > + if (!ret) {
>
> So this would be the second copy of a similar function. I propose that
> we replace the formatter which in this case copies everything from our
> structs into structs of janson to format it with a formatter which
> directly uses virBuffer to do so.
Note the second copy will drop back down to 1 copy when we later
drop YAJL support, so this is not a long term problem.
>
> https://www.redhat.com/archives/libvir-list/2018-March/msg01935.html
>
> This will allow us to have a single copy of the formatter and
> additionally it will not depend on the library.
That means that we are basically reinventing JSON formatting & escaping
rules in our code. I don't think that would be a step forward. I wish
we could someday get rid of our use of virBuffer for formatting XML too
and rely on a XML library for formatting just as we do for JSON.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Apr 03, 2018 at 11:39:20 +0100, Daniel Berrange wrote:
> On Sat, Mar 31, 2018 at 11:13:13AM +0200, Peter Krempa wrote:
> > On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote:
> > > Check for the presence of Jansson library and prefer it to yajl
> > > if possible.
> > >
> > > The minimum required version is 2.7.
> > >
> > > Internally, virJSONValue still stores numbers as strings even
> > > though Jansson uses numeric variables for them.
> > >
> > > The configure script is particularly hideous, but will hopefully
> > > go away after we stop aiming to support compiling on CentOS 6.
> > >
> > > Signed-off-by: Ján Tomko <jtomko@redhat.com>
> > > ---
> > > configure.ac | 1 +
> > > m4/virt-json.m4 | 55 +++++++++++---
> > > src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 264 insertions(+), 11 deletions(-)
[...]
> > > +static json_t *
> > > +virJSONValueToJansson(virJSONValuePtr object)
> > > +{
> > > + json_error_t error;
> > > + json_t *ret = NULL;
> > > + size_t i;
> > > +
> > > + switch (object->type) {
> > > + case VIR_JSON_TYPE_OBJECT:
> > > + ret = json_object();
> > > + if (!ret) {
> >
> > So this would be the second copy of a similar function. I propose that
> > we replace the formatter which in this case copies everything from our
> > structs into structs of janson to format it with a formatter which
> > directly uses virBuffer to do so.
>
> Note the second copy will drop back down to 1 copy when we later
> drop YAJL support, so this is not a long term problem.
>
> >
> > https://www.redhat.com/archives/libvir-list/2018-March/msg01935.html
> >
> > This will allow us to have a single copy of the formatter and
> > additionally it will not depend on the library.
>
> That means that we are basically reinventing JSON formatting & escaping
> rules in our code. I don't think that would be a step forward. I wisha
What I don't find being a step forwar is that when we are formatting the
JSON with current approach we are basically translating our internal
virJSONValue tree into a second copy of that tree represented by the
janson data types, which is then used to do the formatting.
Since the escaping rules are simple enough in case of JSON and are fully
tested I don't really see a problem with that.
> we could someday get rid of our use of virBuffer for formatting XML too
> and rely on a XML library for formatting just as we do for JSON.
So are we going to ditch libxml2 eventually?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Apr 03, 2018 at 12:45:28PM +0200, Peter Krempa wrote:
> On Tue, Apr 03, 2018 at 11:39:20 +0100, Daniel Berrange wrote:
> > On Sat, Mar 31, 2018 at 11:13:13AM +0200, Peter Krempa wrote:
> > > On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote:
> > > > Check for the presence of Jansson library and prefer it to yajl
> > > > if possible.
> > > >
> > > > The minimum required version is 2.7.
> > > >
> > > > Internally, virJSONValue still stores numbers as strings even
> > > > though Jansson uses numeric variables for them.
> > > >
> > > > The configure script is particularly hideous, but will hopefully
> > > > go away after we stop aiming to support compiling on CentOS 6.
> > > >
> > > > Signed-off-by: Ján Tomko <jtomko@redhat.com>
> > > > ---
> > > > configure.ac | 1 +
> > > > m4/virt-json.m4 | 55 +++++++++++---
> > > > src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 264 insertions(+), 11 deletions(-)
>
> [...]
>
> > > > +static json_t *
> > > > +virJSONValueToJansson(virJSONValuePtr object)
> > > > +{
> > > > + json_error_t error;
> > > > + json_t *ret = NULL;
> > > > + size_t i;
> > > > +
> > > > + switch (object->type) {
> > > > + case VIR_JSON_TYPE_OBJECT:
> > > > + ret = json_object();
> > > > + if (!ret) {
> > >
> > > So this would be the second copy of a similar function. I propose that
> > > we replace the formatter which in this case copies everything from our
> > > structs into structs of janson to format it with a formatter which
> > > directly uses virBuffer to do so.
> >
> > Note the second copy will drop back down to 1 copy when we later
> > drop YAJL support, so this is not a long term problem.
> >
> > >
> > > https://www.redhat.com/archives/libvir-list/2018-March/msg01935.html
> > >
> > > This will allow us to have a single copy of the formatter and
> > > additionally it will not depend on the library.
> >
> > That means that we are basically reinventing JSON formatting & escaping
> > rules in our code. I don't think that would be a step forward. I wisha
>
> What I don't find being a step forwar is that when we are formatting the
> JSON with current approach we are basically translating our internal
> virJSONValue tree into a second copy of that tree represented by the
> janson data types, which is then used to do the formatting.
We could potentially change virJSONValue so that it is more of a wrapper
for the janson data type, so we don't have the translation step in terms
of data storage.
>
> Since the escaping rules are simple enough in case of JSON and are fully
> tested I don't really see a problem with that.
>
> > we could someday get rid of our use of virBuffer for formatting XML too
> > and rely on a XML library for formatting just as we do for JSON.
>
> So are we going to ditch libxml2 eventually?
I doubt it, I just didn't want to imply that we must use libxml for
formatting. We could use libxml, but we could use an alternate if
there was one better suited to it.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Sat, Mar 31, 2018 at 11:13:13 +0200, Peter Krempa wrote: > On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote: > > Check for the presence of Jansson library and prefer it to yajl > > if possible. > > > > The minimum required version is 2.7. > > > > Internally, virJSONValue still stores numbers as strings even > > though Jansson uses numeric variables for them. > > > > The configure script is particularly hideous, but will hopefully > > go away after we stop aiming to support compiling on CentOS 6. > > > > Signed-off-by: Ján Tomko <jtomko@redhat.com> > > --- > > configure.ac | 1 + > > m4/virt-json.m4 | 55 +++++++++++--- > > src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 264 insertions(+), 11 deletions(-) [...] > > + > > + case JSON_INTEGER: > > + ret = virJSONValueNewNumberLong(json_integer_value(json)); > > + break; > > + > > + case JSON_REAL: > > + ret = virJSONValueNewNumberDouble(json_real_value(json)); > > + break; > > After mi privatization of struct _virJSONValue it should be simple > enough to add the same differetiation to our code. As a side-note, the qemu QAPI schema differentiates these as well, thus this change would make our schema validator more strict. Also it would possibly have some test fallout -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.