From: Anthony PERARD <anthony.perard@vates.tech>
Convert yajl_gen to json_object from lib json-c.
And make use of it in qmp_prepare_cmd(), which can be compiled with
either lib.
Signed-off-by: Anthony PERARD <anthony.perard@vates.tech>
---
tools/include/libxl_json.h | 6 ++
tools/libs/light/libxl_internal.h | 7 +++
tools/libs/light/libxl_json.c | 95 +++++++++++++++++++++++++++++++
tools/libs/light/libxl_qmp.c | 53 +++++++++++++++++
4 files changed, 161 insertions(+)
diff --git a/tools/include/libxl_json.h b/tools/include/libxl_json.h
index f0b4871e0e..e2ef8151f0 100644
--- a/tools/include/libxl_json.h
+++ b/tools/include/libxl_json.h
@@ -15,12 +15,18 @@
#ifndef LIBXL_JSON_H
#define LIBXL_JSON_H
+#ifdef HAVE_LIBJSONC
+#include <json-c/json.h>
+#endif
+
+#ifdef HAVE_LIBYAJL
#include <yajl/yajl_gen.h>
#include <yajl/yajl_parse.h>
#ifdef HAVE_YAJL_YAJL_VERSION_H
# include <yajl/yajl_version.h>
#endif
+#endif
yajl_gen_status libxl__uint64_gen_json(yajl_gen hand, uint64_t val);
yajl_gen_status libxl_defbool_gen_json(yajl_gen hand, libxl_defbool *p);
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index 4b6587a27a..b66aaa779d 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -2234,9 +2234,16 @@ _hidden const libxl__json_object *libxl__json_map_get(const char *key,
*/
_hidden libxl__json_object *libxl__json_object_alloc(libxl__gc *gc_opt,
libxl__json_node_type type);
+#ifdef HAVE_LIBJSONC
+_hidden int libxl__json_object_to_json_object(libxl__gc *gc,
+ json_object **jso_out,
+ const libxl__json_object *obj);
+#endif
+#ifdef HAVE_LIBYAJL
_hidden yajl_status libxl__json_object_to_yajl_gen(libxl__gc *gc_opt,
yajl_gen hand,
const libxl__json_object *param);
+#endif
_hidden void libxl__json_object_free(libxl__gc *gc_opt,
libxl__json_object *obj);
diff --git a/tools/libs/light/libxl_json.c b/tools/libs/light/libxl_json.c
index 44ee6e213f..b26ac901d6 100644
--- a/tools/libs/light/libxl_json.c
+++ b/tools/libs/light/libxl_json.c
@@ -631,6 +631,100 @@ const libxl__json_object *libxl__json_map_get(const char *key,
return NULL;
}
+#ifdef HAVE_LIBJSONC
+int libxl__json_object_to_json_object(libxl__gc *gc,
+ json_object **jso_out,
+ const libxl__json_object *obj)
+{
+ int idx = 0;
+ int rc, r;
+
+ switch (obj->type) {
+ case JSON_NULL:
+ *jso_out = json_object_new_null();
+ return 0;
+ case JSON_BOOL:
+ *jso_out = json_object_new_boolean(obj->u.b);
+ if (!*jso_out)
+ return ERROR_NOMEM;
+ return 0;
+ case JSON_INTEGER:
+ *jso_out = json_object_new_int64(obj->u.i);
+ if (!*jso_out)
+ return ERROR_NOMEM;
+ return 0;
+ case JSON_DOUBLE:
+ *jso_out = json_object_new_double(obj->u.d);
+ if (!*jso_out)
+ return ERROR_NOMEM;
+ return 0;
+ case JSON_NUMBER:
+ *jso_out = json_object_new_string(obj->u.string);
+ if (!*jso_out)
+ return ERROR_NOMEM;
+ return 0;
+ case JSON_STRING:
+ *jso_out = json_object_new_string(obj->u.string);
+ if (!*jso_out)
+ return ERROR_NOMEM;
+ return 0;
+ case JSON_MAP: {
+ libxl__json_map_node *node = NULL;
+ json_object *map_root = json_object_new_object();
+ json_object *node_value;
+
+ for (idx = 0; idx < obj->u.map->count; idx++) {
+ if (flexarray_get(obj->u.map, idx, (void**)&node) != 0)
+ break;
+
+ rc = libxl__json_object_to_json_object(gc, &node_value, node->obj);
+ if (rc) {
+ json_object_put(map_root);
+ return rc;
+ }
+
+ r = json_object_object_add(map_root, node->map_key, node_value);
+ if (r < 0) {
+ json_object_put(node_value);
+ json_object_put(map_root);
+ return ERROR_FAIL;
+ }
+ }
+ *jso_out = map_root;
+ return 0;
+ }
+ case JSON_ARRAY: {
+ libxl__json_object *node = NULL;
+ json_object *array_root = json_object_new_array_ext(obj->u.array->count);
+ json_object *node_value;
+
+ for (idx = 0; idx < obj->u.array->count; idx++) {
+ if (flexarray_get(obj->u.array, idx, (void**)&node) != 0)
+ break;
+
+ rc = libxl__json_object_to_json_object(gc, &node_value, node);
+ if (rc) {
+ json_object_put(array_root);
+ return rc;
+ }
+ r = json_object_array_add(array_root, node_value);
+ if (r < 0) {
+ json_object_put(node_value);
+ json_object_put(array_root);
+ return ERROR_FAIL;
+ }
+ }
+ *jso_out = array_root;
+ return 0;
+ }
+ case JSON_ANY:
+ default:
+ /* JSON_ANY is not a valid value for obj->type. */
+ return ERROR_FAIL;
+ }
+}
+#endif
+#ifdef HAVE_LIBYAJL
yajl_status libxl__json_object_to_yajl_gen(libxl__gc *gc,
yajl_gen hand,
const libxl__json_object *obj)
@@ -698,6 +792,7 @@ yajl_status libxl__json_object_to_yajl_gen(libxl__gc *gc,
abort();
#undef CONVERT_YAJL_GEN_TO_STATUS
}
+#endif
/*
diff --git a/tools/libs/light/libxl_qmp.c b/tools/libs/light/libxl_qmp.c
index 84740bd4b3..94b6fdb559 100644
--- a/tools/libs/light/libxl_qmp.c
+++ b/tools/libs/light/libxl_qmp.c
@@ -61,7 +61,11 @@
#include <sys/un.h>
+#ifdef HAVE_LIBJSONC
+#include <json-c/json.h>
+#elif defined(HAVE_LIBYAJL)
#include <yajl/yajl_gen.h>
+#endif
#include "xen_list.h"
#include "libxl_internal.h"
@@ -481,13 +485,56 @@ static char *qmp_prepare_cmd(libxl__gc *gc, const char *cmd,
const libxl__json_object *args,
int id)
{
+#ifdef HAVE_LIBJSONC
+ json_object *jso = NULL;
+ json_object *jso_value = NULL;
+ /* memory for 'buf' is owned by 'jso' */
+ const char *buf;
+ int rc, r;
+#elif defined(HAVE_LIBYAJL)
yajl_gen hand = NULL;
/* memory for 'buf' is owned by 'hand' */
const unsigned char *buf;
libxl_yajl_length len;
yajl_gen_status s;
+#else
+# error Missing JSON library
+#endif
char *ret = NULL;
+#ifdef HAVE_LIBJSONC
+ jso = json_object_new_object();
+ if (!jso)
+ goto out;
+
+ jso_value = json_object_new_string(cmd);
+ if (!jso_value)
+ goto out;
+ r = json_object_object_add(jso, "execute", jso_value);
+ if (r < 0)
+ goto out;
+ jso_value = json_object_new_int(id);
+ if (!jso_value)
+ goto out;
+ r = json_object_object_add(jso, "id", jso_value);
+ if (r < 0)
+ goto out;
+ /* `jso_value` now part of `jso`, shouldn't free it anymore */
+ jso_value = NULL;
+ if (args) {
+ rc = libxl__json_object_to_json_object(gc, &jso_value, args);
+ if (rc)
+ goto out;
+ r = json_object_object_add(jso, "arguments", jso_value);
+ if (r < 0)
+ goto out;
+ jso_value = NULL;
+ }
+
+ buf = json_object_to_json_string_ext(jso, JSON_C_TO_STRING_PLAIN);
+ ret = libxl__sprintf(gc, "%s\r\n", buf);
+
+#elif defined(HAVE_LIBYAJL)
hand = libxl_yajl_gen_alloc(NULL);
if (!hand) {
@@ -516,9 +563,15 @@ static char *qmp_prepare_cmd(libxl__gc *gc, const char *cmd,
goto out;
ret = libxl__sprintf(gc, "%*.*s\r\n", (int)len, (int)len, buf);
+#endif
out:
+#ifdef HAVE_LIBJSONC
+ json_object_put(jso_value);
+ json_object_put(jso);
+#elif defined(HAVE_LIBYAJL)
yajl_gen_free(hand);
+#endif
return ret;
}
--
Anthony PERARDOn 2025-08-08 10:55, Anthony PERARD wrote:
> From: Anthony PERARD <anthony.perard@vates.tech>
>
> Convert yajl_gen to json_object from lib json-c.
>
> And make use of it in qmp_prepare_cmd(), which can be compiled with
> either lib.
>
> Signed-off-by: Anthony PERARD <anthony.perard@vates.tech>
> ---
> diff --git a/tools/libs/light/libxl_json.c b/tools/libs/light/libxl_json.c
> index 44ee6e213f..b26ac901d6 100644
> --- a/tools/libs/light/libxl_json.c
> +++ b/tools/libs/light/libxl_json.c
> @@ -631,6 +631,100 @@ const libxl__json_object *libxl__json_map_get(const char *key,
> return NULL;
> }
>
> +#ifdef HAVE_LIBJSONC
> +int libxl__json_object_to_json_object(libxl__gc *gc,
> + json_object **jso_out,
> + const libxl__json_object *obj)
> +{
> + int idx = 0;
> + int rc, r;
> +
> + switch (obj->type) {
> + case JSON_NULL:
> + *jso_out = json_object_new_null();
> + return 0;
> + case JSON_BOOL:
> + *jso_out = json_object_new_boolean(obj->u.b);
> + if (!*jso_out)
> + return ERROR_NOMEM;
> + return 0;
> + case JSON_INTEGER:
> + *jso_out = json_object_new_int64(obj->u.i);
> + if (!*jso_out)
> + return ERROR_NOMEM;
> + return 0;
> + case JSON_DOUBLE:
> + *jso_out = json_object_new_double(obj->u.d);
> + if (!*jso_out)
> + return ERROR_NOMEM;
> + return 0;
> + case JSON_NUMBER:
> + *jso_out = json_object_new_string(obj->u.string);
Is JSON_NUMBER calling json_object_new_string() correct? It looks like
the yajl code falls back to a string, so that is okay but surprising.
So I just want to double check. If so:
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Thanks,
Jason
> + if (!*jso_out)
> + return ERROR_NOMEM;
> + return 0;
On Wed, Aug 27, 2025 at 11:37:07AM -0400, Jason Andryuk wrote:
> On 2025-08-08 10:55, Anthony PERARD wrote:
> > + case JSON_NUMBER:
> > + *jso_out = json_object_new_string(obj->u.string);
>
> Is JSON_NUMBER calling json_object_new_string() correct? It looks like the
> yajl code falls back to a string, so that is okay but surprising.
Yeah, I think that's correct.
:-( maybe not. Even if we have these too comments:
In libxl_internal.h, enum libxl__json_node_type:
/* number is store in string, it's too big to be a long long or a double */
JSON_NUMBER = (1 << 4),
In json_callback_number():
/* If the conversion fail, we just store the original string. */
With yajl, we call yajl_gen_number(), which probably write 2^128 as:
340282366920938463463374607431768211456
but this new json-c generator would write instead:
"340282366920938463463374607431768211456"
I guess we might be able to replicate the same behavior by using
json_object_set_serializer() or json_object_new_double_s() (which use
the former). But I don't know if it is worth the effort. I hope we won't
have int bigger than 64 bits.
And there's probably no tests for JSON_NUMBERs. So I guess first step
would be to write a test that have numbers that can't be converted to
`long long` and see what happens.
Thanks,
--
Anthony PERARD
On 2025-08-29 09:56, Anthony PERARD wrote: > On Wed, Aug 27, 2025 at 11:37:07AM -0400, Jason Andryuk wrote: >> On 2025-08-08 10:55, Anthony PERARD wrote: >>> + case JSON_NUMBER: >>> + *jso_out = json_object_new_string(obj->u.string); >> >> Is JSON_NUMBER calling json_object_new_string() correct? It looks like the >> yajl code falls back to a string, so that is okay but surprising. > > Yeah, I think that's correct. > :-( maybe not. Even if we have these too comments: > > In libxl_internal.h, enum libxl__json_node_type: > /* number is store in string, it's too big to be a long long or a double */ > JSON_NUMBER = (1 << 4), > > In json_callback_number(): > /* If the conversion fail, we just store the original string. */ > > With yajl, we call yajl_gen_number(), which probably write 2^128 as: > > 340282366920938463463374607431768211456 > > but this new json-c generator would write instead: > > "340282366920938463463374607431768211456" > > I guess we might be able to replicate the same behavior by using > json_object_set_serializer() or json_object_new_double_s() (which use > the former). But I don't know if it is worth the effort. I hope we won't > have int bigger than 64 bits. I didn't check, but I thought uint64_t is the biggest size libxl uses. Regards, Jason
On Sun, Aug 31, 2025 at 10:51:53AM -0400, Jason Andryuk wrote:
> On 2025-08-29 09:56, Anthony PERARD wrote:
> > On Wed, Aug 27, 2025 at 11:37:07AM -0400, Jason Andryuk wrote:
> > > On 2025-08-08 10:55, Anthony PERARD wrote:
> > > > + case JSON_NUMBER:
> > > > + *jso_out = json_object_new_string(obj->u.string);
> > >
> > > Is JSON_NUMBER calling json_object_new_string() correct? It looks like the
> > > yajl code falls back to a string, so that is okay but surprising.
> >
> > Yeah, I think that's correct.
> > :-( maybe not. Even if we have these too comments:
> >
> > In libxl_internal.h, enum libxl__json_node_type:
> > /* number is store in string, it's too big to be a long long or a double */
> > JSON_NUMBER = (1 << 4),
> >
> > In json_callback_number():
> > /* If the conversion fail, we just store the original string. */
> >
> > With yajl, we call yajl_gen_number(), which probably write 2^128 as:
> >
> > 340282366920938463463374607431768211456
> >
> > but this new json-c generator would write instead:
> >
> > "340282366920938463463374607431768211456"
> >
> > I guess we might be able to replicate the same behavior by using
> > json_object_set_serializer() or json_object_new_double_s() (which use
> > the former). But I don't know if it is worth the effort. I hope we won't
> > have int bigger than 64 bits.
>
> I didn't check, but I thought uint64_t is the biggest size libxl uses.
Yes, but we also parse json that are produce else where. (could be file
saved by libxl, but also json produced by QEMU)
Anywhy, I investitaged this a bit, and it's very unlikely that a
`JSON_NUMBER` would be created, so it's even less likely that this
`case` would happen. But I've change the call to
`json_object_new_string` by:
/*
* Use json_object_new_double_s() to rewrite the number exactly as
* we parsed it. When generating the JSON string the value `0` will
* be ignored and `obj->u.string` will be written instead.
*/
*jso_out = json_object_new_double_s(0, obj->u.string);
That way, we get the same json blob, with both library.
Cheers,
--
Anthony PERARD
© 2016 - 2025 Red Hat, Inc.