[PATCH RFC 0/9] libxl: JSON infrastructure improvement

[PATCH RFC 0/9] libxl: JSON infrastructure improvement

[Wei Liu]

This is the first step to tackle the problem that domain runtime configurations
are not preserved across save / restore. There's not existing user in libxl
that makes use of the new code yet.

Libxl introduced JSON infrastructure to communicate with upstream QEMU. The
ability of exisiting JSON infrastructure is to convert libxl_FOO struct to
libxl JSON object and parse JSON object (string) returned by QEMU.

This series makes libxl able to convert a JSON string to libxl_FOO struct.
The conversion is done in two macro steps:
 1. convert JSON string to libxl JSON object (libxl__json_object)
 2. convert libxl JSON object to libxl_FOO struct

The first stage is done with existing infrastructure. This series focuses on
the second stage.

To break things down:
 * Patch 1 is only mechanical change
 * Patch 2, 3, 4 and 6 are changes to generic libxl JSON infrastructure
 * Patch 5 introduces parser functions for builtin types
 * Patch 7 and 8 are changes to code generator
 * Patch 9 contains changes to test code generator

Now we have the ability to do the following transformation:
  libxl_FOO struct -> libxl_FOO JSON object -> libxl JSON internal object ->
  libxl_FOO struct

This full cycle can be used to preserved runtime configurations of a domain.

Wei.

Wei Liu (9):
  libxl IDL: rename json_fn to json_gen_fn
  libxl_json: introduce libx__object_from_json
  libxl_internal: make JSON_* types or-able
  libxl_internal: introduce libxl__json_object_is_{null,number,double}
  libxl_json: introduce parser functions for builtin types
  libxl_json: allow basic JSON type objects generation
  libxl/gentypes.py: generate JSON object for keyed-union discriminator
  libxl IDL: generate code to parse libxl__json_object to libxl_FOO
    struct
  libxl/gentest.py: test JSON parser

 tools/libxl/gentest.py               |   27 +++-
 tools/libxl/gentypes.py              |  119 ++++++++++++++-
 tools/libxl/idl.py                   |   23 ++-
 tools/libxl/libxl_cpuid.c            |   92 ++++++++++--
 tools/libxl/libxl_internal.h         |   39 +++--
 tools/libxl/libxl_json.c             |  267 ++++++++++++++++++++++++++++++++--
 tools/libxl/libxl_json.h             |   27 +++-
 tools/libxl/libxl_nocpuid.c          |    7 +
 tools/libxl/libxl_types.idl          |   31 ++--
 tools/libxl/libxl_types_internal.idl |    4 +-
 10 files changed, 571 insertions(+), 65 deletions(-)

-- 
1.7.10.4

[PATCH RFC 1/9] libxl IDL: rename json_fn to json_gen_fn

[Wei Liu]

This json_fn is in fact used to generate string representation of a json
data structure. We will instroduce another json function to parse json
data structure in later changeset, so rename json_fn to json_gen_fn to
clarify.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/gentest.py               |    4 ++--
 tools/libxl/gentypes.py              |   12 ++++++------
 tools/libxl/idl.py                   |   10 +++++-----
 tools/libxl/libxl_types.idl          |    4 ++--
 tools/libxl/libxl_types_internal.idl |    2 +-
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
index 722b7f4..eb9a21b 100644
--- a/tools/libxl/gentest.py
+++ b/tools/libxl/gentest.py
@@ -52,7 +52,7 @@ def gen_rand_init(ty, v, indent = "    ", parent = None):
             s += "    break;\n"
         s += "}\n"
     elif isinstance(ty, idl.Struct) \
-     and (parent is None or ty.json_fn is None):
+     and (parent is None or ty.json_gen_fn is None):
         for f in [f for f in ty.fields if not f.const]:
             (nparent,fexpr) = ty.member(v, f, parent is None)
             s += gen_rand_init(f.type, fexpr, "", nparent)
@@ -243,7 +243,7 @@ int main(int argc, char **argv)
     f.write("    printf(\"Testing TYPE_to_json()\\n\");\n")
     f.write("    printf(\"----------------------\\n\");\n")
     f.write("    printf(\"\\n\");\n")
-    for ty in [t for t in types if t.json_fn is not None]:
+    for ty in [t for t in types if t.json_gen_fn is not None]:
         arg = ty.typename + "_val"
         f.write("    %s_rand_init(%s);\n" % (ty.typename, \
             ty.pass_arg(arg, isref=False, passby=idl.PASS_BY_REFERENCE)))
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index bfb95e2..67ec18d 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -229,7 +229,7 @@ def libxl_C_type_gen_json(ty, v, indent = "    ", parent = None):
                 s += "        goto out;\n"
             s += "    break;\n"
         s += "}\n"
-    elif isinstance(ty, idl.Struct) and (parent is None or ty.json_fn is None):
+    elif isinstance(ty, idl.Struct) and (parent is None or ty.json_gen_fn is None):
         s += "s = yajl_gen_map_open(hand);\n"
         s += "if (s != yajl_gen_status_ok)\n"
         s += "    goto out;\n"
@@ -243,8 +243,8 @@ def libxl_C_type_gen_json(ty, v, indent = "    ", parent = None):
         s += "if (s != yajl_gen_status_ok)\n"
         s += "    goto out;\n"
     else:
-        if ty.json_fn is not None:
-            s += "s = %s(hand, %s);\n" % (ty.json_fn, ty.pass_arg(v, parent is None))
+        if ty.json_gen_fn is not None:
+            s += "s = %s(hand, %s);\n" % (ty.json_gen_fn, ty.pass_arg(v, parent is None))
             s += "if (s != yajl_gen_status_ok)\n"
             s += "    goto out;\n"
 
@@ -341,7 +341,7 @@ if __name__ == '__main__':
                 f.write("%svoid %s(%s, %s);\n" % (ty.hidden(), ty.init_fn + "_" + ku.keyvar.name,
                                                ty.make_arg("p"),
                                                ku.keyvar.type.make_arg(ku.keyvar.name)))
-        if ty.json_fn is not None:
+        if ty.json_gen_fn is not None:
             f.write("%schar *%s_to_json(libxl_ctx *ctx, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p")))
         if isinstance(ty, idl.Enumeration):
             f.write("%sconst char *%s_to_string(%s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p")))
@@ -369,7 +369,7 @@ if __name__ == '__main__':
 
 """ % (header_json_define, header_json_define, " ".join(sys.argv)))
 
-    for ty in [ty for ty in types if ty.json_fn is not None]:
+    for ty in [ty for ty in types if ty.json_gen_fn is not None]:
         f.write("%syajl_gen_status %s_gen_json(yajl_gen hand, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
 
     f.write("\n")
@@ -426,7 +426,7 @@ if __name__ == '__main__':
         f.write("}\n")
         f.write("\n")
 
-    for ty in [t for t in types if t.json_fn is not None]:
+    for ty in [t for t in types if t.json_gen_fn is not None]:
         f.write("yajl_gen_status %s_gen_json(yajl_gen hand, %s)\n" % (ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
         f.write("{\n")
         f.write(libxl_C_type_gen_json(ty, "p"))
diff --git a/tools/libxl/idl.py b/tools/libxl/idl.py
index e4dc79b..92133a3 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -65,9 +65,9 @@ class Type(object):
         self.autogenerate_init_fn = kwargs.setdefault('autogenerate_init_fn', False)
 
         if self.typename is not None and not self.private:
-            self.json_fn = kwargs.setdefault('json_fn', self.typename + "_gen_json")
+            self.json_gen_fn = kwargs.setdefault('json_gen_fn', self.typename + "_gen_json")
         else:
-            self.json_fn = kwargs.setdefault('json_fn', None)
+            self.json_gen_fn = kwargs.setdefault('json_gen_fn', None)
 
         self.autogenerate_json = kwargs.setdefault('autogenerate_json', True)
 
@@ -118,7 +118,7 @@ class Number(Builtin):
         kwargs.setdefault('namespace', None)
         kwargs.setdefault('dispose_fn', None)
         kwargs.setdefault('signed', False)
-        kwargs.setdefault('json_fn', "yajl_gen_integer")
+        kwargs.setdefault('json_gen_fn', "yajl_gen_integer")
         self.signed = kwargs['signed']
         Builtin.__init__(self, ctype, **kwargs)
 
@@ -256,7 +256,7 @@ class KeyedUnion(Aggregate):
 
 void = Builtin("void *", namespace = None)
 bool = Builtin("bool", namespace = None,
-               json_fn = "yajl_gen_bool",
+               json_gen_fn = "yajl_gen_bool",
                autogenerate_json = False)
 
 size_t = Number("size_t", namespace = None)
@@ -269,7 +269,7 @@ uint32 = UInt(32)
 uint64 = UInt(64)
 
 string = Builtin("char *", namespace = None, dispose_fn = "free",
-                 json_fn = "libxl__string_gen_json",
+                 json_gen_fn = "libxl__string_gen_json",
                  autogenerate_json = False)
 
 class Array(Type):
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index d0b9ff0..08e83b7 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -7,8 +7,8 @@ namespace("libxl_")
 
 libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
 
-libxl_domid = Builtin("domid", json_fn = "yajl_gen_integer", autogenerate_json = False)
-libxl_devid = Builtin("devid", json_fn = "yajl_gen_integer", autogenerate_json = False, signed = True, init_val="-1")
+libxl_domid = Builtin("domid", json_gen_fn = "yajl_gen_integer", autogenerate_json = False)
+libxl_devid = Builtin("devid", json_gen_fn = "yajl_gen_integer", autogenerate_json = False, signed = True, init_val="-1")
 libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE)
 libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE)
 libxl_bitmap = Builtin("bitmap", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE)
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index cb9444f..a964851 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -1,7 +1,7 @@
 namespace("libxl__")
 hidden(True)
 
-libxl_domid = Builtin("domid", namespace="libxl_", json_fn = "yajl_gen_integer")
+libxl_domid = Builtin("domid", namespace="libxl_", json_gen_fn = "yajl_gen_integer")
 
 libxl__qmp_message_type = Enumeration("qmp_message_type", [
     (1, "QMP"),
-- 
1.7.10.4

[PATCH RFC 2/9] libxl_json: function to convert JSON object to libxl object

[Wei Liu]

Given a JSON string, we need to convert it to libxl_FOO struct.

The approach is:
JSON string -> libxl__json_object -> libxl_FOO struct

With this approach we can make use of libxl's infrastructure to do the
first half (JSON string -> libxl__json_object).

Second half is done by auto-generated code by libxl's IDL
infrastructure. IDL patch(es) will come later.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_internal.h |    9 +++++++++
 tools/libxl/libxl_json.c     |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b3a200d..34b6a26 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1494,6 +1494,15 @@ typedef yajl_gen_status (*libxl__gen_json_callback)(yajl_gen hand, void *);
 _hidden char *libxl__object_to_json(libxl_ctx *ctx, const char *type,
                                     libxl__gen_json_callback gen, void *p);
 
+typedef struct libxl__json_object libxl__json_object;
+typedef int (*libxl__parse_json_callback)(libxl_ctx *ctx,
+                                          libxl__json_object *o,
+                                          void *p);
+_hidden int libxl__object_from_json(libxl_ctx *ctx, const char *type,
+                                    libxl__parse_json_callback parse,
+                                    void *p,
+                                    const char *s);
+
   /* holds the CPUID response for a single CPUID leaf
    * input contains the value of the EAX and ECX register,
    * and each policy string contains a filter to apply to
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index 3ea56a4..002ae2d 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -794,6 +794,40 @@ out:
     return ret;
 }
 
+int libxl__object_from_json(libxl_ctx *ctx, const char *type,
+                            libxl__parse_json_callback parse,
+                            void *p, const char *s)
+{
+    GC_INIT(ctx);
+    libxl__json_object *o = NULL;
+    int ret;
+
+    o = libxl__json_parse(gc, s);
+
+    if (!o) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "unable to generate libxl__json_object from JSON representation for %s.",
+                   type);
+        ret = -1;
+        goto out;
+    }
+
+    ret = parse(ctx, o, p);
+
+    if (ret) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "unable to convert libxl__json_object to %s.",
+                   type);
+        ret = -1;
+        goto out;
+    }
+
+    ret = 0;
+out:
+    GC_FREE;
+    return ret;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
1.7.10.4

[PATCH RFC 2/9] libxl_json: introduce libx__object_from_json

[Wei Liu]

Given a JSON string, we need to convert it to libxl_FOO struct.

The approach is:
JSON string -> libxl__json_object -> libxl_FOO struct

With this approach we can make use of libxl's infrastructure to do the
first half (JSON string -> libxl__json_object).

Second half is done by auto-generated code by libxl's IDL
infrastructure. IDL patch(es) will come later.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_internal.h |    9 +++++++++
 tools/libxl/libxl_json.c     |   34 ++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b3a200d..34b6a26 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1494,6 +1494,15 @@ typedef yajl_gen_status (*libxl__gen_json_callback)(yajl_gen hand, void *);
 _hidden char *libxl__object_to_json(libxl_ctx *ctx, const char *type,
                                     libxl__gen_json_callback gen, void *p);
 
+typedef struct libxl__json_object libxl__json_object;
+typedef int (*libxl__parse_json_callback)(libxl_ctx *ctx,
+                                          libxl__json_object *o,
+                                          void *p);
+_hidden int libxl__object_from_json(libxl_ctx *ctx, const char *type,
+                                    libxl__parse_json_callback parse,
+                                    void *p,
+                                    const char *s);
+
   /* holds the CPUID response for a single CPUID leaf
    * input contains the value of the EAX and ECX register,
    * and each policy string contains a filter to apply to
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index 3ea56a4..002ae2d 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -794,6 +794,40 @@ out:
     return ret;
 }
 
+int libxl__object_from_json(libxl_ctx *ctx, const char *type,
+                            libxl__parse_json_callback parse,
+                            void *p, const char *s)
+{
+    GC_INIT(ctx);
+    libxl__json_object *o = NULL;
+    int ret;
+
+    o = libxl__json_parse(gc, s);
+
+    if (!o) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "unable to generate libxl__json_object from JSON representation for %s.",
+                   type);
+        ret = -1;
+        goto out;
+    }
+
+    ret = parse(ctx, o, p);
+
+    if (ret) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+                   "unable to convert libxl__json_object to %s.",
+                   type);
+        ret = -1;
+        goto out;
+    }
+
+    ret = 0;
+out:
+    GC_FREE;
+    return ret;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
1.7.10.4

[PATCH RFC 3/9] libxl_internal: make JSON_* types or-able

[Wei Liu]

Libxl can generate number as type JSON_INTEGER, JSON_DOUBLE or
JSON_NUMBER, string as type JSON_STRING or JSON_NULL (if string is
null).

So make JSON_* type or-able and use it in libxl__json_map_get. This is
useful when parsing a libxl__json_object to libxl_FOO struct. We can
enforce type checking on libxl__json_object in an easy way.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_internal.h |   18 +++++++++---------
 tools/libxl/libxl_json.c     |    2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 34b6a26..0d2480d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1620,16 +1620,16 @@ _hidden yajl_gen_status libxl__yajl_gen_asciiz(yajl_gen hand, const char *str);
 _hidden yajl_gen_status libxl__yajl_gen_enum(yajl_gen hand, const char *str);
 
 typedef enum {
-    JSON_NULL,
-    JSON_BOOL,
-    JSON_INTEGER,
-    JSON_DOUBLE,
+    JSON_NULL    = (1 << 0),
+    JSON_BOOL    = (1 << 1),
+    JSON_INTEGER = (1 << 2),
+    JSON_DOUBLE  = (1 << 3),
     /* number is store in string, it's too big to be a long long or a double */
-    JSON_NUMBER,
-    JSON_STRING,
-    JSON_MAP,
-    JSON_ARRAY,
-    JSON_ANY
+    JSON_NUMBER  = (1 << 4),
+    JSON_STRING  = (1 << 5),
+    JSON_MAP     = (1 << 6),
+    JSON_ARRAY   = (1 << 7),
+    JSON_ANY     = (1 << 8)
 } libxl__json_node_type;
 
 typedef struct libxl__json_object {
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index 002ae2d..da315f6 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -363,7 +363,7 @@ const libxl__json_object *libxl__json_map_get(const char *key,
                 return NULL;
             if (strcmp(key, node->map_key) == 0) {
                 if (expected_type == JSON_ANY
-                    || (node->obj && node->obj->type == expected_type)) {
+                    || (node->obj && (node->obj->type & expected_type))) {
                     return node->obj;
                 } else {
                     return NULL;
-- 
1.7.10.4

[PATCH RFC 4/9] libxl_internal: introduce libxl__json_object_is_{null, number, double}

[Wei Liu]

... which return true if json object is valid and of type
JSON_{NULL,NUMBER,DOUBLE}.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_internal.h |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 0d2480d..a2d532f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1654,6 +1654,10 @@ typedef struct {
 
 typedef struct libxl__yajl_ctx libxl__yajl_ctx;
 
+static inline bool libxl__json_object_is_null(const libxl__json_object *o)
+{
+    return o != NULL && o->type == JSON_NULL;
+}
 static inline bool libxl__json_object_is_bool(const libxl__json_object *o)
 {
     return o != NULL && o->type == JSON_BOOL;
@@ -1666,6 +1670,14 @@ static inline bool libxl__json_object_is_integer(const libxl__json_object *o)
 {
     return o != NULL && o->type == JSON_INTEGER;
 }
+static inline bool libxl__json_object_is_double(const libxl__json_object *o)
+{
+    return o != NULL && o->type == JSON_DOUBLE;
+}
+static inline bool libxl__json_object_is_number(const libxl__json_object *o)
+{
+    return o != NULL && o->type == JSON_NUMBER;
+}
 static inline bool libxl__json_object_is_map(const libxl__json_object *o)
 {
     return o != NULL && o->type == JSON_MAP;
-- 
1.7.10.4

[PATCH RFC 5/9] libxl_json: introduce parser functions for builtin types

[Wei Liu]

This changeset introduces following functions:
 * libxl_defbool_parse_json
 * libxl__bool_parse_json
 * libxl_uuid_parse_json
 * libxl_mac_parse_json
 * libxl_bitmap_parse_json
 * libxl_cpuid_policy_list_parse_json
 * libxl_string_list_parse_json
 * libxl_key_value_list_parse_json
 * libxl_hwcap_parse_json
 * libxl__integer_parse_json
 * libxl__string_parse_json

They will be used in later patch to convert the libxl__json_object
tree of a builtin type to libxl_FOO struct.

Also remove delcaration of libxl_domid_gen_json as libxl_domid uses
yajl_gen_integer to generate JSON object.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_cpuid.c   |   92 +++++++++++++++++----
 tools/libxl/libxl_json.c    |  191 +++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_json.h    |   27 +++++-
 tools/libxl/libxl_nocpuid.c |    7 ++
 4 files changed, 300 insertions(+), 17 deletions(-)

diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index dd21b78..2c725a5 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -336,29 +336,29 @@ void libxl_cpuid_set(libxl_ctx *ctx, uint32_t domid,
                      (const char**)(cpuid[i].policy), cpuid_res);
 }
 
+static const char *input_names[2] = { "leaf", "subleaf" };
+static const char *policy_names[4] = { "eax", "ebx", "ecx", "edx" };
+/*
+ * Aiming for:
+ * [
+ *     { 'leaf':    'val-eax',
+ *       'subleaf': 'val-ecx',
+ *       'eax':     'filter',
+ *       'ebx':     'filter',
+ *       'ecx':     'filter',
+ *       'edx':     'filter' },
+ *     { 'leaf':    'val-eax', ..., 'eax': 'filter', ... },
+ *     ... etc ...
+ * ]
+ */
+
 yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
                                 libxl_cpuid_policy_list *pcpuid)
 {
     libxl_cpuid_policy_list cpuid = *pcpuid;
     yajl_gen_status s;
-    const char *input_names[2] = { "leaf", "subleaf" };
-    const char *policy_names[4] = { "eax", "ebx", "ecx", "edx" };
     int i, j;
 
-    /*
-     * Aiming for:
-     * [
-     *     { 'leaf':    'val-eax',
-     *       'subleaf': 'val-ecx',
-     *       'eax':     'filter',
-     *       'ebx':     'filter',
-     *       'ecx':     'filter',
-     *       'edx':     'filter' },
-     *     { 'leaf':    'val-eax', ..., 'eax': 'filter', ... },
-     *     ... etc ...
-     * ]
-     */
-
     s = yajl_gen_array_open(hand);
     if (s != yajl_gen_status_ok) goto out;
 
@@ -396,6 +396,66 @@ out:
     return s;
 }
 
+int libxl_cpuid_policy_list_parse_json(libxl_ctx *ctx,
+                                       const libxl__json_object *o,
+                                       libxl_cpuid_policy_list *p)
+{
+    int i, size;
+    libxl_cpuid_policy_list l;
+
+    if (!libxl__json_object_is_array(o))
+        return -1;
+
+    if (!o->u.array->count)
+        return 0;
+
+    size = o->u.array->count;
+    /* need one extra slot as setinel */
+    l = *p = calloc(size + 1, sizeof(libxl_cpuid_policy));
+
+    if (!l)
+        return -1;
+
+    memset(l, 0, sizeof(libxl_cpuid_policy) * (size + 1));
+    l[size].input[0] = XEN_CPUID_INPUT_UNUSED;
+    l[size].input[1] = XEN_CPUID_INPUT_UNUSED;
+
+    for (i = 0; i < size; i++) {
+        const libxl__json_object *t;
+        int j;
+
+        if (flexarray_get(o->u.array, i, (void**)&t) != 0)
+            return -1;          /* dispose_fn will clean up memory
+                                 * allocation */
+
+        assert(libxl__json_object_is_map(t));
+
+        for (j = 0; j < 2; j++) {
+            const libxl__json_object *r;
+
+            r = libxl__json_map_get(input_names[j], t, JSON_INTEGER);
+            if (!r)
+                l[i].input[j] = XEN_CPUID_INPUT_UNUSED;
+            else
+                l[i].input[j] = r->u.i;
+        }
+
+        for (j = 0; j < 4; j++) {
+            const libxl__json_object *r;
+
+            r = libxl__json_map_get(policy_names[j], t, JSON_STRING);
+            if (!r)
+                l[i].policy[j] = NULL;
+            else
+                l[i].policy[j] = strdup(r->u.string);
+        }
+    }
+
+    assert(i == size);
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index da315f6..34ac035 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -100,6 +100,35 @@ yajl_gen_status libxl_defbool_gen_json(yajl_gen hand,
     return libxl__yajl_gen_asciiz(hand, libxl_defbool_to_string(*db));
 }
 
+int libxl_defbool_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                             libxl_defbool *p)
+{
+    if (!libxl__json_object_is_string(o))
+        return -1;
+
+    if (!strncmp(o->u.string, "<default>", strlen("<default>")))
+        p->val = 0;
+    else if (!strncmp(o->u.string, "True", strlen("True")))
+        p->val = 1;
+    else if (!strncmp(o->u.string, "False", strlen("False")))
+        p->val = -1;
+    else
+        return -1;
+
+    return 0;
+}
+
+int libxl__bool_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                           bool *p)
+{
+    if (!libxl__json_object_is_bool(o))
+        return -1;
+
+    *p = o->u.b;
+
+    return 0;
+}
+
 yajl_gen_status libxl_uuid_gen_json(yajl_gen hand,
                                     libxl_uuid *uuid)
 {
@@ -108,6 +137,15 @@ yajl_gen_status libxl_uuid_gen_json(yajl_gen hand,
     return yajl_gen_string(hand, (const unsigned char *)buf, LIBXL_UUID_FMTLEN);
 }
 
+int libxl_uuid_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                          libxl_uuid *p)
+{
+    if (!libxl__json_object_is_string(o))
+        return -1;
+
+    return libxl_uuid_from_string(p, o->u.string);
+}
+
 yajl_gen_status libxl_bitmap_gen_json(yajl_gen hand,
                                       libxl_bitmap *bitmap)
 {
@@ -128,6 +166,34 @@ out:
     return s;
 }
 
+int libxl_bitmap_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                            libxl_bitmap *p)
+{
+    int i;
+    int size;
+    const libxl__json_object *t;
+
+    if (!libxl__json_object_is_array(o))
+        return -1;
+
+    if (!o->u.array->count)
+        return 0;
+
+    t = libxl__json_array_get(o, o->u.array->count - 1);
+    size = t->u.i + 1;
+
+    libxl_bitmap_alloc(ctx, p, size);
+
+    for (i = 0; (t = libxl__json_array_get(o, i)); i++) {
+        if (!libxl__json_object_is_integer(t))
+            return -1;          /* dispose_fn will clean up memory
+                                 * allocation */
+        libxl_bitmap_set(p, t->u.i);
+    }
+
+    return 0;
+}
+
 yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
                                               libxl_key_value_list *pkvl)
 {
@@ -155,6 +221,43 @@ out:
     return s;
 }
 
+int libxl_key_value_list_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                                    libxl_key_value_list *p)
+{
+    libxl__json_map_node *node = NULL;
+    flexarray_t *maps = NULL;
+    int i, size;
+    libxl_key_value_list kvl;
+
+    if (!libxl__json_object_is_map(o))
+        return -1;
+
+    maps = o->u.map;
+    size = maps->count * 2;
+    kvl = *p = calloc(size, sizeof(char *));
+    if (!kvl)
+        return -1;
+
+    memset(kvl, 0, sizeof(char *) * size);
+
+    for (i = 0; i < maps->count; i++) {
+        int idx = i * 2;
+        if (flexarray_get(maps, i, (void**)&node) != 0)
+            return -1;
+        assert(libxl__json_object_is_string(node->obj) ||
+               libxl__json_object_is_null(node->obj));
+        kvl[idx]   = strdup(node->map_key);
+        if (libxl__json_object_is_string(node->obj))
+            kvl[idx+1] = strdup(node->obj->u.string);
+        else
+            kvl[idx+1] = NULL;
+    }
+
+    assert(i == maps->count);
+
+    return 0;
+}
+
 yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *pl)
 {
     libxl_string_list l = *pl;
@@ -176,6 +279,36 @@ out:
     return s;
 }
 
+int libxl_string_list_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                                 libxl_string_list *p)
+{
+    const libxl__json_object *t;
+    libxl_string_list l;
+    flexarray_t *array = NULL;
+    int i, size;
+
+    if (!libxl__json_object_is_array(o))
+        return -1;
+
+    array = o->u.array;
+    size = array->count;
+    /* need one extra slot as sentinel */
+    l = *p = calloc(size + 1, sizeof(char *));
+
+    if (!l)
+        return -1;
+
+    memset(l, 0, sizeof(char *) * size);
+    l[size] = NULL;
+
+    for (i = 0; (t = libxl__json_array_get(o, i)); i++) {
+        assert(libxl__json_object_is_string(t));
+        l[i] = strdup(t->u.string);
+    }
+
+    return 0;
+}
+
 yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *mac)
 {
     char buf[LIBXL_MAC_FMTLEN+1];
@@ -183,6 +316,15 @@ yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *mac)
     return yajl_gen_string(hand, (const unsigned char *)buf, LIBXL_MAC_FMTLEN);
 }
 
+int libxl_mac_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                         libxl_mac *p)
+{
+    if (!libxl__json_object_is_string(o))
+        return -1;
+
+    return libxl__parse_mac(o->u.string, *p);
+}
+
 yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand,
                                      libxl_hwcap *p)
 {
@@ -201,6 +343,27 @@ out:
     return s;
 }
 
+int libxl_hwcap_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                           libxl_hwcap *p)
+{
+    int i;
+
+    if (!libxl__json_object_is_array(o))
+        return -1;
+
+    for (i = 0; i<4; i++) {
+        const libxl__json_object *t;
+
+        t = libxl__json_array_get(o, i);
+        if (!t || !libxl__json_object_is_integer(t))
+            return -1;
+
+        (*p)[i] = t->u.i;
+    }
+
+    return 0;
+}
+
 yajl_gen_status libxl__string_gen_json(yajl_gen hand,
                                        const char *p)
 {
@@ -210,6 +373,23 @@ yajl_gen_status libxl__string_gen_json(yajl_gen hand,
         return yajl_gen_null(hand);
 }
 
+int libxl__string_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                             char **p)
+{
+    if (!libxl__json_object_is_string(o) && !libxl__json_object_is_null(o))
+        return -1;
+
+    if (libxl__json_object_is_null(o)) {
+        *p = NULL;
+    } else {
+        *p = strdup(o->u.string);
+        if (!*p)
+            return -1;
+    }
+
+    return 0;
+}
+
 /*
  * libxl__json_object helper functions
  */
@@ -828,6 +1008,17 @@ out:
     return ret;
 }
 
+int libxl__integer_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                              void *p)
+{
+    if (!libxl__json_object_is_integer(o))
+        return -1;
+
+    *((long long *)p) = o->u.i;
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_json.h b/tools/libxl/libxl_json.h
index a4dd8fc..d7bdeb5 100644
--- a/tools/libxl/libxl_json.h
+++ b/tools/libxl/libxl_json.h
@@ -22,17 +22,42 @@
 #  include <yajl/yajl_version.h>
 #endif
 
+typedef struct libxl__json_object libxl__json_object;
+
 yajl_gen_status libxl_defbool_gen_json(yajl_gen hand, libxl_defbool *p);
-yajl_gen_status libxl_domid_gen_json(yajl_gen hand, libxl_domid *p);
+int libxl_defbool_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                             libxl_defbool *p);
+int libxl__bool_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                           bool *p);
 yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, libxl_uuid *p);
+int libxl_uuid_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                          libxl_uuid *p);
 yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *p);
+int libxl_mac_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                         libxl_mac *p);
 yajl_gen_status libxl_bitmap_gen_json(yajl_gen hand, libxl_bitmap *p);
+int libxl_bitmap_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                            libxl_bitmap *p);
 yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
                                                  libxl_cpuid_policy_list *p);
+int libxl_cpuid_policy_list_parse_json(libxl_ctx *ctx,
+                                       const libxl__json_object *o,
+                                       libxl_cpuid_policy_list *p);
 yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *p);
+int libxl_string_list_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                                 libxl_string_list *p);
 yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
                                               libxl_key_value_list *p);
+int libxl_key_value_list_parse_json(libxl_ctx *ctx,
+                                    const libxl__json_object *o,
+                                    libxl_key_value_list *p);
 yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand, libxl_hwcap *p);
+int libxl_hwcap_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                           libxl_hwcap *p);
+int libxl__integer_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                              void *p);
+int libxl__string_parse_json(libxl_ctx *ctx, const libxl__json_object *o,
+                             char **p);
 
 #include <_libxl_types_json.h>
 
diff --git a/tools/libxl/libxl_nocpuid.c b/tools/libxl/libxl_nocpuid.c
index 5f7cb6a..7364dd5 100644
--- a/tools/libxl/libxl_nocpuid.c
+++ b/tools/libxl/libxl_nocpuid.c
@@ -44,6 +44,13 @@ yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
     return 0;
 }
 
+int libxl_cpuid_policy_list_parse_json(libxl_ctx *ctx,
+                                       const libxl__json_object *o,
+                                       libxl_cpuid_policy_list *p)
+{
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
1.7.10.4

[PATCH RFC 6/9] libxl_json: allow basic JSON type objects generation

[Wei Liu]

The original logis is that basic JSON types (number, string and null)
must be an element of JSON map or array. This assumption doesn't hold
true anymore when we need to return basic JSON types.

Returning basic JSON types is required for parsing number, string and
null objects back into libxl__json_object.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_json.c |   40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index 34ac035..675abe3 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -629,8 +629,14 @@ static int json_callback_null(void *opaque)
 
     obj = libxl__json_object_alloc(ctx->gc, JSON_NULL);
 
-    if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
-        return 0;
+    if (ctx->current) {
+        if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
+            return 0;
+        }
+    }
+
+    if (ctx->head == NULL) {
+        ctx->head = obj;
     }
 
     return 1;
@@ -646,8 +652,14 @@ static int json_callback_boolean(void *opaque, int boolean)
     obj = libxl__json_object_alloc(ctx->gc, JSON_BOOL);
     obj->u.b = boolean;
 
-    if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
-        return 0;
+    if (ctx->current) {
+        if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
+            return 0;
+        }
+    }
+
+    if (ctx->head == NULL) {
+        ctx->head = obj;
     }
 
     return 1;
@@ -703,8 +715,14 @@ error:
     obj->u.string = t;
 
 out:
-    if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
-        return 0;
+    if (ctx->current) {
+        if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
+            return 0;
+        }
+    }
+
+    if (ctx->head == NULL) {
+        ctx->head = obj;
     }
 
     return 1;
@@ -727,8 +745,14 @@ static int json_callback_string(void *opaque, const unsigned char *str,
     obj = libxl__json_object_alloc(ctx->gc, JSON_STRING);
     obj->u.string = t;
 
-    if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
-        return 0;
+    if (ctx->current) {
+        if (libxl__json_object_append_to(ctx->gc, obj, ctx->current) == -1) {
+            return 0;
+        }
+    }
+
+    if (ctx->head == NULL) {
+        ctx->head = obj;
     }
 
     return 1;
-- 
1.7.10.4

[PATCH RFC 7/9] libxl/gentypes.py: generate JSON object for keyed-union discriminator

[Wei Liu]

Parser relies on the discriminator to go to correct branch.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/gentypes.py |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 67ec18d..14a4d16 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -229,6 +229,11 @@ def libxl_C_type_gen_json(ty, v, indent = "    ", parent = None):
                 s += "        goto out;\n"
             s += "    break;\n"
         s += "}\n"
+        s += "s = yajl_gen_string(hand, (const unsigned char *)\"%s\", sizeof(\"%s\")-1);\n" \
+             % (ty.keyvar.name, ty.keyvar.name)
+        s += "if (s != yajl_gen_status_ok)\n"
+        s += "    goto out;\n"
+        s += libxl_C_type_gen_json(ty.keyvar.type, (parent + ty.keyvar.name), indent, parent)
     elif isinstance(ty, idl.Struct) and (parent is None or ty.json_gen_fn is None):
         s += "s = yajl_gen_map_open(hand);\n"
         s += "if (s != yajl_gen_status_ok)\n"
-- 
1.7.10.4

[PATCH RFC 8/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct

[Wei Liu]

libxl_FOO_parse_json functions are generated.

Note that these functions are used to parse libxl__json_object to
libxl__FOO struct. They don't consume JSON string.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/gentypes.py              |  102 ++++++++++++++++++++++++++++++++++
 tools/libxl/idl.py                   |   13 +++++
 tools/libxl/libxl_types.idl          |   31 ++++++-----
 tools/libxl/libxl_types_internal.idl |    4 +-
 4 files changed, 136 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 14a4d16..7ac86f7 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -270,6 +270,90 @@ def libxl_C_type_to_json(ty, v, indent = "    "):
         s = indent + s
     return s.replace("\n", "\n%s" % indent).rstrip(indent)
 
+def libxl_C_type_parse_json(ty, w, v, indent = "    ", parent = None):
+    s = ""
+    if parent is None:
+        s += "int rc = 0;\n"
+        s += "const libxl__json_object *x = o;\n"
+
+    if isinstance(ty, idl.Array):
+        if parent is None:
+            raise Exception("Array type must have a parent")
+        lenvar = parent + ty.lenvar.name
+        s += "{\n"
+        s += "    libxl__json_object *t;\n"
+        s += "    int i;\n"
+        s += "    assert(libxl__json_object_is_array(x));\n"
+        s += "    %s = x->u.array->count;\n" % lenvar
+        s += "    %s = calloc(%s, sizeof(*%s));\n" % (v, lenvar, v)
+        s += "    if (!%s) {\n" % v
+        s += "        rc = -1;\n"
+        s += "        goto out;\n"
+        s += "    }\n"
+        s += "    for (i=0; (t=libxl__json_array_get(x,i)); i++) {\n"
+        s += libxl_C_type_parse_json(ty.elem_type, "t", v+"[i]",
+                                     indent + "        ", parent)
+        s += "    }\n"
+        s += "    assert(i == %s);\n" % lenvar
+        s += "}\n"
+    elif isinstance(ty, idl.Enumeration):
+        s += "{\n"
+        s += "    const char *enum_str;\n"
+        s += "    assert(libxl__json_object_is_string(x));\n"
+        s += "    enum_str = libxl__json_object_get_string(x);\n"
+        s += "    rc = %s_from_string(enum_str, %s);\n" % (ty.typename, ty.pass_arg(v, parent is None, idl.PASS_BY_REFERENCE))
+        s += "    if (rc)\n"
+        s += "        goto out;\n"
+        s += "}\n"
+    elif isinstance(ty, idl.KeyedUnion):
+        if parent is None:
+            raise Exception("KeyedUnion type must have a parent")
+        s += "switch (%s) {\n" % (parent + ty.keyvar.name)
+        for f in ty.fields:
+            (nparent,fexpr) = ty.member(v, f, parent is None)
+            s += "case %s:\n" % f.enumname
+            if f.type is not None:
+                s += libxl_C_type_parse_json(f.type, w, fexpr, indent + "    ", nparent)
+            s += "    break;\n"
+        s += "}\n"
+    elif isinstance(ty, idl.Struct) and (parent is None or ty.json_parse_fn is None):
+        for f in [f for f in ty.fields if not f.const and not f.type.private]:
+            if isinstance(f.type, idl.KeyedUnion):
+                s += "x = libxl__json_map_get(\"%s\", %s, %s);\n" % \
+                     (f.type.keyvar.name, w, f.type.keyvar.type.json_parse_type)
+                s += "if (x) {\n"
+                (nparent,fexpr) = ty.member(v, f.type.keyvar, parent is None)
+                s += libxl_C_type_parse_json(f.type.keyvar.type, "x", fexpr, "    ", nparent)
+                s += "}\n"
+            s += "x = libxl__json_map_get(\"%s\", %s, %s);\n" % (f.name, w, f.type.json_parse_type)
+            s += "if (x) {\n"
+            (nparent,fexpr) = ty.member(v, f, parent is None)
+            s += libxl_C_type_parse_json(f.type, "x", fexpr, "    ", nparent)
+            s += "    x = x->parent;\n"
+            s += "}\n"
+    else:
+        if ty.json_parse_fn is not None:
+            s += "rc = %s(ctx, %s, &%s);\n" % (ty.json_parse_fn, w, v)
+            s += "if (rc)\n"
+            s += "    goto out;\n"
+
+    if parent is None:
+        s += "out:\n"
+        s += "return rc;\n"
+
+    if s != "":
+        s = indent +s
+    return s.replace("\n", "\n%s" % indent).rstrip(indent)
+
+def libxl_C_type_from_json(ty, v, w, indent = "    "):
+    s = ""
+    parse = "(libxl__parse_json_callback)&%s_parse_json" % ty.typename
+    s += "return libxl__object_from_json(ctx, \"%s\", %s, %s, %s);\n" % (ty.typename, parse, v, w)
+
+    if s != "":
+        s = indent + s
+    return s.replace("\n", "\n%s" % indent).rstrip(indent)
+
 def libxl_C_enum_to_string(ty, e, indent = "    "):
     s = ""
     s += "switch(%s) {\n" % e
@@ -348,6 +432,8 @@ if __name__ == '__main__':
                                                ku.keyvar.type.make_arg(ku.keyvar.name)))
         if ty.json_gen_fn is not None:
             f.write("%schar *%s_to_json(libxl_ctx *ctx, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p")))
+        if ty.json_parse_fn is not None:
+            f.write("%sint %s_from_json(libxl_ctx *ctx, %s, const char *s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
         if isinstance(ty, idl.Enumeration):
             f.write("%sconst char *%s_to_string(%s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p")))
             f.write("%sint %s_from_string(const char *s, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("e", passby=idl.PASS_BY_REFERENCE)))
@@ -377,6 +463,9 @@ if __name__ == '__main__':
     for ty in [ty for ty in types if ty.json_gen_fn is not None]:
         f.write("%syajl_gen_status %s_gen_json(yajl_gen hand, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
 
+    for ty in [ty for ty in types if ty.json_parse_fn is not None]:
+        f.write("%sint %s_parse_json(libxl_ctx *ctx, const libxl__json_object *o, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
+
     f.write("\n")
     f.write("""#endif /* %s */\n""" % header_json_define)
     f.close()
@@ -444,4 +533,17 @@ if __name__ == '__main__':
         f.write("}\n")
         f.write("\n")
 
+    for ty in [t for t in types if t.json_parse_fn is not None]:
+        f.write("int %s_parse_json(libxl_ctx *ctx, const libxl__json_object *%s, %s)\n" % (ty.typename,"o",ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
+        f.write("{\n")
+        f.write(libxl_C_type_parse_json(ty, "o", "p"))
+        f.write("}\n")
+        f.write("\n")
+
+        f.write("int %s_from_json(libxl_ctx *ctx, %s, const char *s)\n" % (ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
+        f.write("{\n")
+        f.write(libxl_C_type_from_json(ty, "p", "s"))
+        f.write("}\n")
+        f.write("\n")
+
     f.close()
diff --git a/tools/libxl/idl.py b/tools/libxl/idl.py
index 92133a3..ec1c429 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -66,8 +66,12 @@ class Type(object):
 
         if self.typename is not None and not self.private:
             self.json_gen_fn = kwargs.setdefault('json_gen_fn', self.typename + "_gen_json")
+            self.json_parse_type = kwargs.setdefault('json_parse_type', "JSON_ANY")
+            self.json_parse_fn = kwargs.setdefault('json_parse_fn', self.typename + "_parse_json")
         else:
             self.json_gen_fn = kwargs.setdefault('json_gen_fn', None)
+            self.json_parse_type = kwargs.setdefault('json_parse_type', None)
+            self.json_parse_fn = kwargs.setdefault('json_parse_fn', None)
 
         self.autogenerate_json = kwargs.setdefault('autogenerate_json', True)
 
@@ -119,6 +123,8 @@ class Number(Builtin):
         kwargs.setdefault('dispose_fn', None)
         kwargs.setdefault('signed', False)
         kwargs.setdefault('json_gen_fn', "yajl_gen_integer")
+        kwargs.setdefault('json_parse_type', "JSON_INTEGER")
+        kwargs.setdefault('json_parse_fn', "libxl__integer_parse_json")
         self.signed = kwargs['signed']
         Builtin.__init__(self, ctype, **kwargs)
 
@@ -142,6 +148,7 @@ class EnumerationValue(object):
 class Enumeration(Type):
     def __init__(self, typename, values, **kwargs):
         kwargs.setdefault('dispose_fn', None)
+        kwargs.setdefault('json_parse_type', "JSON_STRING")
         Type.__init__(self, typename, **kwargs)
 
         self.value_namespace = kwargs.setdefault('value_namespace',
@@ -171,6 +178,7 @@ class Field(object):
 class Aggregate(Type):
     """A type containing a collection of other types"""
     def __init__(self, kind, typename, fields, **kwargs):
+        kwargs.setdefault('json_parse_type', "JSON_MAP")
         Type.__init__(self, typename, **kwargs)
 
         if self.typename is not None:
@@ -257,6 +265,8 @@ class KeyedUnion(Aggregate):
 void = Builtin("void *", namespace = None)
 bool = Builtin("bool", namespace = None,
                json_gen_fn = "yajl_gen_bool",
+               json_parse_type = "JSON_BOOL",
+               json_parse_fn = "libxl__bool_parse_json",
                autogenerate_json = False)
 
 size_t = Number("size_t", namespace = None)
@@ -270,12 +280,15 @@ uint64 = UInt(64)
 
 string = Builtin("char *", namespace = None, dispose_fn = "free",
                  json_gen_fn = "libxl__string_gen_json",
+                 json_parse_type = "JSON_STRING | JSON_NULL",
+                 json_parse_fn = "libxl__string_parse_json",
                  autogenerate_json = False)
 
 class Array(Type):
     """An array of the same type"""
     def __init__(self, elem_type, lenvar_name, **kwargs):
         kwargs.setdefault('dispose_fn', 'free')
+        kwargs.setdefault('json_parse_type', 'JSON_ARRAY')
         Type.__init__(self, namespace=elem_type.namespace, typename=elem_type.rawname + " *", **kwargs)
 
         lv_kwargs = dict([(x.lstrip('lenvar_'),y) for (x,y) in kwargs.items() if x.startswith('lenvar_')])
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 08e83b7..cfde4a7 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -5,18 +5,23 @@
 
 namespace("libxl_")
 
-libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
-
-libxl_domid = Builtin("domid", json_gen_fn = "yajl_gen_integer", autogenerate_json = False)
-libxl_devid = Builtin("devid", json_gen_fn = "yajl_gen_integer", autogenerate_json = False, signed = True, init_val="-1")
-libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE)
-libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE)
-libxl_bitmap = Builtin("bitmap", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE)
-libxl_cpuid_policy_list = Builtin("cpuid_policy_list", dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE)
-
-libxl_string_list = Builtin("string_list", dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE)
-libxl_key_value_list = Builtin("key_value_list", dispose_fn="libxl_key_value_list_dispose", passby=PASS_BY_REFERENCE)
-libxl_hwcap = Builtin("hwcap", passby=PASS_BY_REFERENCE)
+libxl_defbool = Builtin("defbool", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE)
+
+libxl_domid = Builtin("domid", json_gen_fn = "yajl_gen_integer", json_parse_fn = "libxl__integer_parse_json",
+                      json_parse_type = "JSON_INTEGER", autogenerate_json = False)
+libxl_devid = Builtin("devid", json_gen_fn = "yajl_gen_integer", json_parse_fn = "libxl__integer_parse_json",
+                      json_parse_type = "JSON_INTEGER", autogenerate_json = False, signed = True, init_val="-1")
+libxl_uuid = Builtin("uuid", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE)
+libxl_mac = Builtin("mac", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE)
+libxl_bitmap = Builtin("bitmap", json_parse_type="JSON_ARRAY", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE)
+libxl_cpuid_policy_list = Builtin("cpuid_policy_list", json_parse_type="JSON_ARRAY",
+                                  dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE)
+
+libxl_string_list = Builtin("string_list", json_parse_type="JSON_ARRAY",
+                            dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE)
+libxl_key_value_list = Builtin("key_value_list", json_parse_type="JSON_MAP",
+                               dispose_fn="libxl_key_value_list_dispose", passby=PASS_BY_REFERENCE)
+libxl_hwcap = Builtin("hwcap", json_parse_type="JSON_ARRAY", passby=PASS_BY_REFERENCE)
 
 #
 # Specific integer types
@@ -575,7 +580,7 @@ libxl_event_type = Enumeration("event_type", [
 
 libxl_ev_user = UInt(64)
 
-libxl_ev_link = Builtin("ev_link", passby=PASS_BY_REFERENCE, private=True)
+libxl_ev_link = Builtin("ev_link", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE, private=True)
 
 libxl_event = Struct("event",[
     ("link",     libxl_ev_link),
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index a964851..125dbac 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -1,7 +1,9 @@
 namespace("libxl__")
 hidden(True)
 
-libxl_domid = Builtin("domid", namespace="libxl_", json_gen_fn = "yajl_gen_integer")
+libxl_domid = Builtin("domid", namespace="libxl_", json_gen_fn = "yajl_gen_integer",
+		      json_parse_fn = "libxl__integer_parse_json", json_parse_type = "JSON_INTEGER",
+		      autogenerate_json = False)
 
 libxl__qmp_message_type = Enumeration("qmp_message_type", [
     (1, "QMP"),
-- 
1.7.10.4

[PATCH RFC 9/9] libxl/gentest.py: test JSON parser

[Wei Liu]

The test is done in following steps:

1. initialise libxl_FOO struct
2. generate JSON string A for libxl_FOO struct FOO1
3. convert JSON string A to libxl_FOO struct FOO2
4. generate JSON string B for libxl_FOO struct FOO2
5. compare A and B

If A and B are the same then we are good.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/gentest.py |   23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
index eb9a21b..b92d092 100644
--- a/tools/libxl/gentest.py
+++ b/tools/libxl/gentest.py
@@ -225,10 +225,11 @@ int main(int argc, char **argv)
 """)
 
     for ty in types:
-        f.write("    %s %s_val;\n" % (ty.typename, ty.typename))
+        f.write("    %s %s_val, %s_val_new;\n" % \
+                (ty.typename, ty.typename, ty.typename))
     f.write("""
     int rc;
-    char *s;
+    char *s, *new_s;
     xentoollog_logger_stdiostream *logger;
     libxl_ctx *ctx;
 
@@ -240,20 +241,36 @@ int main(int argc, char **argv)
         exit(1);
     }
 """)
-    f.write("    printf(\"Testing TYPE_to_json()\\n\");\n")
+    f.write("    printf(\"Testing TYPE_to/from_json()\\n\");\n")
     f.write("    printf(\"----------------------\\n\");\n")
     f.write("    printf(\"\\n\");\n")
     for ty in [t for t in types if t.json_gen_fn is not None]:
         arg = ty.typename + "_val"
         f.write("    %s_rand_init(%s);\n" % (ty.typename, \
             ty.pass_arg(arg, isref=False, passby=idl.PASS_BY_REFERENCE)))
+        if not isinstance(ty, idl.Enumeration):
+            f.write("    %s_init(%s_new);\n" % (ty.typename, \
+                ty.pass_arg(arg, isref=False, passby=idl.PASS_BY_REFERENCE)))
         f.write("    s = %s_to_json(ctx, %s);\n" % \
                 (ty.typename, ty.pass_arg(arg, isref=False)))
         f.write("    printf(\"%%s: %%s\\n\", \"%s\", s);\n" % ty.typename)
         f.write("    if (s == NULL) abort();\n")
+        f.write("    rc = %s_from_json(ctx, &%s_val_new, s);\n" % \
+                (ty.typename, ty.typename))
+        f.write("    if (rc) abort();\n")
+        f.write("    new_s = %s_to_json(ctx, %s_new);\n" % \
+                (ty.typename, ty.pass_arg(arg, isref=False)))
+        f.write("    if (new_s == NULL) abort();\n")
+        f.write("    if (strcmp(s, new_s)) {\n")
+        f.write("        printf(\"Huh? Regenerated string different from original string.\\n\");\n")
+        f.write("        printf(\"Regenerated string: %s\\n\", new_s);\n")
+        f.write("        abort();\n")
+        f.write("    }\n")
         f.write("    free(s);\n")
+        f.write("    free(new_s);\n")
         if ty.dispose_fn is not None:
             f.write("    %s(&%s_val);\n" % (ty.dispose_fn, ty.typename))
+            f.write("    %s(&%s_val_new);\n" % (ty.dispose_fn, ty.typename))
         f.write("\n")
 
     f.write("    printf(\"Testing Enumerations\\n\");\n")
-- 
1.7.10.4

Re: [PATCH RFC 1/9] libxl IDL: rename json_fn to json_gen_fn

[Ian Campbell]

On Thu, 2014-04-10 at 16:40 +0100, Wei Liu wrote:
> This json_fn is in fact used to generate string representation of a json
> data structure. We will instroduce another json function to parse json

"introduce"

> data structure in later changeset, so rename json_fn to json_gen_fn to
> clarify.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [PATCH RFC 2/9] libxl_json: introduce libx__object_from_json

[Ian Campbell]

On Thu, 2014-04-10 at 16:40 +0100, Wei Liu wrote:
> Given a JSON string, we need to convert it to libxl_FOO struct.
> 
> The approach is:
> JSON string -> libxl__json_object -> libxl_FOO struct
> 
> With this approach we can make use of libxl's infrastructure to do the
> first half (JSON string -> libxl__json_object).
> 
> Second half is done by auto-generated code by libxl's IDL
> infrastructure. IDL patch(es) will come later.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxl/libxl_internal.h |    9 +++++++++
>  tools/libxl/libxl_json.c     |   34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
> 
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index b3a200d..34b6a26 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1494,6 +1494,15 @@ typedef yajl_gen_status (*libxl__gen_json_callback)(yajl_gen hand, void *);
>  _hidden char *libxl__object_to_json(libxl_ctx *ctx, const char *type,
>                                      libxl__gen_json_callback gen, void *p);
>  
> +typedef struct libxl__json_object libxl__json_object;
> +typedef int (*libxl__parse_json_callback)(libxl_ctx *ctx,
> +                                          libxl__json_object *o,
> +                                          void *p);

libxl__json_parse_callback reads slightly more naturally to me and fits
in with the existing libxl__json_* 

> +_hidden int libxl__object_from_json(libxl_ctx *ctx, const char *type,
> +                                    libxl__parse_json_callback parse,
> +                                    void *p,
> +                                    const char *s);
> +
>    /* holds the CPUID response for a single CPUID leaf
>     * input contains the value of the EAX and ECX register,
>     * and each policy string contains a filter to apply to
> diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
> index 3ea56a4..002ae2d 100644
> --- a/tools/libxl/libxl_json.c
> +++ b/tools/libxl/libxl_json.c
> @@ -794,6 +794,40 @@ out:
>      return ret;
>  }
>  
> +int libxl__object_from_json(libxl_ctx *ctx, const char *type,

Internal functions should take libxl__gc *gc and avoid all the GC_INIT,
GC_FREE stuff. You can still access ctx but as CTX (a macro which uses
gc)

> +                            libxl__parse_json_callback parse,
> +                            void *p, const char *s)
> +{
> +    GC_INIT(ctx);
> +    libxl__json_object *o = NULL;

Unnecessary initialiser.

> +    int ret;
> +
> +    o = libxl__json_parse(gc, s);
> +

Drop this blank line to cuddle the error check up with the function.

> +    if (!o) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                   "unable to generate libxl__json_object from JSON representation for %s.",
> +                   type);

You can use the LOG macro here.

I think you want "representation of %s"

> +        ret = -1;

ERROR_FAIL is more normal, but it might depend on the eventual usage.

> +        goto out;
> +    }
> +
> +    ret = parse(ctx, o, p);
> +

Drop this blank.
> +    if (ret) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> +                   "unable to convert libxl__json_object to %s.",
> +                   type);
> +        ret = -1;

LOG()  and ERROR_FAIL again?

> +        goto out;
> +    }
> +
> +    ret = 0;
> +out:
> +    GC_FREE;
> +    return ret;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C

Re: [PATCH RFC 3/9] libxl_internal: make JSON_* types or-able

[Ian Campbell]

On Thu, 2014-04-10 at 16:40 +0100, Wei Liu wrote:
> Libxl can generate number as type JSON_INTEGER, JSON_DOUBLE or
> JSON_NUMBER,

Those are three distinct types, right? Rather than NUMBER being the
union of INTERGERs and DOUBLEs?

>  string as type JSON_STRING or JSON_NULL (if string is
> null).
> 
> So make JSON_* type or-able

I think the phrase you want is "make FOO a bit-field".

> +    JSON_ANY     = (1 << 8)

Is ANY here meaning "Any of the others" or is it a specific JSON type?
If the former then should this be 0xff? (or whatever mask achieves the
aim of matching all types)

>  } libxl__json_node_type;
>  
>  typedef struct libxl__json_object {
> diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
> index 002ae2d..da315f6 100644
> --- a/tools/libxl/libxl_json.c
> +++ b/tools/libxl/libxl_json.c
> @@ -363,7 +363,7 @@ const libxl__json_object *libxl__json_map_get(const char *key,
>                  return NULL;
>              if (strcmp(key, node->map_key) == 0) {
>                  if (expected_type == JSON_ANY
> -                    || (node->obj && node->obj->type == expected_type)) {
> +                    || (node->obj && (node->obj->type & expected_type))) {
>                      return node->obj;
>                  } else {
>                      return NULL;

Re: [PATCH RFC 4/9] libxl_internal: introduce libxl__json_object_is_{null, number, double}

[Ian Campbell]

On Thu, 2014-04-10 at 16:40 +0100, Wei Liu wrote:
> ... which return true if json object is valid and of type
> JSON_{NULL,NUMBER,DOUBLE}.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

But... 
> +static inline bool libxl__json_object_is_null(const libxl__json_object *o)
> +{
> +    return o != NULL && o->type == JSON_NULL;

Shouldn't these (new and existing) use the bit field stuff you just
introduced?

Ian.

Re: [PATCH RFC 2/9] libxl_json: introduce libx__object_from_json

[Wei Liu]

On Mon, Apr 14, 2014 at 05:59:57PM +0100, Ian Campbell wrote:
> On Thu, 2014-04-10 at 16:40 +0100, Wei Liu wrote:
> > Given a JSON string, we need to convert it to libxl_FOO struct.
> > 
> > The approach is:
> > JSON string -> libxl__json_object -> libxl_FOO struct
> > 
> > With this approach we can make use of libxl's infrastructure to do the
> > first half (JSON string -> libxl__json_object).
> > 
> > Second half is done by auto-generated code by libxl's IDL
> > infrastructure. IDL patch(es) will come later.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  tools/libxl/libxl_internal.h |    9 +++++++++
> >  tools/libxl/libxl_json.c     |   34 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 43 insertions(+)
> > 
> > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > index b3a200d..34b6a26 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -1494,6 +1494,15 @@ typedef yajl_gen_status (*libxl__gen_json_callback)(yajl_gen hand, void *);
> >  _hidden char *libxl__object_to_json(libxl_ctx *ctx, const char *type,
> >                                      libxl__gen_json_callback gen, void *p);
> >  
> > +typedef struct libxl__json_object libxl__json_object;
> > +typedef int (*libxl__parse_json_callback)(libxl_ctx *ctx,
> > +                                          libxl__json_object *o,
> > +                                          void *p);
> 
> libxl__json_parse_callback reads slightly more naturally to me and fits
> in with the existing libxl__json_* 
> 

OK.

> > +_hidden int libxl__object_from_json(libxl_ctx *ctx, const char *type,
> > +                                    libxl__parse_json_callback parse,
> > +                                    void *p,
> > +                                    const char *s);
> > +
> >    /* holds the CPUID response for a single CPUID leaf
> >     * input contains the value of the EAX and ECX register,
> >     * and each policy string contains a filter to apply to
> > diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
> > index 3ea56a4..002ae2d 100644
> > --- a/tools/libxl/libxl_json.c
> > +++ b/tools/libxl/libxl_json.c
> > @@ -794,6 +794,40 @@ out:
> >      return ret;
> >  }
> >  
> > +int libxl__object_from_json(libxl_ctx *ctx, const char *type,
> 
> Internal functions should take libxl__gc *gc and avoid all the GC_INIT,
> GC_FREE stuff. You can still access ctx but as CTX (a macro which uses
> gc)
> 

Huh? But libxl__object_to_json also takes a libxl_ctx, am I missing
something?

> > +                            libxl__parse_json_callback parse,
> > +                            void *p, const char *s)
> > +{
> > +    GC_INIT(ctx);
> > +    libxl__json_object *o = NULL;
> 
> Unnecessary initialiser.
> 
> > +    int ret;
> > +
> > +    o = libxl__json_parse(gc, s);
> > +
> 
> Drop this blank line to cuddle the error check up with the function.
> 
> > +    if (!o) {
> > +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> > +                   "unable to generate libxl__json_object from JSON representation for %s.",
> > +                   type);
> 
> You can use the LOG macro here.
> 

Just following the coding style in libxl__object_to_json. But I guess
LOG is the new convention to follow. I will change this.

> I think you want "representation of %s"

Yes.

> 
> > +        ret = -1;
> 
> ERROR_FAIL is more normal, but it might depend on the eventual usage.
> 

Any non-zero negative value is OK, I think.

Wei.

Re: [PATCH RFC 3/9] libxl_internal: make JSON_* types or-able

[Wei Liu]

On Mon, Apr 14, 2014 at 06:03:03PM +0100, Ian Campbell wrote:
> On Thu, 2014-04-10 at 16:40 +0100, Wei Liu wrote:
> > Libxl can generate number as type JSON_INTEGER, JSON_DOUBLE or
> > JSON_NUMBER,
> 
> Those are three distinct types, right? Rather than NUMBER being the
> union of INTERGERs and DOUBLEs?
> 

Yes. They are distinct types.

> >  string as type JSON_STRING or JSON_NULL (if string is
> > null).
> > 
> > So make JSON_* type or-able
> 
> I think the phrase you want is "make FOO a bit-field".
> 

> > +    JSON_ANY     = (1 << 8)
> 
> Is ANY here meaning "Any of the others" or is it a specific JSON type?
> If the former then should this be 0xff? (or whatever mask achieves the
> aim of matching all types)
> 

It means "any of the others". I will make it a mask.

Wei.

Re: [PATCH RFC 2/9] libxl_json: introduce libx__object_from_json

[Ian Campbell]

On Tue, 2014-04-15 at 10:19 +0100, Wei Liu wrote:
> On Mon, Apr 14, 2014 at 05:59:57PM +0100, Ian Campbell wrote:
> > > diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
> > > index 3ea56a4..002ae2d 100644
> > > --- a/tools/libxl/libxl_json.c
> > > +++ b/tools/libxl/libxl_json.c
> > > @@ -794,6 +794,40 @@ out:
> > >      return ret;
> > >  }
> > >  
> > > +int libxl__object_from_json(libxl_ctx *ctx, const char *type,
> > 
> > Internal functions should take libxl__gc *gc and avoid all the GC_INIT,
> > GC_FREE stuff. You can still access ctx but as CTX (a macro which uses
> > gc)
> > 
> 
> Huh? But libxl__object_to_json also takes a libxl_ctx, am I missing
> something?

I guess there are exceptions to every rule ;-)

I guess this is because libxl__object_to_json is a helper and the public
functions are very thin autogenerated wrappers, so it makes sense to
bend the rules a bit.

So I suspect the same applies to your new function, sorry for the noise.

> > 
> > > +        ret = -1;
> > 
> > ERROR_FAIL is more normal, but it might depend on the eventual usage.
> > 
> 
> Any non-zero negative value is OK, I think.

Public libxl functions normally return ERROR_* so please do for
consistency. Even for internal functions it is usually best to use
ERROR_* since the error may well be propagated to the user eventually.

Also we conventionally use "rc" for variables which are going to contain
ERROR_*.

Ian.

Re: [PATCH RFC 4/9] libxl_internal: introduce libxl__json_object_is_{null, number, double}

[Wei Liu]

On Mon, Apr 14, 2014 at 06:04:27PM +0100, Ian Campbell wrote:
> On Thu, 2014-04-10 at 16:40 +0100, Wei Liu wrote:
> > ... which return true if json object is valid and of type
> > JSON_{NULL,NUMBER,DOUBLE}.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> But... 
> > +static inline bool libxl__json_object_is_null(const libxl__json_object *o)
> > +{
> > +    return o != NULL && o->type == JSON_NULL;
> 
> Shouldn't these (new and existing) use the bit field stuff you just
> introduced?
> 

No, this tests on the exact type, because a JSON object only has one
type.

The previous bit-field patch is for a different purpose. When you try to
parse a libxl_FOO field from a JSON object, the JSON object can be of
different type. For example, (key, value) pair in key_value_list type,
the generated key in JSON object is a string, but the value can be a
string or null depending on the input to JSON object generator
(libxl__object_to_json).

The bit-field patch works like this, say, if we want to parse that (key,
value) pair, we will certainly expect (JSON_STRING | JSON_NULL) in

    x = libxl__json_map_get("value", o, JSON_STRING|JSON_NULL);

but later in the actual function that parses x, x can be either
JSON_STRING or JSON_NULL; it cannot be of two types at the same time.

Wei.

> Ian.

Re: [PATCH RFC 4/9] libxl_internal: introduce libxl__json_object_is_{null, number, double}

[Ian Campbell]

On Tue, 2014-04-15 at 10:29 +0100, Wei Liu wrote:
> On Mon, Apr 14, 2014 at 06:04:27PM +0100, Ian Campbell wrote:
> > On Thu, 2014-04-10 at 16:40 +0100, Wei Liu wrote:
> > > ... which return true if json object is valid and of type
> > > JSON_{NULL,NUMBER,DOUBLE}.
> > > 
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > But... 
> > > +static inline bool libxl__json_object_is_null(const libxl__json_object *o)
> > > +{
> > > +    return o != NULL && o->type == JSON_NULL;
> > 
> > Shouldn't these (new and existing) use the bit field stuff you just
> > introduced?
> > 
> 
> No, [...]

Makes sense, thanks for the explanation.

Ian.