All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API
@ 2014-06-17  9:32 Wei Liu
  2014-06-17  9:32 ` [PATCH v7 1/9] xl / libxl: push parsing of SSID and CPU pool ID down to libxl Wei Liu
                   ` (9 more replies)
  0 siblings, 10 replies; 26+ messages in thread
From: Wei Liu @ 2014-06-17  9:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

V7 of the series. A patch to push CPU pinning is taken out as Dario has
generously offered to take up that patch.

The JSON infrastructure was tested with running testidl over and over again. A
new testidl executable was generated every run.

Rebased on top of staging.

Legend:
  A - acked
  C - changed since last version
  D - previous acked, but new change introduced so acked-by dropped
  No marker - new patch

Wei.

Wei Liu (9):
 A  xl / libxl: push parsing of SSID and CPU pool ID down to libxl
 A  libxl: libxl_uuid_copy now takes a ctx argument
    libxl_internal: functions to check default values for builtin types
 C  libxl/gentypes.py: don't generate default values
 D  libxl IDL: generate code to parse libxl__json_object to libxl_FOO
      struct
 A  libxl/gentest.py: test JSON parser
 A  libxl: copy function for builtin types
 A  libxl IDL: generate deep copy functions
 A  libxl/gentest.py: test deep copy functions

 tools/libxl/Makefile                 |    4 +-
 tools/libxl/gentest.py               |   60 +++++++-
 tools/libxl/gentypes.py              |  258 +++++++++++++++++++++++++++++++++-
 tools/libxl/idl.py                   |   30 +++-
 tools/libxl/idl.txt                  |   28 +++-
 tools/libxl/libxl.c                  |   94 ++++++++++++-
 tools/libxl/libxl.h                  |   60 +++++++-
 tools/libxl/libxl_cpuid.c            |   33 +++++
 tools/libxl/libxl_create.c           |   59 +++++++-
 tools/libxl/libxl_dm.c               |    4 +
 tools/libxl/libxl_internal.c         |    6 +
 tools/libxl/libxl_internal.h         |   30 ++++
 tools/libxl/libxl_nocpuid.c          |    6 +
 tools/libxl/libxl_types.idl          |   51 ++++---
 tools/libxl/libxl_types_internal.idl |    4 +-
 tools/libxl/libxl_utils.c            |   27 +++-
 tools/libxl/libxl_utils.h            |    4 +
 tools/libxl/libxl_uuid.c             |    8 +-
 tools/libxl/libxl_uuid.h             |    7 +-
 tools/libxl/xl_cmdimpl.c             |  107 ++++----------
 tools/libxl/xl_sxp.c                 |    7 +-
 21 files changed, 760 insertions(+), 127 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v7 1/9] xl / libxl: push parsing of SSID and CPU pool ID down to libxl
  2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
@ 2014-06-17  9:32 ` Wei Liu
  2014-06-17 22:46   ` Daniel De Graaf
  2014-06-17  9:32 ` [PATCH v7 2/9] libxl: libxl_uuid_copy now takes a ctx argument Wei Liu
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Wei Liu @ 2014-06-17  9:32 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, Wei Liu, ian.campbell, Dario Faggioli,
	ian.jackson, Daniel De Graaf

This patch pushes parsing of "init_seclabel", "seclabel",
"device_model_stubdomain_seclabel" and "pool" down to libxl level.

Originally the parsing is done in xl level, which is not ideal because
libxl won't have the truely relevant information. With this patch libxl
holds important information by itself.

The libxl IDL is extended to hold the string of labels and pool name.
And if there those strings are present they take precedence over the
numeric representations.

As all relevant structures (libxl_dominfo etc) have a field called
X_name / X_label now, a string is also copied there so that callers
won't have to do ID to name / label translation.

In order to be compatible with users of older versions of libxl, this
patch also defines LIBXL_HAVE_SSID_LABEL and LIBXL_HAVE_CPUPOOL_NAME. If
they are defined, the respective strings are available. And if those
strings are not NULL, libxl will do the parsing and ignore the numeric
values.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl.c         |   22 +++++++--
 tools/libxl/libxl.h         |   20 ++++++++
 tools/libxl/libxl_create.c  |   57 +++++++++++++++++++++++
 tools/libxl/libxl_dm.c      |    4 ++
 tools/libxl/libxl_types.idl |    6 +++
 tools/libxl/xl_cmdimpl.c    |  107 ++++++++++++-------------------------------
 tools/libxl/xl_sxp.c        |    7 +--
 7 files changed, 137 insertions(+), 86 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 4b66afc..08f1d4d 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -529,12 +529,18 @@ int libxl_domain_preserve(libxl_ctx *ctx, uint32_t domid,
     return 0;
 }
 
-static void xcinfo2xlinfo(const xc_domaininfo_t *xcinfo,
+static void xcinfo2xlinfo(libxl_ctx *ctx,
+                          const xc_domaininfo_t *xcinfo,
                           libxl_dominfo *xlinfo)
 {
+    size_t size;
+
     memcpy(&(xlinfo->uuid), xcinfo->handle, sizeof(xen_domain_handle_t));
     xlinfo->domid = xcinfo->domain;
     xlinfo->ssidref = xcinfo->ssidref;
+    if (libxl_flask_sid_to_context(ctx, xlinfo->ssidref,
+                                   &xlinfo->ssid_label, &size) < 0)
+        xlinfo->ssid_label = NULL;
 
     xlinfo->dying    = !!(xcinfo->flags&XEN_DOMINF_dying);
     xlinfo->shutdown = !!(xcinfo->flags&XEN_DOMINF_shutdown);
@@ -581,7 +587,7 @@ libxl_dominfo * libxl_list_domain(libxl_ctx *ctx, int *nb_domain_out)
     }
 
     for (i = 0; i < ret; i++) {
-        xcinfo2xlinfo(&info[i], &ptr[i]);
+        xcinfo2xlinfo(ctx, &info[i], &ptr[i]);
     }
     *nb_domain_out = ret;
     return ptr;
@@ -600,7 +606,7 @@ int libxl_domain_info(libxl_ctx *ctx, libxl_dominfo *info_r,
     if (ret==0 || xcinfo.domain != domid) return ERROR_INVAL;
 
     if (info_r)
-        xcinfo2xlinfo(&xcinfo, info_r);
+        xcinfo2xlinfo(ctx, &xcinfo, info_r);
     return 0;
 }
 
@@ -628,6 +634,11 @@ static int cpupool_info(libxl__gc *gc,
     }
 
     info->poolid = xcinfo->cpupool_id;
+    info->pool_name = libxl_cpupoolid_to_name(CTX, info->poolid);
+    if (!info->pool_name) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
     info->sched = xcinfo->sched_id;
     info->n_dom = xcinfo->n_dom;
     rc = libxl_cpu_bitmap_alloc(CTX, &info->cpumap, 0);
@@ -4172,10 +4183,13 @@ retry_transaction:
         abort_transaction = 1;
         goto out;
     }
-    xcinfo2xlinfo(&info, &ptr);
+
+    libxl_dominfo_init(&ptr);
+    xcinfo2xlinfo(ctx, &info, &ptr);
     uuid = libxl__uuid2string(gc, ptr.uuid);
     libxl__xs_write(gc, t, libxl__sprintf(gc, "/vm/%s/memory", uuid),
             "%"PRIu32, new_target_memkb / 1024);
+    libxl_dominfo_dispose(&ptr);
 
 out:
     if (!xs_transaction_end(ctx->xsh, t, abort_transaction)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 17b8a7b..8fbfa2d 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -498,6 +498,26 @@
 #define LIBXL_EXTERNAL_CALLERS_ONLY /* disappears for callers outside libxl */
 #endif
 
+/*
+ * LIBXL_HAVE_SSID_LABEL
+ *
+ * If this is defined, then libxl IDL contains string of XSM security
+ * label in all XSM related structures.
+ *
+ * If set this string takes precedence over the numeric field.
+ */
+#define LIBXL_HAVE_SSID_LABEL 1
+
+/*
+ * LIBXL_HAVE_CPUPOOL_NAME
+ *
+ * If this is defined, then libxl IDL contains string of CPU pool
+ * name in all CPU pool related structures.
+ *
+ * If set this string takes precedence over the numeric field.
+ */
+#define LIBXL_HAVE_CPUPOOL_NAME 1
+
 typedef uint8_t libxl_mac[6];
 #define LIBXL_MAC_FMT "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx"
 #define LIBXL_MAC_FMTLEN ((2*6)+5) /* 6 hex bytes plus 5 colons */
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index d015cf4..fe3bdd2 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -724,6 +724,63 @@ static void initiate_domain_create(libxl__egc *egc,
 
     domid = 0;
 
+    if (d_config->c_info.ssid_label) {
+        char *s = d_config->c_info.ssid_label;
+        ret = libxl_flask_context_to_sid(ctx, s, strlen(s),
+                                         &d_config->c_info.ssidref);
+        if (ret) {
+            if (errno == ENOSYS) {
+                LOG(WARN, "XSM Disabled: init_seclabel not supported");
+                ret = 0;
+            } else {
+                LOG(ERROR, "Invalid init_seclabel: %s", s);
+                goto error_out;
+            }
+        }
+    }
+
+    if (d_config->b_info.exec_ssid_label) {
+        char *s = d_config->b_info.exec_ssid_label;
+        ret = libxl_flask_context_to_sid(ctx, s, strlen(s),
+                                         &d_config->b_info.exec_ssidref);
+        if (ret) {
+            if (errno == ENOSYS) {
+                LOG(WARN, "XSM Disabled: seclabel not supported");
+                ret = 0;
+            } else {
+                LOG(ERROR, "Invalid seclabel: %s", s);
+                goto error_out;
+            }
+        }
+    }
+
+    if (d_config->b_info.device_model_ssid_label) {
+        char *s = d_config->b_info.device_model_ssid_label;
+        ret = libxl_flask_context_to_sid(ctx, s, strlen(s),
+                                         &d_config->b_info.device_model_ssidref);
+        if (ret) {
+            if (errno == ENOSYS) {
+                LOG(WARN,"XSM Disabled: device_model_stubdomain_seclabel not supported");
+                ret = 0;
+            } else {
+                LOG(ERROR, "Invalid device_model_stubdomain_seclabel: %s", s);
+                goto error_out;
+            }
+        }
+    }
+
+    if (d_config->c_info.pool_name) {
+        d_config->c_info.poolid = -1;
+        libxl_cpupool_qualifier_to_cpupoolid(ctx, d_config->c_info.pool_name,
+                                             &d_config->c_info.poolid,
+                                             NULL);
+    }
+    if (!libxl_cpupoolid_is_valid(ctx, d_config->c_info.poolid)) {
+        LOG(ERROR, "Illegal pool specified: %s", d_config->c_info.pool_name);
+        ret = ERROR_INVAL;
+        goto error_out;
+    }
+
     /* If target_memkb is smaller than max_memkb, the subsequent call
      * to libxc when building HVM domain will enable PoD mode.
      */
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 51ab2bf..addacdb 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -910,7 +910,11 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     dm_config->c_info.type = LIBXL_DOMAIN_TYPE_PV;
     dm_config->c_info.name = libxl__stub_dm_name(gc,
                                     libxl__domid_to_name(gc, guest_domid));
+    /* When we are here to launch stubdom, ssidref is a valid value
+     * already, no need to parse it again.
+     */
     dm_config->c_info.ssidref = guest_config->b_info.device_model_ssidref;
+    dm_config->c_info.ssid_label = NULL;
 
     libxl_uuid_generate(&dm_config->c_info.uuid);
 
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index f0f6e34..1018142 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -215,6 +215,7 @@ libxl_dominfo = Struct("dominfo",[
     ("uuid",        libxl_uuid),
     ("domid",       libxl_domid),
     ("ssidref",     uint32),
+    ("ssid_label",  string),
     ("running",     bool),
     ("blocked",     bool),
     ("paused",      bool),
@@ -240,6 +241,7 @@ libxl_dominfo = Struct("dominfo",[
 
 libxl_cpupoolinfo = Struct("cpupoolinfo", [
     ("poolid",      uint32),
+    ("pool_name",   string),
     ("sched",       libxl_scheduler),
     ("n_dom",       uint32),
     ("cpumap",      libxl_bitmap)
@@ -270,11 +272,13 @@ libxl_domain_create_info = Struct("domain_create_info",[
     ("hap",          libxl_defbool),
     ("oos",          libxl_defbool),
     ("ssidref",      uint32),
+    ("ssid_label",   string),
     ("name",         string),
     ("uuid",         libxl_uuid),
     ("xsdata",       libxl_key_value_list),
     ("platformdata", libxl_key_value_list),
     ("poolid",       uint32),
+    ("pool_name",    string),
     ("run_hotplug_scripts",libxl_defbool),
     ("pvh",          libxl_defbool),
     ("driver_domain",libxl_defbool),
@@ -307,6 +311,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("shadow_memkb",    MemKB),
     ("rtc_timeoffset",  uint32),
     ("exec_ssidref",    uint32),
+    ("exec_ssid_label", string),
     ("localtime",       libxl_defbool),
     ("disable_migrate", libxl_defbool),
     ("cpuid",           libxl_cpuid_policy_list),
@@ -317,6 +322,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
     # if you set device_model you must set device_model_version too
     ("device_model",     string),
     ("device_model_ssidref", uint32),
+    ("device_model_ssid_label", string),
 
     # extra parameters pass directly to qemu, NULL terminated
     ("extra",            libxl_string_list),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 64a1c77..be041f2 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -725,35 +725,17 @@ static void parse_config_data(const char *config_source,
         exit(1);
     }
 
-    if (!xlu_cfg_get_string (config, "init_seclabel", &buf, 0)) {
-        e = libxl_flask_context_to_sid(ctx, (char *)buf, strlen(buf),
-                                    &c_info->ssidref);
-        if (e) {
-            if (errno == ENOSYS) {
-                fprintf(stderr, "XSM Disabled: init_seclabel not supported\n");
-            } else {
-                fprintf(stderr, "Invalid init_seclabel: %s\n", buf);
-                exit(1);
-            }
-        }
-    }
+    if (!xlu_cfg_get_string (config, "init_seclabel", &buf, 0))
+        xlu_cfg_replace_string(config, "init_seclabel",
+                               &c_info->ssid_label, 0);
 
     if (!xlu_cfg_get_string (config, "seclabel", &buf, 0)) {
-        uint32_t ssidref;
-        e = libxl_flask_context_to_sid(ctx, (char *)buf, strlen(buf),
-                                    &ssidref);
-        if (e) {
-            if (errno == ENOSYS) {
-                fprintf(stderr, "XSM Disabled: seclabel not supported\n");
-            } else {
-                fprintf(stderr, "Invalid seclabel: %s\n", buf);
-                exit(1);
-            }
-        } else if (c_info->ssidref) {
-            b_info->exec_ssidref = ssidref;
-        } else {
-            c_info->ssidref = ssidref;
-        }
+        if (c_info->ssid_label)
+            xlu_cfg_replace_string(config, "seclabel",
+                                   &b_info->exec_ssid_label, 0);
+        else
+            xlu_cfg_replace_string(config, "seclabel",
+                                   &c_info->ssid_label, 0);
     }
 
     libxl_defbool_set(&c_info->run_hotplug_scripts, run_hotplug_scripts);
@@ -781,14 +763,8 @@ static void parse_config_data(const char *config_source,
 
     xlu_cfg_get_defbool(config, "oos", &c_info->oos, 0);
 
-    if (!xlu_cfg_get_string (config, "pool", &buf, 0)) {
-        c_info->poolid = -1;
-        libxl_cpupool_qualifier_to_cpupoolid(ctx, buf, &c_info->poolid, NULL);
-    }
-    if (!libxl_cpupoolid_is_valid(ctx, c_info->poolid)) {
-        fprintf(stderr, "Illegal pool specified\n");
-        exit(1);
-    }
+    if (!xlu_cfg_get_string (config, "pool", &buf, 0))
+        xlu_cfg_replace_string(config, "pool", &c_info->pool_name, 0);
 
     libxl_domain_build_info_init_type(b_info, c_info->type);
     if (blkdev_start)
@@ -1577,20 +1553,10 @@ skip_vfb:
                          &b_info->device_model_stubdomain, 0);
 
     if (!xlu_cfg_get_string (config, "device_model_stubdomain_seclabel",
-                             &buf, 0)) {
-        e = libxl_flask_context_to_sid(ctx, (char *)buf, strlen(buf),
-                                    &b_info->device_model_ssidref);
-        if (e) {
-            if (errno == ENOSYS) {
-                fprintf(stderr, "XSM Disabled:"
-                        " device_model_stubdomain_seclabel not supported\n");
-            } else {
-                fprintf(stderr, "Invalid device_model_stubdomain_seclabel:"
-                        " %s\n", buf);
-                exit(1);
-            }
-        }
-    }
+                             &buf, 0))
+        xlu_cfg_replace_string(config, "device_model_stubdomain_seclabel",
+                               &b_info->device_model_ssid_label, 0);
+
 #define parse_extra_args(type)                                            \
     e = xlu_cfg_get_list_as_string_list(config, "device_model_args"#type, \
                                     &b_info->extra##type, 0);            \
@@ -3302,15 +3268,8 @@ static void list_domains(int verbose, int context, int claim, int numa,
         }
         if (claim)
             printf(" %5lu", (unsigned long)info[i].outstanding_memkb / 1024);
-        if (verbose || context) {
-            int rc;
-            size_t size;
-            char *buf = NULL;
-            rc = libxl_flask_sid_to_context(ctx, info[i].ssidref, &buf,
-                                            &size);
-            printf(" %16s", rc < 0 ? "-" : buf);
-            free(buf);
-        }
+        if (verbose || context)
+            printf(" %16s", info[i].ssid_label ? : "-");
         if (numa) {
             libxl_domain_get_nodeaffinity(ctx, info[i].domid, &nodemap);
 
@@ -6775,27 +6734,21 @@ int main_cpupoollist(int argc, char **argv)
 
     for (p = 0; p < n_pools; p++) {
         if (!ret && (!pool || (poolinfo[p].poolid == poolid))) {
-            name = libxl_cpupoolid_to_name(ctx, poolinfo[p].poolid);
-            if (!name) {
-                fprintf(stderr, "error getting cpupool info\n");
-                ret = -ERROR_NOMEM;
-            } else {
-                printf("%-19s", name);
-                free(name);
-                n = 0;
-                libxl_for_each_bit(c, poolinfo[p].cpumap)
-                    if (libxl_bitmap_test(&poolinfo[p].cpumap, c)) {
-                        if (n && opt_cpus) printf(",");
-                        if (opt_cpus) printf("%d", c);
-                        n++;
-                    }
-                if (!opt_cpus) {
-                    printf("%3d %9s       y       %4d", n,
-                           libxl_scheduler_to_string(poolinfo[p].sched),
-                           poolinfo[p].n_dom);
+            name = poolinfo[p].pool_name;
+            printf("%-19s", name);
+            n = 0;
+            libxl_for_each_bit(c, poolinfo[p].cpumap)
+                if (libxl_bitmap_test(&poolinfo[p].cpumap, c)) {
+                    if (n && opt_cpus) printf(",");
+                    if (opt_cpus) printf("%d", c);
+                    n++;
                 }
-                printf("\n");
+            if (!opt_cpus) {
+                printf("%3d %9s       y       %4d", n,
+                       libxl_scheduler_to_string(poolinfo[p].sched),
+                       poolinfo[p].n_dom);
             }
+            printf("\n");
         }
     }
 
diff --git a/tools/libxl/xl_sxp.c b/tools/libxl/xl_sxp.c
index a16a025..48eb608 100644
--- a/tools/libxl/xl_sxp.c
+++ b/tools/libxl/xl_sxp.c
@@ -37,7 +37,6 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
 
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
-    char *pool;
 
     printf("(domain\n\t(domid %d)\n", domid);
     printf("\t(create_info)\n");
@@ -55,10 +54,8 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
     } else {
         printf("\t(uuid <unknown>)\n");
     }
-    pool = libxl_cpupoolid_to_name(ctx, c_info->poolid);
-    if (pool)
-        printf("\t(cpupool %s)\n", pool);
-    free(pool);
+    if (c_info->pool_name)
+        printf("\t(cpupool %s)\n", c_info->pool_name);
     if (c_info->xsdata)
         printf("\t(xsdata contains data)\n");
     else
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v7 2/9] libxl: libxl_uuid_copy now takes a ctx argument
  2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
  2014-06-17  9:32 ` [PATCH v7 1/9] xl / libxl: push parsing of SSID and CPU pool ID down to libxl Wei Liu
@ 2014-06-17  9:32 ` Wei Liu
  2014-06-17  9:32 ` [PATCH v7 3/9] libxl_internal: functions to check default values for builtin types Wei Liu
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Wei Liu @ 2014-06-17  9:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

Make it consistent with existing libxl functions like libxl_bitmap_copy
etc.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl.c        |    2 +-
 tools/libxl/libxl.h        |   14 +++++++++++---
 tools/libxl/libxl_create.c |    2 +-
 tools/libxl/libxl_utils.c  |    2 +-
 tools/libxl/libxl_uuid.c   |    8 ++++----
 tools/libxl/libxl_uuid.h   |    7 ++++++-
 6 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 08f1d4d..38d4b2a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2019,7 +2019,7 @@ int libxl_devid_to_device_vtpm(libxl_ctx *ctx,
         if(devid == vtpms[i].devid) {
             vtpm->backend_domid = vtpms[i].backend_domid;
             vtpm->devid = vtpms[i].devid;
-            libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
+            libxl_uuid_copy(ctx, &vtpm->uuid, &vtpms[i].uuid);
             rc = 0;
             break;
         }
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 8fbfa2d..69ceac8 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -319,13 +319,15 @@
 
 #include <xentoollog.h>
 
+typedef struct libxl__ctx libxl_ctx;
+
 #include <libxl_uuid.h>
 #include <_libxl_list.h>
 
 /* API compatibility. */
 #ifdef LIBXL_API_VERSION
 #if LIBXL_API_VERSION != 0x040200 && LIBXL_API_VERSION != 0x040300 && \
-    LIBXL_API_VERSION != 0x040400
+    LIBXL_API_VERSION != 0x040400 && LIBXL_API_VERSION != 0x040500
 #error Unknown LIBXL_API_VERSION
 #endif
 #endif
@@ -499,6 +501,14 @@
 #endif
 
 /*
+ *  LIBXL_HAVE_UUID_COPY_CTX_PARAM
+ *
+ * If this is defined, libxl_uuid_copy has changed to take a libxl_ctx
+ * structure.
+ */
+#define LIBXL_HAVE_UUID_COPY_CTX_PARAM 1
+
+/*
  * LIBXL_HAVE_SSID_LABEL
  *
  * If this is defined, then libxl IDL contains string of XSM security
@@ -594,8 +604,6 @@ bool libxl_defbool_val(libxl_defbool db);
 
 const char *libxl_defbool_to_string(libxl_defbool b);
 
-typedef struct libxl__ctx libxl_ctx;
-
 #define LIBXL_TIMER_MODE_DEFAULT -1
 #define LIBXL_MEMKB_DEFAULT ~0ULL
 
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index fe3bdd2..da1517c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -479,7 +479,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_create_info *info,
     *domid = -1;
 
     /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
-    libxl_uuid_copy((libxl_uuid *)handle, &info->uuid);
+    libxl_uuid_copy(ctx, (libxl_uuid *)handle, &info->uuid);
 
     ret = xc_domain_create(ctx->xch, info->ssidref, handle, flags, domid);
     if (ret < 0) {
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 476921e..16b734e 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -536,7 +536,7 @@ int libxl_uuid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
         if(!libxl_uuid_compare(uuid, &vtpms[i].uuid)) {
             vtpm->backend_domid = vtpms[i].backend_domid;
             vtpm->devid = vtpms[i].devid;
-            libxl_uuid_copy(&vtpm->uuid, &vtpms[i].uuid);
+            libxl_uuid_copy(ctx, &vtpm->uuid, &vtpms[i].uuid);
             rc = 0;
             break;
         }
diff --git a/tools/libxl/libxl_uuid.c b/tools/libxl/libxl_uuid.c
index 6591cb7..172b7d1 100644
--- a/tools/libxl/libxl_uuid.c
+++ b/tools/libxl/libxl_uuid.c
@@ -14,8 +14,6 @@
 
 #include "libxl_osdeps.h" /* must come before any other headers */
 
-#include <libxl_uuid.h>
-
 #include "libxl_internal.h"
 
 #if defined(__linux__)
@@ -35,7 +33,8 @@ int libxl_uuid_from_string(libxl_uuid *uuid, const char *in)
      return uuid_parse(in, uuid->uuid);
 }
 
-void libxl_uuid_copy(libxl_uuid *dst, const libxl_uuid *src)
+void libxl_uuid_copy(libxl_ctx *ctx_opt, libxl_uuid *dst,
+                     const libxl_uuid *src)
 {
      uuid_copy(dst->uuid, src->uuid);
 }
@@ -87,7 +86,8 @@ int libxl_uuid_from_string(libxl_uuid *uuid, const char *in)
 }
 #undef LIBXL__UUID_PTRS
 
-void libxl_uuid_copy(libxl_uuid *dst, const libxl_uuid *src)
+void libxl_uuid_copy(libxl_ctx *ctx_opt, libxl_uuid *dst,
+                     const libxl_uuid *src)
 {
      memcpy(dst->uuid, src->uuid, sizeof(dst->uuid));
 }
diff --git a/tools/libxl/libxl_uuid.h b/tools/libxl/libxl_uuid.h
index fbde7b6..0c2a1e7 100644
--- a/tools/libxl/libxl_uuid.h
+++ b/tools/libxl/libxl_uuid.h
@@ -56,7 +56,12 @@ typedef struct {
 int libxl_uuid_is_nil(const libxl_uuid *uuid);
 void libxl_uuid_generate(libxl_uuid *uuid);
 int libxl_uuid_from_string(libxl_uuid *uuid, const char *in);
-void libxl_uuid_copy(libxl_uuid *dst, const libxl_uuid *src);
+void libxl_uuid_copy(libxl_ctx *ctx_opt, libxl_uuid *dst,
+                     const libxl_uuid *src);
+#if defined(LIBXL_API_VERSION) && LIBXL_API_VERSION < 0x040500
+void libxl_uuid_copy(dst, src) libxl_uuid_copy(NULL, dst, src)
+#endif
+
 void libxl_uuid_clear(libxl_uuid *uuid);
 int libxl_uuid_compare(const libxl_uuid *uuid1, const libxl_uuid *uuid2);
 const uint8_t *libxl_uuid_bytearray_const(const libxl_uuid *uuid);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v7 3/9] libxl_internal: functions to check default values for builtin types
  2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
  2014-06-17  9:32 ` [PATCH v7 1/9] xl / libxl: push parsing of SSID and CPU pool ID down to libxl Wei Liu
  2014-06-17  9:32 ` [PATCH v7 2/9] libxl: libxl_uuid_copy now takes a ctx argument Wei Liu
@ 2014-06-17  9:32 ` Wei Liu
  2014-06-18 14:07   ` Ian Campbell
  2014-06-17  9:32 ` [PATCH v7 4/9] libxl/gentypes.py: don't generate default values Wei Liu
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Wei Liu @ 2014-06-17  9:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

They will be used in later patch to determine whether we should generate
JSON output for a type. If that type has default value we just skip
generating JSON output.

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

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 38d4b2a..1b0b08f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2858,8 +2858,7 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
         nic->model = strdup("rtl8139");
         if (!nic->model) return ERROR_NOMEM;
     }
-    if (!nic->mac[0] && !nic->mac[1] && !nic->mac[2] &&
-        !nic->mac[3] && !nic->mac[4] && !nic->mac[5]) {
+    if (libxl__mac_is_default(&nic->mac)) {
         const uint8_t *r;
         libxl_uuid uuid;
 
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 6c94d3e..81f8985 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -307,6 +307,12 @@ _hidden int libxl__compare_macs(libxl_mac *a, libxl_mac *b)
     return 0;
 }
 
+_hidden int libxl__mac_is_default(libxl_mac *mac)
+{
+    return (!(*mac)[0] && !(*mac)[1] && !(*mac)[2] &&
+            !(*mac)[3] && !(*mac)[4] && !(*mac)[5]);
+}
+
 _hidden int libxl__init_recursive_mutex(libxl_ctx *ctx, pthread_mutex_t *lock)
 {
     pthread_mutexattr_t attr;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index a0d4f24..2a8f552 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1551,6 +1551,8 @@ struct libxl__xen_console_reader {
 _hidden int libxl__parse_mac(const char *s, libxl_mac mac);
 /* compare mac address @a and @b. 0 if the same, -ve if a<b and +ve if a>b */
 _hidden int libxl__compare_macs(libxl_mac *a, libxl_mac *b);
+/* return true if mac address is all zero (the default value) */
+_hidden int libxl__mac_is_default(libxl_mac *mac);
 /* init a recursive mutex */
 _hidden int libxl__init_recursive_mutex(libxl_ctx *ctx, pthread_mutex_t *lock);
 
@@ -3097,6 +3099,10 @@ void libxl__numa_candidate_put_nodemap(libxl__gc *gc,
 #define LIBXL__DEFBOOL_STR_DEFAULT "<default>"
 #define LIBXL__DEFBOOL_STR_FALSE   "False"
 #define LIBXL__DEFBOOL_STR_TRUE    "True"
+static inline int libxl__defbool_is_default(libxl_defbool *db)
+{
+    return !db->val;
+}
 
 /*
  * Inserts "elm_new" into the sorted list "head".
@@ -3180,6 +3186,27 @@ int libxl__uint64_parse_json(libxl__gc *gc, const libxl__json_object *o,
 int libxl__string_parse_json(libxl__gc *gc, const libxl__json_object *o,
                              char **p);
 
+/* This always return false, there's no "default value" for hw cap */
+static inline int libxl__hwcap_is_default(libxl_hwcap *hwcap)
+{
+    return 0;
+}
+
+static inline int libxl__string_list_is_empty(libxl_string_list *psl)
+{
+    return !libxl_string_list_length(psl);
+}
+
+static inline int libxl__key_value_list_is_empty(libxl_key_value_list *pkvl)
+{
+    return !libxl_key_value_list_length(pkvl);
+}
+
+static inline int libxl__cpuid_policy_is_empty(libxl_cpuid_policy_list *pl)
+{
+    return !libxl_cpuid_policy_list_length(pl);
+}
+
 #endif
 
 /*
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v7 4/9] libxl/gentypes.py: don't generate default values
  2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
                   ` (2 preceding siblings ...)
  2014-06-17  9:32 ` [PATCH v7 3/9] libxl_internal: functions to check default values for builtin types Wei Liu
@ 2014-06-17  9:32 ` Wei Liu
  2014-06-18 14:13   ` Ian Campbell
  2014-06-17  9:32 ` [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct Wei Liu
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Wei Liu @ 2014-06-17  9:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

If a field:
    0. is not of aggregate type
and
    1. is of array type and the array is not empty
or  2. is of a type which has init_val and has been set to init_val,
or  3. is of builtin type and has been set to internal default value,
or  4. has been set to 0

then there's no need to generate output for that field in JSON
output.

Note that 0 can result in output like
  {
    ...
    FOO : { }
    ...
  }
where FOO is aggregate type but all its fields are set to default, hence
no JSON output in {} at all. This is not pretty, but it's still valid
JSON. And the parser should be able to skip touching those fields in the
resulting C structures. When the parser consumes that generated JSON
object, all default values should be automatically filled in.

Also change some non-zero init_vals to LIBXL_* for better readability in
generated C code.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/gentypes.py     |   33 +++++++++++++++++++++++++++++++--
 tools/libxl/idl.py          |    2 ++
 tools/libxl/idl.txt         |    9 +++++++++
 tools/libxl/libxl_types.idl |   29 +++++++++++++++++------------
 4 files changed, 59 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 01416e7..f06afd9 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -206,6 +206,29 @@ def libxl_C_type_gen_map_key(f, parent, indent = ""):
         s = indent + s
     return s.replace("\n", "\n%s" % indent).rstrip(indent)
 
+def get_init_val(f):
+    if f.init_val is not None:
+        return f.init_val
+    elif f.type.init_val is not None:
+        return f.type.init_val
+    return None
+
+def get_default_expr(f, nparent, fexpr):
+    if isinstance(f.type, idl.Aggregate):
+        return "1 /* always generate JSON output for aggregate type */"
+
+    if isinstance(f.type, idl.Array):
+        return "%s && %s" % (fexpr, nparent + f.type.lenvar.name)
+
+    init_val = get_init_val(f)
+    if init_val is not None:
+        return "%s != %s" % (fexpr, init_val)
+
+    if f.type.check_default_fn:
+        return "!%s(&%s)" % (f.type.check_default_fn, fexpr)
+
+    return "%s" % fexpr
+
 def libxl_C_type_gen_json(ty, v, indent = "    ", parent = None):
     s = ""
     if parent is None:
@@ -255,8 +278,14 @@ def libxl_C_type_gen_json(ty, v, indent = "    ", parent = None):
         s += "    goto out;\n"
         for f in [f for f in ty.fields if not f.const and not f.type.private]:
             (nparent,fexpr) = ty.member(v, f, parent is None)
-            s += libxl_C_type_gen_map_key(f, nparent)
-            s += libxl_C_type_gen_json(f.type, fexpr, "", nparent)
+            default_expr = get_default_expr(f, nparent, fexpr)
+            s += "if (%s) {\n" % default_expr
+
+            s += libxl_C_type_gen_map_key(f, nparent, "    ")
+            s += libxl_C_type_gen_json(f.type, fexpr, "    ", nparent)
+
+            s += "}\n"
+
         s += "s = yajl_gen_map_close(hand);\n"
         s += "if (s != yajl_gen_status_ok)\n"
         s += "    goto out;\n"
diff --git a/tools/libxl/idl.py b/tools/libxl/idl.py
index 8b118dd..ada801a 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -64,6 +64,8 @@ class Type(object):
         self.init_val = kwargs.setdefault('init_val', None)
         self.autogenerate_init_fn = kwargs.setdefault('autogenerate_init_fn', False)
 
+        self.check_default_fn = kwargs.setdefault('check_default_fn', None)
+
         if self.typename is not None and not self.private:
             self.json_gen_fn = kwargs.setdefault('json_gen_fn', self.typename + "_gen_json")
         else:
diff --git a/tools/libxl/idl.txt b/tools/libxl/idl.txt
index 6a53dd8..d47eba8 100644
--- a/tools/libxl/idl.txt
+++ b/tools/libxl/idl.txt
@@ -69,6 +69,15 @@ Type.autogenerate_json: (default: True)
 
  Indicates if the above named Type.json_gen_fn should be autogenerated.
 
+Type.check_default_fn:
+
+ If it's set then calling this function shall return true if this type
+ has been set to default value (internal libxl implementation).
+
+ If this is not set, that means we can check the type against init_val
+ (if it has one) or zero to determine whether the value is default
+ value.
+
 Other simple type-Classes
 -------------------------
 
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 1018142..7234519 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_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE, check_default_fn="libxl__defbool_is_default")
 
 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_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE, check_default_fn="libxl_uuid_is_nil")
+libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE, check_default_fn="libxl__mac_is_default")
+libxl_bitmap = Builtin("bitmap", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE,
+                       check_default_fn="libxl_bitmap_is_empty")
+libxl_cpuid_policy_list = Builtin("cpuid_policy_list", dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE,
+                                  check_default_fn="libxl__cpuid_policy_is_empty")
+
+libxl_string_list = Builtin("string_list", dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE,
+                            check_default_fn="libxl__string_list_is_empty")
+libxl_key_value_list = Builtin("key_value_list", dispose_fn="libxl_key_value_list_dispose", passby=PASS_BY_REFERENCE,
+                               check_default_fn="libxl__key_value_list_is_empty")
+libxl_hwcap = Builtin("hwcap", passby=PASS_BY_REFERENCE,
+                      check_default_fn="libxl__hwcap_is_default")
 
 #
 # Specific integer types
@@ -49,7 +54,7 @@ libxl_domain_type = Enumeration("domain_type", [
     (-1, "INVALID"),
     (1, "HVM"),
     (2, "PV"),
-    ], init_val = -1)
+    ], init_val = "LIBXL_DOMAIN_TYPE_INVALID")
 
 libxl_device_model_version = Enumeration("device_model_version", [
     (0, "UNKNOWN"),
@@ -95,7 +100,7 @@ libxl_action_on_shutdown = Enumeration("action_on_shutdown", [
 
     (5, "COREDUMP_DESTROY"),
     (6, "COREDUMP_RESTART"),
-    ], init_val = 1)
+    ], init_val = "LIBXL_ACTION_ON_SHUTDOWN_DESTROY")
 
 libxl_trigger = Enumeration("trigger", [
     (0, "UNKNOWN"),
@@ -154,7 +159,7 @@ libxl_vga_interface_type = Enumeration("vga_interface_type", [
     (1, "CIRRUS"),
     (2, "STD"),
     (3, "NONE"),
-    ], init_val = 1)
+    ], init_val = "LIBXL_VGA_INTERFACE_TYPE_CIRRUS")
 
 libxl_vendor_device = Enumeration("vendor_device", [
     (0, "NONE"),
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct
  2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
                   ` (3 preceding siblings ...)
  2014-06-17  9:32 ` [PATCH v7 4/9] libxl/gentypes.py: don't generate default values Wei Liu
@ 2014-06-17  9:32 ` Wei Liu
  2014-06-18 14:22   ` Ian Campbell
  2014-06-17  9:32 ` [PATCH v7 6/9] libxl/gentest.py: test JSON parser Wei Liu
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 26+ messages in thread
From: Wei Liu @ 2014-06-17  9:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

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.

The new function definitions are generated to new header files called
__libxl_types_*_json_internal.h so that they don't contaiminate public
header.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Change since last version: output parser function definitions to
dedicated header file.
---
 tools/libxl/Makefile                 |    4 +-
 tools/libxl/gentypes.py              |  158 +++++++++++++++++++++++++++++++++-
 tools/libxl/idl.py                   |   15 ++++
 tools/libxl/idl.txt                  |    7 +-
 tools/libxl/libxl.h                  |   14 +++
 tools/libxl/libxl_internal.h         |    3 +
 tools/libxl/libxl_types.idl          |   27 +++---
 tools/libxl/libxl_types_internal.idl |    4 +-
 8 files changed, 214 insertions(+), 18 deletions(-)

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 4cfa275..bac6287 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -180,9 +180,11 @@ $(LIBXL_OBJS) $(LIBXL_TEST_OBJS) $(LIBXLU_OBJS) \
 $(LIBXL_OBJS) $(LIBXL_TEST_OBJS): libxl_internal.h
 
 _libxl_type%.h _libxl_type%_json.h _libxl_type%.c: libxl_type%.idl gentypes.py idl.py
-	$(PYTHON) gentypes.py libxl_type$*.idl __libxl_type$*.h __libxl_type$*_json.h __libxl_type$*.c
+	$(PYTHON) gentypes.py libxl_type$*.idl __libxl_type$*.h __libxl_type$*_json.h \
+		__libxl_type$*_json_internal.h __libxl_type$*.c
 	$(call move-if-changed,__libxl_type$*.h,_libxl_type$*.h)
 	$(call move-if-changed,__libxl_type$*_json.h,_libxl_type$*_json.h)
+	$(call move-if-changed,__libxl_type$*_json_internal.h,_libxl_type$*_json_internal.h)
 	$(call move-if-changed,__libxl_type$*.c,_libxl_type$*.c)
 
 libxenlight.so: libxenlight.so.$(MAJOR)
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index f06afd9..605e26e 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -312,6 +312,116 @@ 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, discriminator = 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")
+        if discriminator is not None:
+            raise Exception("Only KeyedUnion can have discriminator")
+        lenvar = parent + ty.lenvar.name
+        s += "{\n"
+        s += "    libxl__json_object *t;\n"
+        s += "    int i;\n"
+        s += "    if (!libxl__json_object_is_array(x)) {\n"
+        s += "        rc = -1;\n"
+        s += "        goto out;\n"
+        s += "    }\n"
+        s += "    %s = x->u.array->count;\n" % lenvar
+        s += "    %s = libxl__calloc(NOGC, %s, sizeof(*%s));\n" % (v, lenvar, v)
+        s += "    if (!%s && %s != 0) {\n" % (v, lenvar)
+        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 += "    if (i != %s) {\n" % lenvar
+        s += "        rc = -1;\n"
+        s += "        goto out;\n"
+        s += "    }\n"
+        s += "}\n"
+    elif isinstance(ty, idl.Enumeration):
+        if discriminator is not None:
+            raise Exception("Only KeyedUnion can have discriminator")
+        s += "{\n"
+        s += "    const char *enum_str;\n"
+        s += "    if (!libxl__json_object_is_string(x)) {\n"
+        s += "        rc = -1;\n"
+        s += "        goto out;\n"
+        s += "    }\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")
+        if discriminator is None:
+            raise Excpetion("KeyedUnion type must have a discriminator")
+        for f in ty.fields:
+            if f.enumname != discriminator:
+                continue
+            (nparent,fexpr) = ty.member(v, f, parent is None)
+            if f.type is not None:
+                s += libxl_C_type_parse_json(f.type, w, fexpr, indent + "    ", nparent)
+    elif isinstance(ty, idl.Struct) and (parent is None or ty.json_parse_fn is None):
+        if discriminator is not None:
+            raise Exception("Only KeyedUnion can have discriminator")
+        for f in [f for f in ty.fields if not f.const and not f.type.private]:
+            saved_var_name = "saved_%s" % f.name
+            s += "{\n"
+            s += "    const libxl__json_object *%s = NULL;\n" % saved_var_name
+            s += "    %s = x;\n" % saved_var_name
+            if isinstance(f.type, idl.KeyedUnion):
+                for x in f.type.fields:
+                    s += "    x = libxl__json_map_get(\"%s\", %s, JSON_MAP);\n" % \
+                         (f.type.keyvar.name + "." + x.name, w)
+                    s += "    if (x) {\n"
+                    (nparent, fexpr) = ty.member(v, f.type.keyvar, parent is None)
+                    s += "        %s_init_type(%s, %s);\n" % (ty.typename, v, x.enumname)
+                    (nparent,fexpr) = ty.member(v, f, parent is None)
+                    s += libxl_C_type_parse_json(f.type, "x", fexpr, "  ", nparent, x.enumname)
+                    s += "    }\n"
+            else:
+                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 += "    }\n"
+            s += "    x = %s;\n" % saved_var_name
+            s += "}\n"
+    else:
+        if discriminator is not None:
+            raise Exception("Only KeyedUnion can have discriminator")
+        if ty.json_parse_fn is not None:
+            s += "rc = %s(gc, %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__json_parse_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
@@ -350,11 +460,11 @@ def libxl_C_enum_from_string(ty, str, e, indent = "    "):
 
 
 if __name__ == '__main__':
-    if len(sys.argv) != 5:
-        print >>sys.stderr, "Usage: gentypes.py <idl> <header> <header-json> <implementation>"
+    if len(sys.argv) != 6:
+        print >>sys.stderr, "Usage: gentypes.py <idl> <header> <header-json> <header-json-internal> <implementation>"
         sys.exit(1)
 
-    (_, idlname, header, header_json, impl) = sys.argv
+    (_, idlname, header, header_json, header_json_internal, impl) = sys.argv
 
     (builtins,types) = idl.parse(idlname)
 
@@ -390,6 +500,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)))
@@ -423,6 +535,31 @@ if __name__ == '__main__':
     f.write("""#endif /* %s */\n""" % header_json_define)
     f.close()
 
+    print "outputting libxl JSON internal definitions to %s" % header_json_internal
+
+    f = open(header_json_internal, "w")
+
+    header_json_internal_define = header_json_internal.upper().replace('.','_')
+    f.write("""#ifndef %s
+#define %s
+
+/*
+ * DO NOT EDIT.
+ *
+ * This file is autogenerated by
+ * "%s"
+ */
+
+""" % (header_json_internal_define, header_json_internal_define, " ".join(sys.argv)))
+
+    for ty in [ty for ty in types if ty.json_parse_fn is not None]:
+        f.write("%sint %s_parse_json(libxl__gc *gc, 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()
+
     print "outputting libxl type implementations to %s" % impl
 
     f = open(impl, "w")
@@ -486,4 +623,19 @@ 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__gc *gc, 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")
+        if not isinstance(ty, idl.Enumeration):
+            f.write("    %s_init(p);\n" % ty.typename)
+        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 ada801a..381f11b 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -68,8 +68,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)
 
@@ -121,6 +125,9 @@ 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")
+        # json_parse_fn might be overriden on specific type
+        kwargs.setdefault('json_parse_fn', "libxl__int_parse_json")
         self.signed = kwargs['signed']
         Builtin.__init__(self, ctype, **kwargs)
 
@@ -128,6 +135,7 @@ class UInt(Number):
     def __init__(self, w, **kwargs):
         kwargs.setdefault('namespace', None)
         kwargs.setdefault('dispose_fn', None)
+        kwargs.setdefault('json_parse_fn', "libxl__uint%d_parse_json" % w)
         Number.__init__(self, "uint%d_t" % w, **kwargs)
 
         self.width = w
@@ -144,6 +152,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',
@@ -173,6 +182,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:
@@ -259,6 +269,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)
@@ -272,12 +284,15 @@ uint64 = UInt(64, json_gen_fn = "libxl__uint64_gen_json")
 
 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/idl.txt b/tools/libxl/idl.txt
index d47eba8..87c4952 100644
--- a/tools/libxl/idl.txt
+++ b/tools/libxl/idl.txt
@@ -65,9 +65,14 @@ Type.json_gen_fn: (default: typename + "_gen_json" or None if type == None)
  The name of the C function which will generate a YAJL data structure
  representing this type.
 
+Type.json_parse_fn: (default: typename + "_parse_json" or None if type == None)
+
+ The name of the C function which will parse a libxl JSON structure
+ representing this type to C type.
+
 Type.autogenerate_json: (default: True)
 
- Indicates if the above named Type.json_gen_fn should be autogenerated.
+ Indicates if the above named Type.json_*_fn should be autogenerated.
 
 Type.check_default_fn:
 
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 69ceac8..e52a43f 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -305,6 +305,20 @@
  *
  *    Generates a JSON object from "p" in the form of a NULL terminated
  *    string.
+ *
+ * <type *> libxl_<type>_from_json(const char *json)
+ * int      libxl_<type>_from_json(const char *json)
+ *
+ *    Parses "json" and returns:
+ *
+ *    an int value, if <type> is enumeration type. The value is the enum value
+ *    representing the respective string in "json".
+ *
+ *    an instance of <type>, if <type> is aggregate type. The returned
+ *    instance has its fields filled in by the parser according to "json".
+ *
+ *    If the parsing fails, caller cannot rely on the value / instance
+ *    returned.
  */
 #ifndef LIBXL_H
 #define LIBXL_H
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 2a8f552..37fd77f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3186,6 +3186,9 @@ int libxl__uint64_parse_json(libxl__gc *gc, const libxl__json_object *o,
 int libxl__string_parse_json(libxl__gc *gc, const libxl__json_object *o,
                              char **p);
 
+#include "_libxl_types_json_internal.h"
+#include "_libxl_types_internal_json_internal.h"
+
 /* This always return false, there's no "default value" for hw cap */
 static inline int libxl__hwcap_is_default(libxl_hwcap *hwcap)
 {
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 7234519..2cb856c 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -5,22 +5,25 @@
 
 namespace("libxl_")
 
-libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE, check_default_fn="libxl__defbool_is_default")
-
-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, check_default_fn="libxl_uuid_is_nil")
-libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE, check_default_fn="libxl__mac_is_default")
-libxl_bitmap = Builtin("bitmap", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE,
+libxl_defbool = Builtin("defbool", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE,
+                        check_default_fn="libxl__defbool_is_default")
+
+libxl_domid = Builtin("domid", json_gen_fn = "yajl_gen_integer", json_parse_fn = "libxl__uint32_parse_json",
+                      json_parse_type = "JSON_INTEGER", autogenerate_json = False)
+libxl_devid = Builtin("devid", json_gen_fn = "yajl_gen_integer", json_parse_fn = "libxl__int_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, check_default_fn="libxl_uuid_is_nil")
+libxl_mac = Builtin("mac", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE, check_default_fn="libxl__mac_is_default")
+libxl_bitmap = Builtin("bitmap", json_parse_type="JSON_ARRAY", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE,
                        check_default_fn="libxl_bitmap_is_empty")
 libxl_cpuid_policy_list = Builtin("cpuid_policy_list", dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE,
-                                  check_default_fn="libxl__cpuid_policy_is_empty")
+                                  json_parse_type="JSON_ARRAY", check_default_fn="libxl__cpuid_policy_is_empty")
 
 libxl_string_list = Builtin("string_list", dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE,
-                            check_default_fn="libxl__string_list_is_empty")
+                            json_parse_type="JSON_ARRAY", check_default_fn="libxl__string_list_is_empty")
 libxl_key_value_list = Builtin("key_value_list", dispose_fn="libxl_key_value_list_dispose", passby=PASS_BY_REFERENCE,
-                               check_default_fn="libxl__key_value_list_is_empty")
-libxl_hwcap = Builtin("hwcap", passby=PASS_BY_REFERENCE,
+                               json_parse_type="JSON_MAP", check_default_fn="libxl__key_value_list_is_empty")
+libxl_hwcap = Builtin("hwcap", passby=PASS_BY_REFERENCE, json_parse_type="JSON_ARRAY",
                       check_default_fn="libxl__hwcap_is_default")
 
 #
@@ -586,7 +589,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..17533f1 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__uint32_parse_json", json_parse_type = "JSON_INTEGER",
+		      autogenerate_json = False)
 
 libxl__qmp_message_type = Enumeration("qmp_message_type", [
     (1, "QMP"),
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v7 6/9] libxl/gentest.py: test JSON parser
  2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
                   ` (4 preceding siblings ...)
  2014-06-17  9:32 ` [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct Wei Liu
@ 2014-06-17  9:32 ` Wei Liu
  2014-06-17  9:32 ` [PATCH v7 7/9] libxl: copy function for builtin types Wei Liu
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Wei Liu @ 2014-06-17  9:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

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 identical then we are good.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@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

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v7 7/9] libxl: copy function for builtin types
  2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
                   ` (5 preceding siblings ...)
  2014-06-17  9:32 ` [PATCH v7 6/9] libxl/gentest.py: test JSON parser Wei Liu
@ 2014-06-17  9:32 ` Wei Liu
  2014-06-17  9:32 ` [PATCH v7 8/9] libxl IDL: generate deep copy functions Wei Liu
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 26+ messages in thread
From: Wei Liu @ 2014-06-17  9:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

These functions will be used in later patch to deep-copy a structure.

Functions introduced:
 * libxl_string_list_copy
 * libxl_key_value_list_copy
 * libxl_hwcap_copy
 * libxl_mac_copy
 * libxl_cpuid_policy_list_copy
 * libxl_string_copy
 * libxl_bitmap_copy_alloc

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/libxl.c         |   67 +++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl.h         |   12 ++++++++
 tools/libxl/libxl_cpuid.c   |   33 +++++++++++++++++++++
 tools/libxl/libxl_nocpuid.c |    6 ++++
 tools/libxl/libxl_utils.c   |   25 ++++++++++++++++
 tools/libxl/libxl_utils.h   |    4 +++
 6 files changed, 147 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1b0b08f..e355e20 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -205,6 +205,29 @@ void libxl_string_list_dispose(libxl_string_list *psl)
     free(sl);
 }
 
+void libxl_string_list_copy(libxl_ctx *ctx,
+                            libxl_string_list *dst,
+                            libxl_string_list *src)
+{
+    GC_INIT(ctx);
+    int i, len;
+
+    if (!*src) {
+        *dst = NULL;
+        goto out;
+    }
+
+    len = libxl_string_list_length(src);
+    /* one extra slot for sentinel */
+    *dst = libxl__calloc(NOGC, len + 1, sizeof(char *));
+
+    for (i = 0; i < len; i++)
+        (*dst)[i] = libxl__strdup(NOGC, (*src)[i]);
+
+out:
+    GC_FREE;
+}
+
 int libxl_string_list_length(const libxl_string_list *psl)
 {
     int i = 0;
@@ -245,6 +268,34 @@ void libxl_key_value_list_dispose(libxl_key_value_list *pkvl)
     free(kvl);
 }
 
+void libxl_key_value_list_copy(libxl_ctx *ctx,
+                               libxl_key_value_list *dst,
+                               libxl_key_value_list *src)
+{
+    GC_INIT(ctx);
+    int i, len;
+
+    if (*src == NULL) {
+        *dst = NULL;
+        goto out;
+    }
+
+    len = libxl_key_value_list_length(src);
+    /* one extra slot for sentinel */
+    *dst = libxl__calloc(NOGC, len * 2 + 1, sizeof(char *));
+
+    for (i = 0; i < len * 2; i += 2) {
+        (*dst)[i] = libxl__strdup(NOGC, (*src)[i]);
+        if ((*src)[i+1])
+            (*dst)[i+1] = libxl__strdup(NOGC, (*src)[i+1]);
+        else
+            (*dst)[i+1] = NULL;
+    }
+
+out:
+    GC_FREE;
+}
+
 void libxl_defbool_set(libxl_defbool *db, bool b)
 {
     db->val = b ? LIBXL__DEFBOOL_TRUE : LIBXL__DEFBOOL_FALSE;
@@ -5690,6 +5741,22 @@ int libxl_fd_set_cloexec(libxl_ctx *ctx, int fd, int cloexec)
 int libxl_fd_set_nonblock(libxl_ctx *ctx, int fd, int nonblock)
   { return fd_set_flags(ctx,fd, F_GETFL,F_SETFL,"FL", O_NONBLOCK, nonblock); }
 
+
+void libxl_hwcap_copy(libxl_ctx *ctx,libxl_hwcap *dst, libxl_hwcap *src)
+{
+    int i;
+
+    for (i = 0; i < 8; i++)
+        (*dst)[i] = (*src)[i];
+}
+
+void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src)
+{
+    int i;
+
+    for (i = 0; i < 6; i++)
+        (*dst)[i] = (*src)[i];
+}
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index e52a43f..1fb365d 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -542,20 +542,29 @@ typedef struct libxl__ctx libxl_ctx;
  */
 #define LIBXL_HAVE_CPUPOOL_NAME 1
 
+typedef struct libxl__ctx libxl_ctx;
+
 typedef uint8_t libxl_mac[6];
 #define LIBXL_MAC_FMT "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx"
 #define LIBXL_MAC_FMTLEN ((2*6)+5) /* 6 hex bytes plus 5 colons */
 #define LIBXL_MAC_BYTES(mac) mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]
+void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
 
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
+void libxl_string_list_copy(libxl_ctx *ctx, libxl_string_list *dst,
+                            libxl_string_list *src);
 
 typedef char **libxl_key_value_list;
 void libxl_key_value_list_dispose(libxl_key_value_list *kvl);
 int libxl_key_value_list_length(libxl_key_value_list *kvl);
+void libxl_key_value_list_copy(libxl_ctx *ctx,
+                               libxl_key_value_list *dst,
+                               libxl_key_value_list *src);
 
 typedef uint32_t libxl_hwcap[8];
+void libxl_hwcap_copy(libxl_ctx *ctx, libxl_hwcap *dst, libxl_hwcap *src);
 
 typedef uint64_t libxl_ev_user;
 
@@ -574,6 +583,9 @@ typedef struct libxl__cpuid_policy libxl_cpuid_policy;
 typedef libxl_cpuid_policy * libxl_cpuid_policy_list;
 void libxl_cpuid_dispose(libxl_cpuid_policy_list *cpuid_list);
 int libxl_cpuid_policy_list_length(libxl_cpuid_policy_list *l);
+void libxl_cpuid_policy_list_copy(libxl_ctx *ctx,
+                                  libxl_cpuid_policy_list *dst,
+                                  libxl_cpuid_policy_list *src);
 
 #define LIBXL_PCI_FUNC_ALL (~0U)
 
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 1f2bcd5..f3e67ab 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -468,6 +468,39 @@ int libxl_cpuid_policy_list_length(libxl_cpuid_policy_list *pl)
     return i;
 }
 
+void libxl_cpuid_policy_list_copy(libxl_ctx *ctx,
+                                  libxl_cpuid_policy_list *dst,
+                                  libxl_cpuid_policy_list *src)
+{
+    GC_INIT(ctx);
+    int i, j, len;
+
+    if (*src == NULL) {
+        *dst = NULL;
+        goto out;
+    }
+
+    len = libxl_cpuid_policy_list_length(src);
+    /* one extra slot for sentinel */
+    *dst = libxl__calloc(NOGC, len + 1, sizeof(libxl_cpuid_policy));
+    (*dst)[len].input[0] = XEN_CPUID_INPUT_UNUSED;
+    (*dst)[len].input[1] = XEN_CPUID_INPUT_UNUSED;
+
+    for (i = 0; i < len; i++) {
+        for (j = 0; j < 2; j++)
+            (*dst)[i].input[j] = (*src)[i].input[j];
+        for (j = 0; j < 4; j++)
+            if ((*src)[i].policy[j])
+                (*dst)[i].policy[j] =
+                    libxl__strdup(NOGC, (*src)[i].policy[j]);
+            else
+                (*dst)[i].policy[j] = NULL;
+    }
+
+out:
+    GC_FREE;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_nocpuid.c b/tools/libxl/libxl_nocpuid.c
index eb525fc..698d252 100644
--- a/tools/libxl/libxl_nocpuid.c
+++ b/tools/libxl/libxl_nocpuid.c
@@ -51,6 +51,12 @@ int libxl_cpuid_policy_list_parse_json(libxl__gc *gc,
     return 0;
 }
 
+void libxl_cpuid_policy_list_copy(libxl_ctx *ctx,
+                                  libxl_cpuid_policy_list *dst,
+                                  libxl_cpuid_policy_list *src)
+{
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 16b734e..6053017 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -614,6 +614,19 @@ void libxl_bitmap_copy(libxl_ctx *ctx, libxl_bitmap *dptr,
     memcpy(dptr->map, sptr->map, sz * sizeof(*dptr->map));
 }
 
+void libxl_bitmap_copy_alloc(libxl_ctx *ctx,
+                             libxl_bitmap *dptr,
+                             const libxl_bitmap *sptr)
+{
+    GC_INIT(ctx);
+
+    dptr->map = libxl__calloc(NOGC, sptr->size, sizeof(*sptr->map));
+    dptr->size = sptr->size;
+    memcpy(dptr->map, sptr->map, sptr->size * sizeof(*sptr->map));
+
+    GC_FREE;
+}
+
 int libxl_bitmap_is_full(const libxl_bitmap *bitmap)
 {
     int i;
@@ -1013,6 +1026,18 @@ int libxl_domid_valid_guest(uint32_t domid)
     return domid > 0 && domid < DOMID_FIRST_RESERVED;
 }
 
+void libxl_string_copy(libxl_ctx *ctx, char **dst, char **src)
+{
+    GC_INIT(ctx);
+
+    if (*src)
+        *dst = libxl__strdup(NOGC, *src);
+    else
+        *dst = NULL;
+
+    GC_FREE;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 8bfb81b..cc528d2 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -76,6 +76,8 @@ int libxl_devid_to_device_vtpm(libxl_ctx *ctx, uint32_t domid,
 int libxl_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *bitmap, int n_bits);
     /* Allocated bimap is from malloc, libxl_bitmap_dispose() to be
      * called by the application when done. */
+void libxl_bitmap_copy_alloc(libxl_ctx *ctx, libxl_bitmap *dptr,
+                             const libxl_bitmap *sptr);
 void libxl_bitmap_copy(libxl_ctx *ctx, libxl_bitmap *dptr,
                        const libxl_bitmap *sptr);
 int libxl_bitmap_is_full(const libxl_bitmap *bitmap);
@@ -121,6 +123,8 @@ int libxl_cpumap_to_nodemap(libxl_ctx *ctx,
     return (s + 1023) / 1024;
 }
 
+void libxl_string_copy(libxl_ctx *ctx, char **dst, char **src);
+
 #endif
 
 /*
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v7 8/9] libxl IDL: generate deep copy functions
  2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
                   ` (6 preceding siblings ...)
  2014-06-17  9:32 ` [PATCH v7 7/9] libxl: copy function for builtin types Wei Liu
@ 2014-06-17  9:32 ` Wei Liu
  2014-06-17  9:32 ` [PATCH v7 9/9] libxl/gentest.py: test " Wei Liu
  2014-06-18 16:02 ` [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Ian Campbell
  9 siblings, 0 replies; 26+ messages in thread
From: Wei Liu @ 2014-06-17  9:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

Introduces copy_fn for a type.

For most builtin types we can use direct assignment. For those builtin
types which cannot use direct assignment we use the copy functions in
previous patch.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/gentypes.py              |   67 ++++++++++++++++++++++++++++++++++
 tools/libxl/idl.py                   |   13 ++++++-
 tools/libxl/idl.txt                  |   12 ++++++
 tools/libxl/libxl_types.idl          |   27 ++++++++------
 tools/libxl/libxl_types_internal.idl |    2 +-
 5 files changed, 108 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 605e26e..3690cf3 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -100,6 +100,61 @@ def libxl_C_type_dispose(ty, v, indent = "    ", parent = None):
         s = indent + s
     return s.replace("\n", "\n%s" % indent).rstrip(indent)
 
+def libxl_C_type_copy(ty, v, w, indent = "    ", vparent = None, wparent = None):
+    s = ""
+
+    if vparent is None:
+        s += "GC_INIT(ctx);\n";
+
+    if isinstance(ty, idl.KeyedUnion):
+        if vparent is None or wparent is None:
+            raise Exception("KeyedUnion type must have a parent")
+        s += "%s = %s;\n" % ((vparent + ty.keyvar.name), (wparent + ty.keyvar.name))
+        s += "switch (%s) {\n" % (wparent + ty.keyvar.name)
+        for f in ty.fields:
+            (vnparent,vfexpr) = ty.member(v, f, vparent is None)
+            (wnparent,wfexpr) = ty.member(w, f, wparent is None)
+            s += "case %s:\n" % f.enumname
+            if f.type is not None:
+                s += libxl_C_type_copy(f.type, vfexpr, wfexpr, indent + "    ",
+                                       vnparent, wnparent)
+            s += "    break;\n"
+        s += "}\n"
+    elif isinstance(ty, idl.Array):
+        if vparent is None or wparent is None:
+            raise Exception("Array type must have a parent")
+        s += "%s = libxl__calloc(NOGC, %s, sizeof(*%s));\n" % (ty.pass_arg(v, vparent is None),
+                                                               (wparent + ty.lenvar.name),
+                                                               ty.pass_arg(w, wparent is None))
+        s += "%s = %s;\n" % ((vparent + ty.lenvar.name), (wparent + ty.lenvar.name))
+        s += "{\n"
+        s += "    int i;\n"
+        s += "    for (i=0; i<%s; i++)\n" % (wparent + ty.lenvar.name)
+        s += libxl_C_type_copy(ty.elem_type, v+"[i]", w+"[i]",
+                               indent + "        ", vparent, wparent)
+        s += "}\n"
+    elif isinstance(ty, idl.Struct) and ((vparent is None and wparent is None) or ty.copy_fn is None):
+        for f in [f for f in ty.fields if not f.const and not f.type.private]:
+            (vnparent,vfexpr) = ty.member(v, f, vparent is None)
+            (wnparent,wfexpr) = ty.member(w, f, wparent is None)
+            s += libxl_C_type_copy(f.type, vfexpr, wfexpr, "", vnparent, wnparent)
+    else:
+        if ty.copy_fn is not None:
+            s += "%s(ctx, %s, %s);\n" % (ty.copy_fn,
+                                         ty.pass_arg(v, vparent is None, passby=idl.PASS_BY_REFERENCE),
+                                         ty.pass_arg(w, wparent is None, passby=idl.PASS_BY_REFERENCE))
+
+        else:
+            s += "%s = %s;\n" % (ty.pass_arg(v, vparent is None, passby=idl.PASS_BY_VALUE),
+                                 ty.pass_arg(w, wparent is None, passby=idl.PASS_BY_VALUE))
+
+    if vparent is None:
+        s += "GC_FREE;\n"
+
+    if s != "":
+        s = indent + s
+    return s.replace("\n", "\n%s" % indent).rstrip(indent)
+
 def libxl_init_members(ty, nesting = 0):
     """Returns a list of members of ty which require a separate init"""
 
@@ -489,6 +544,9 @@ if __name__ == '__main__':
         f.write(libxl_C_type_define(ty) + ";\n")
         if ty.dispose_fn is not None:
             f.write("%svoid %s(%s);\n" % (ty.hidden(), ty.dispose_fn, ty.make_arg("p")))
+        if ty.copy_fn is not None:
+            f.write("%svoid %s(libxl_ctx *ctx, %s, %s);\n" % (ty.hidden(), ty.copy_fn,
+                                              ty.make_arg("dst"), ty.make_arg("src")))
         if ty.init_fn is not None:
             f.write("%svoid %s(%s);\n" % (ty.hidden(), ty.init_fn, ty.make_arg("p")))
             for field in libxl_init_members(ty):
@@ -589,6 +647,15 @@ if __name__ == '__main__':
         f.write("    memset(p, LIBXL_DTOR_POISON, sizeof(*p));\n")
         f.write("}\n")
         f.write("\n")
+
+    for ty in [t for t in types if t.copy_fn and t.autogenerate_copy_fn]:
+        f.write("void %s(libxl_ctx *ctx, %s, %s)\n" % (ty.copy_fn,
+                                       ty.make_arg("dst", passby=idl.PASS_BY_REFERENCE),
+                                       ty.make_arg("src", passby=idl.PASS_BY_REFERENCE)))
+        f.write("{\n")
+        f.write(libxl_C_type_copy(ty, "dst", "src"))
+        f.write("}\n")
+        f.write("\n")
         
     for ty in [t for t in types if t.init_fn is not None and t.autogenerate_init_fn]:
         f.write(libxl_C_type_init(ty))
diff --git a/tools/libxl/idl.py b/tools/libxl/idl.py
index 381f11b..56f9e6c 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -60,6 +60,13 @@ class Type(object):
 
         self.autogenerate_dispose_fn = kwargs.setdefault('autogenerate_dispose_fn', True)
 
+        if self.typename is not None:
+            self.copy_fn = kwargs.setdefault('copy_fn', self.typename + "_copy")
+        else:
+            self.copy_fn = kwargs.setdefault('copy_fn', None)
+
+        self.autogenerate_copy_fn = kwargs.setdefault('autogenerate_copy_fn', True)
+
         self.init_fn = kwargs.setdefault('init_fn', None)
         self.init_val = kwargs.setdefault('init_val', None)
         self.autogenerate_init_fn = kwargs.setdefault('autogenerate_init_fn', False)
@@ -123,6 +130,7 @@ class Number(Builtin):
     def __init__(self, ctype, **kwargs):
         kwargs.setdefault('namespace', None)
         kwargs.setdefault('dispose_fn', None)
+        kwargs.setdefault('copy_fn', None)
         kwargs.setdefault('signed', False)
         kwargs.setdefault('json_gen_fn', "yajl_gen_integer")
         kwargs.setdefault('json_parse_type', "JSON_INTEGER")
@@ -136,6 +144,7 @@ class UInt(Number):
         kwargs.setdefault('namespace', None)
         kwargs.setdefault('dispose_fn', None)
         kwargs.setdefault('json_parse_fn', "libxl__uint%d_parse_json" % w)
+        kwargs.setdefault('copy_fn', None)
         Number.__init__(self, "uint%d_t" % w, **kwargs)
 
         self.width = w
@@ -152,6 +161,7 @@ class EnumerationValue(object):
 class Enumeration(Type):
     def __init__(self, typename, values, **kwargs):
         kwargs.setdefault('dispose_fn', None)
+        kwargs.setdefault('copy_fn', None)
         kwargs.setdefault('json_parse_type', "JSON_STRING")
         Type.__init__(self, typename, **kwargs)
 
@@ -268,6 +278,7 @@ class KeyedUnion(Aggregate):
 
 void = Builtin("void *", namespace = None)
 bool = Builtin("bool", namespace = None,
+               copy_fn=None,
                json_gen_fn = "yajl_gen_bool",
                json_parse_type = "JSON_BOOL",
                json_parse_fn = "libxl__bool_parse_json",
@@ -282,7 +293,7 @@ uint16 = UInt(16)
 uint32 = UInt(32)
 uint64 = UInt(64, json_gen_fn = "libxl__uint64_gen_json")
 
-string = Builtin("char *", namespace = None, dispose_fn = "free",
+string = Builtin("char *", namespace = None, copy_fn = "libxl_string_copy", dispose_fn = "free",
                  json_gen_fn = "libxl__string_gen_json",
                  json_parse_type = "JSON_STRING | JSON_NULL",
                  json_parse_fn = "libxl__string_parse_json",
diff --git a/tools/libxl/idl.txt b/tools/libxl/idl.txt
index 87c4952..7440fb3 100644
--- a/tools/libxl/idl.txt
+++ b/tools/libxl/idl.txt
@@ -44,6 +44,18 @@ Type.autogenerate_dispose_fn: (default: True)
  Indicates if the above named Type.dispose_fn should be
  autogenerated.
 
+Type.copy_fn: (default: typename + "_copy" or None if type == None)
+
+ The name of the C function which will deep copy all fields within
+ this type.
+
+Type.autogenerate_copy_fn: (default: True)
+
+ Indicates if the above named Type.copy_fn should be
+ autogenerated.
+
+Type.autogenerate_copy_fn
+
 Type.init_val: (default: None)
 
  C expression for the value to initialise instances of this type to.
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 2cb856c..d40b5c7 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -5,26 +5,31 @@
 
 namespace("libxl_")
 
-libxl_defbool = Builtin("defbool", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE,
+libxl_defbool = Builtin("defbool", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE, copy_fn=None,
                         check_default_fn="libxl__defbool_is_default")
-
 libxl_domid = Builtin("domid", json_gen_fn = "yajl_gen_integer", json_parse_fn = "libxl__uint32_parse_json",
-                      json_parse_type = "JSON_INTEGER", autogenerate_json = False)
+                      json_parse_type = "JSON_INTEGER", autogenerate_json = False, copy_fn=None)
 libxl_devid = Builtin("devid", json_gen_fn = "yajl_gen_integer", json_parse_fn = "libxl__int_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, check_default_fn="libxl_uuid_is_nil")
-libxl_mac = Builtin("mac", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE, check_default_fn="libxl__mac_is_default")
+                      json_parse_type = "JSON_INTEGER", autogenerate_json = False, signed = True, init_val="-1",
+                      copy_fn=None)
+libxl_uuid = Builtin("uuid", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE, check_default_fn="libxl_uuid_is_nil",
+                     copy_fn="libxl_uuid_copy")
+libxl_mac = Builtin("mac", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE, check_default_fn="libxl__mac_is_default",
+                    copy_fn="libxl_mac_copy")
 libxl_bitmap = Builtin("bitmap", json_parse_type="JSON_ARRAY", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE,
-                       check_default_fn="libxl_bitmap_is_empty")
+                       check_default_fn="libxl_bitmap_is_empty", copy_fn="libxl_bitmap_copy_alloc")
 libxl_cpuid_policy_list = Builtin("cpuid_policy_list", dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE,
-                                  json_parse_type="JSON_ARRAY", check_default_fn="libxl__cpuid_policy_is_empty")
+                                  json_parse_type="JSON_ARRAY", check_default_fn="libxl__cpuid_policy_is_empty",
+                                  copy_fn="libxl_cpuid_policy_list_copy")
 
 libxl_string_list = Builtin("string_list", dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE,
-                            json_parse_type="JSON_ARRAY", check_default_fn="libxl__string_list_is_empty")
+                            json_parse_type="JSON_ARRAY", check_default_fn="libxl__string_list_is_empty",
+                            copy_fn="libxl_string_list_copy")
 libxl_key_value_list = Builtin("key_value_list", dispose_fn="libxl_key_value_list_dispose", passby=PASS_BY_REFERENCE,
-                               json_parse_type="JSON_MAP", check_default_fn="libxl__key_value_list_is_empty")
+                               json_parse_type="JSON_MAP", check_default_fn="libxl__key_value_list_is_empty",
+                               copy_fn="libxl_key_value_list_copy")
 libxl_hwcap = Builtin("hwcap", passby=PASS_BY_REFERENCE, json_parse_type="JSON_ARRAY",
-                      check_default_fn="libxl__hwcap_is_default")
+                      check_default_fn="libxl__hwcap_is_default", copy_fn="libxl_hwcap_copy")
 
 #
 # Specific integer types
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index 17533f1..800361b 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -3,7 +3,7 @@ hidden(True)
 
 libxl_domid = Builtin("domid", namespace="libxl_", json_gen_fn = "yajl_gen_integer",
 		      json_parse_fn = "libxl__uint32_parse_json", json_parse_type = "JSON_INTEGER",
-		      autogenerate_json = False)
+		      autogenerate_json = False, copy_fn = None)
 
 libxl__qmp_message_type = Enumeration("qmp_message_type", [
     (1, "QMP"),
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v7 9/9] libxl/gentest.py: test deep copy functions
  2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
                   ` (7 preceding siblings ...)
  2014-06-17  9:32 ` [PATCH v7 8/9] libxl IDL: generate deep copy functions Wei Liu
@ 2014-06-17  9:32 ` Wei Liu
  2014-06-18 16:02 ` [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Ian Campbell
  9 siblings, 0 replies; 26+ messages in thread
From: Wei Liu @ 2014-06-17  9:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell

The test is done as followed:
1. initialise libxl_FOO struct A
2. deep-copy A to B
3. generate JSON string for A and B
4. compare two JSON strings

If two strings are identical then we're good.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 tools/libxl/gentest.py |   37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
index b92d092..07ffd31 100644
--- a/tools/libxl/gentest.py
+++ b/tools/libxl/gentest.py
@@ -273,6 +273,43 @@ int main(int argc, char **argv)
             f.write("    %s(&%s_val_new);\n" % (ty.dispose_fn, ty.typename))
         f.write("\n")
 
+    f.write("    printf(\"Testing TYPE_copy()\\n\");\n")
+    f.write("    printf(\"----------------------\\n\");\n")
+    f.write("    printf(\"\\n\");\n")
+    for ty in [t for t in types if t.copy_fn is not None]:
+        f.write("    printf(\"Testing %s_copy, \");\n" % ty.typename)
+        arg = ty.typename + "_val"
+        f.write("    %s_init(%s);\n" % (ty.typename, \
+            ty.pass_arg(arg, isref=False, passby=idl.PASS_BY_REFERENCE)))
+        f.write("    %s_rand_init(%s);\n" % (ty.typename, \
+            ty.pass_arg(arg, isref=False, passby=idl.PASS_BY_REFERENCE)))
+        f.write("    %s_init(%s_new);\n" % (ty.typename, \
+            ty.pass_arg(arg, isref=False, passby=idl.PASS_BY_REFERENCE)))
+        f.write("    %s_copy(ctx, %s_new, %s);\n" % (ty.typename, \
+            ty.pass_arg(arg, isref=False, passby=idl.PASS_BY_REFERENCE), \
+            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("    if (s == NULL) 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? Deep copy for %s failed. Regenerated string different from original string.\\n\");\n" \
+                % ty.typename)
+        f.write("        printf(\"Original string: %s\\n\", s);\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("    printf(\"done\\n\");\n")
+        f.write("\n")
+
+    f.write("    printf(\"\\n\");\n")
     f.write("    printf(\"Testing Enumerations\\n\");\n")
     f.write("    printf(\"--------------------\\n\");\n")
     f.write("    printf(\"\\n\");\n")
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 1/9] xl / libxl: push parsing of SSID and CPU pool ID down to libxl
  2014-06-17  9:32 ` [PATCH v7 1/9] xl / libxl: push parsing of SSID and CPU pool ID down to libxl Wei Liu
@ 2014-06-17 22:46   ` Daniel De Graaf
  2014-06-18  8:11     ` Wei Liu
  0 siblings, 1 reply; 26+ messages in thread
From: Daniel De Graaf @ 2014-06-17 22:46 UTC (permalink / raw)
  To: Wei Liu, xen-devel
  Cc: Juergen Gross, Dario Faggioli, ian.jackson, ian.campbell

On 06/17/2014 05:32 AM, Wei Liu wrote:
> This patch pushes parsing of "init_seclabel", "seclabel",
> "device_model_stubdomain_seclabel" and "pool" down to libxl level.
>
> Originally the parsing is done in xl level, which is not ideal because
> libxl won't have the truely relevant information. With this patch libxl
> holds important information by itself.
>
> The libxl IDL is extended to hold the string of labels and pool name.
> And if there those strings are present they take precedence over the
> numeric representations.
>
> As all relevant structures (libxl_dominfo etc) have a field called
> X_name / X_label now, a string is also copied there so that callers
> won't have to do ID to name / label translation.
>
> In order to be compatible with users of older versions of libxl, this
> patch also defines LIBXL_HAVE_SSID_LABEL and LIBXL_HAVE_CPUPOOL_NAME. If
> they are defined, the respective strings are available. And if those
> strings are not NULL, libxl will do the parsing and ignore the numeric
> values.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

This change looks good to me (untested), although I don't believe I am
listed as a maintainer for any of it.

> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>   tools/libxl/libxl.c         |   22 +++++++--
>   tools/libxl/libxl.h         |   20 ++++++++
>   tools/libxl/libxl_create.c  |   57 +++++++++++++++++++++++
>   tools/libxl/libxl_dm.c      |    4 ++
>   tools/libxl/libxl_types.idl |    6 +++
>   tools/libxl/xl_cmdimpl.c    |  107 ++++++++++++-------------------------------
>   tools/libxl/xl_sxp.c        |    7 +--
>   7 files changed, 137 insertions(+), 86 deletions(-)
>


-- 
Daniel De Graaf
National Security Agency

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 1/9] xl / libxl: push parsing of SSID and CPU pool ID down to libxl
  2014-06-17 22:46   ` Daniel De Graaf
@ 2014-06-18  8:11     ` Wei Liu
  0 siblings, 0 replies; 26+ messages in thread
From: Wei Liu @ 2014-06-18  8:11 UTC (permalink / raw)
  To: Daniel De Graaf
  Cc: Juergen Gross, Wei Liu, ian.campbell, Dario Faggioli,
	ian.jackson, xen-devel

On Tue, Jun 17, 2014 at 06:46:04PM -0400, Daniel De Graaf wrote:
> On 06/17/2014 05:32 AM, Wei Liu wrote:
> >This patch pushes parsing of "init_seclabel", "seclabel",
> >"device_model_stubdomain_seclabel" and "pool" down to libxl level.
> >
> >Originally the parsing is done in xl level, which is not ideal because
> >libxl won't have the truely relevant information. With this patch libxl
> >holds important information by itself.
> >
> >The libxl IDL is extended to hold the string of labels and pool name.
> >And if there those strings are present they take precedence over the
> >numeric representations.
> >
> >As all relevant structures (libxl_dominfo etc) have a field called
> >X_name / X_label now, a string is also copied there so that callers
> >won't have to do ID to name / label translation.
> >
> >In order to be compatible with users of older versions of libxl, this
> >patch also defines LIBXL_HAVE_SSID_LABEL and LIBXL_HAVE_CPUPOOL_NAME. If
> >they are defined, the respective strings are available. And if those
> >strings are not NULL, libxl will do the parsing and ignore the numeric
> >values.
> >
> >Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >Cc: Dario Faggioli <dario.faggioli@citrix.com>
> >Cc: Juergen Gross <jgross@suse.com>
> >Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> 
> This change looks good to me (untested), although I don't believe I am
> listed as a maintainer for any of it.
> 

I'm not at all familiar with these features so I CC'ed experts. Thanks
for confirming.

Wei.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 3/9] libxl_internal: functions to check default values for builtin types
  2014-06-17  9:32 ` [PATCH v7 3/9] libxl_internal: functions to check default values for builtin types Wei Liu
@ 2014-06-18 14:07   ` Ian Campbell
  2014-06-18 14:10     ` Wei Liu
  2014-06-18 14:33     ` Wei Liu
  0 siblings, 2 replies; 26+ messages in thread
From: Ian Campbell @ 2014-06-18 14:07 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.jackson, xen-devel

On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> +static inline int libxl__defbool_is_default(libxl_defbool *db)

This already exists, but with a libxl_ prefix only.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 3/9] libxl_internal: functions to check default values for builtin types
  2014-06-18 14:07   ` Ian Campbell
@ 2014-06-18 14:10     ` Wei Liu
  2014-06-18 14:33       ` Ian Campbell
  2014-06-18 14:33     ` Wei Liu
  1 sibling, 1 reply; 26+ messages in thread
From: Wei Liu @ 2014-06-18 14:10 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel

On Wed, Jun 18, 2014 at 03:07:25PM +0100, Ian Campbell wrote:
> On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> > +static inline int libxl__defbool_is_default(libxl_defbool *db)
> 
> This already exists, but with a libxl_ prefix only.
> 

Ah, somehow I missed that one.

Any comments on other functions?

Wei.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 4/9] libxl/gentypes.py: don't generate default values
  2014-06-17  9:32 ` [PATCH v7 4/9] libxl/gentypes.py: don't generate default values Wei Liu
@ 2014-06-18 14:13   ` Ian Campbell
  2014-06-18 15:11     ` Wei Liu
  0 siblings, 1 reply; 26+ messages in thread
From: Ian Campbell @ 2014-06-18 14:13 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.jackson, xen-devel

On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> If a field:
>     0. is not of aggregate type
> and
>     1. is of array type and the array is not empty
> or  2. is of a type which has init_val and has been set to init_val,
> or  3. is of builtin type and has been set to internal default value,
> or  4. has been set to 0

This isn't quite true, since #4 depends on their being no init_val.
Prepend "is of a type which has no init_val and "?

> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct
  2014-06-17  9:32 ` [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct Wei Liu
@ 2014-06-18 14:22   ` Ian Campbell
  2014-06-18 14:45     ` Wei Liu
  2014-06-18 17:21     ` Wei Liu
  0 siblings, 2 replies; 26+ messages in thread
From: Ian Campbell @ 2014-06-18 14:22 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.jackson, xen-devel

On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> 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.
> 
> The new function definitions are generated to new header files called
> __libxl_types_*_json_internal.h so that they don't contaiminate public
> header.

"contaminate".

I'm a bit confused what is going into this file. It seems to be these
two:

    f.write("int %s_parse_json(libxl__gc *gc, const libxl__json_object *%s, %s)\n" % (ty.typename,"o",ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
    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)))


The first looks internal (it takes const libxl__json_object which is
internal) but it is in the libxl_ namespace.

The second looks like it should be public in _libxl_json.h, shouldn't
it?

I think you need to put the first function into a new
_libxl_types_*_internal.h (no need to make it specific to internal json
functions), and the second into just regular _libxl_types.h (it doesn't
require the application to opt into using yajl so it doesn't have to go
into _libxl_types_json.h)

Ian.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 3/9] libxl_internal: functions to check default values for builtin types
  2014-06-18 14:10     ` Wei Liu
@ 2014-06-18 14:33       ` Ian Campbell
  0 siblings, 0 replies; 26+ messages in thread
From: Ian Campbell @ 2014-06-18 14:33 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.jackson, xen-devel

On Wed, 2014-06-18 at 15:10 +0100, Wei Liu wrote:
> On Wed, Jun 18, 2014 at 03:07:25PM +0100, Ian Campbell wrote:
> > On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> > > +static inline int libxl__defbool_is_default(libxl_defbool *db)
> > 
> > This already exists, but with a libxl_ prefix only.
> > 
> 
> Ah, somehow I missed that one.
> 
> Any comments on other functions?

I didn't explicitly check for similar external functions, but the ones
you've got look ok if you've done that check already.

Ian.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 3/9] libxl_internal: functions to check default values for builtin types
  2014-06-18 14:07   ` Ian Campbell
  2014-06-18 14:10     ` Wei Liu
@ 2014-06-18 14:33     ` Wei Liu
  2014-06-18 14:48       ` Ian Campbell
  1 sibling, 1 reply; 26+ messages in thread
From: Wei Liu @ 2014-06-18 14:33 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel

On Wed, Jun 18, 2014 at 03:07:25PM +0100, Ian Campbell wrote:
> On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> > +static inline int libxl__defbool_is_default(libxl_defbool *db)
> 
> This already exists, but with a libxl_ prefix only.
> 

Now I remeber why I introduced a new function - libx_defbool_is_default
doesn't take a pointer and I didn't want to special-case defbool in
python generator.

What do you think?

Wei.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct
  2014-06-18 14:22   ` Ian Campbell
@ 2014-06-18 14:45     ` Wei Liu
  2014-06-18 14:51       ` Ian Campbell
  2014-06-18 17:21     ` Wei Liu
  1 sibling, 1 reply; 26+ messages in thread
From: Wei Liu @ 2014-06-18 14:45 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel

On Wed, Jun 18, 2014 at 03:22:13PM +0100, Ian Campbell wrote:
> On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> > 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.
> > 
> > The new function definitions are generated to new header files called
> > __libxl_types_*_json_internal.h so that they don't contaiminate public
> > header.
> 
> "contaminate".
> 

Fixed. Thanks.

> I'm a bit confused what is going into this file. It seems to be these
> two:
> 
>     f.write("int %s_parse_json(libxl__gc *gc, const libxl__json_object *%s, %s)\n" % (ty.typename,"o",ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
>     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)))
> 
> 
> The first looks internal (it takes const libxl__json_object which is
> internal) but it is in the libxl_ namespace.
> 

The first one should be internal. I should use libxl__ namespace in my
next version. This is loose end I missed.

> The second looks like it should be public in _libxl_json.h, shouldn't
> it?
> 

Yes, you're right.

> I think you need to put the first function into a new
> _libxl_types_*_internal.h (no need to make it specific to internal json
> functions),

This one sounds good.

> and the second into just regular _libxl_types.h (it doesn't
> require the application to opt into using yajl so it doesn't have to go
> into _libxl_types_json.h)
> 

I don't think it can work without YAJL.

Wei.

> Ian.
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 3/9] libxl_internal: functions to check default values for builtin types
  2014-06-18 14:33     ` Wei Liu
@ 2014-06-18 14:48       ` Ian Campbell
  0 siblings, 0 replies; 26+ messages in thread
From: Ian Campbell @ 2014-06-18 14:48 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.jackson, xen-devel

On Wed, 2014-06-18 at 15:33 +0100, Wei Liu wrote:
> On Wed, Jun 18, 2014 at 03:07:25PM +0100, Ian Campbell wrote:
> > On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> > > +static inline int libxl__defbool_is_default(libxl_defbool *db)
> > 
> > This already exists, but with a libxl_ prefix only.
> > 
> 
> Now I remeber why I introduced a new function - libx_defbool_is_default
> doesn't take a pointer and I didn't want to special-case defbool in
> python generator.
> 
> What do you think?

OK, I guess.

Ian.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct
  2014-06-18 14:45     ` Wei Liu
@ 2014-06-18 14:51       ` Ian Campbell
  2014-06-18 15:12         ` Wei Liu
  0 siblings, 1 reply; 26+ messages in thread
From: Ian Campbell @ 2014-06-18 14:51 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.jackson, xen-devel

On Wed, 2014-06-18 at 15:45 +0100, Wei Liu wrote:
> > and the second into just regular _libxl_types.h (it doesn't
> > require the application to opt into using yajl so it doesn't have to go
> > into _libxl_types_json.h)
> > 
> 
> I don't think it can work without YAJL.

It takes a const char * and returns a libxl device. The *application*
has no need to see any yajl, only libxl does, which is the distinction
here.

IOW from_json can be used by an application without ever #including
<yajl.h>

Contrast the libxl_foo_gen_json functions which take a "yajl_gen hand"
and therefore require that the application be using yajl itself

Ian.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 4/9] libxl/gentypes.py: don't generate default values
  2014-06-18 14:13   ` Ian Campbell
@ 2014-06-18 15:11     ` Wei Liu
  0 siblings, 0 replies; 26+ messages in thread
From: Wei Liu @ 2014-06-18 15:11 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel

On Wed, Jun 18, 2014 at 03:13:13PM +0100, Ian Campbell wrote:
> On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> > If a field:
> >     0. is not of aggregate type
> > and
> >     1. is of array type and the array is not empty
> > or  2. is of a type which has init_val and has been set to init_val,
> > or  3. is of builtin type and has been set to internal default value,
> > or  4. has been set to 0
> 
> This isn't quite true, since #4 depends on their being no init_val.
> Prepend "is of a type which has no init_val and "?
> 

You're right. I will prepend that sentence to #4.

Wei.

> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Other than that: Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct
  2014-06-18 14:51       ` Ian Campbell
@ 2014-06-18 15:12         ` Wei Liu
  0 siblings, 0 replies; 26+ messages in thread
From: Wei Liu @ 2014-06-18 15:12 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel

On Wed, Jun 18, 2014 at 03:51:19PM +0100, Ian Campbell wrote:
> On Wed, 2014-06-18 at 15:45 +0100, Wei Liu wrote:
> > > and the second into just regular _libxl_types.h (it doesn't
> > > require the application to opt into using yajl so it doesn't have to go
> > > into _libxl_types_json.h)
> > > 
> > 
> > I don't think it can work without YAJL.
> 
> It takes a const char * and returns a libxl device. The *application*
> has no need to see any yajl, only libxl does, which is the distinction
> here.
> 
> IOW from_json can be used by an application without ever #including
> <yajl.h>
> 
> Contrast the libxl_foo_gen_json functions which take a "yajl_gen hand"
> and therefore require that the application be using yajl itself
> 

I see. I misunderstood. Thanks for clarification.

Wei.

> Ian.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API
  2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
                   ` (8 preceding siblings ...)
  2014-06-17  9:32 ` [PATCH v7 9/9] libxl/gentest.py: test " Wei Liu
@ 2014-06-18 16:02 ` Ian Campbell
  9 siblings, 0 replies; 26+ messages in thread
From: Ian Campbell @ 2014-06-18 16:02 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.jackson, xen-devel

On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> Wei Liu (9):
>  A  xl / libxl: push parsing of SSID and CPU pool ID down to libxl
>  A  libxl: libxl_uuid_copy now takes a ctx argument

Applied.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct
  2014-06-18 14:22   ` Ian Campbell
  2014-06-18 14:45     ` Wei Liu
@ 2014-06-18 17:21     ` Wei Liu
  2014-06-24  9:10       ` Ian Campbell
  1 sibling, 1 reply; 26+ messages in thread
From: Wei Liu @ 2014-06-18 17:21 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel

On Wed, Jun 18, 2014 at 03:22:13PM +0100, Ian Campbell wrote:
> On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> > 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.
> > 
> > The new function definitions are generated to new header files called
> > __libxl_types_*_json_internal.h so that they don't contaiminate public
> > header.
> 
> "contaminate".
> 
> I'm a bit confused what is going into this file. It seems to be these
> two:
> 
>     f.write("int %s_parse_json(libxl__gc *gc, const libxl__json_object *%s, %s)\n" % (ty.typename,"o",ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
>     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)))
> 

This hunk is in fact generating the implementation. I don't see the need
to separate them to different implementation files.

> The first looks internal (it takes const libxl__json_object which is
> internal) but it is in the libxl_ namespace.
> 
> The second looks like it should be public in _libxl_json.h, shouldn't
> it?
> 

I agree with you that the public functions should to go _libxl_types.h
-- it's done like that in this version already. So actually nothing to
worry about with regard to header files.

Wei.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct
  2014-06-18 17:21     ` Wei Liu
@ 2014-06-24  9:10       ` Ian Campbell
  0 siblings, 0 replies; 26+ messages in thread
From: Ian Campbell @ 2014-06-24  9:10 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.jackson, xen-devel

On Wed, 2014-06-18 at 18:21 +0100, Wei Liu wrote:
> On Wed, Jun 18, 2014 at 03:22:13PM +0100, Ian Campbell wrote:
> > On Tue, 2014-06-17 at 10:32 +0100, Wei Liu wrote:
> > > 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.
> > > 
> > > The new function definitions are generated to new header files called
> > > __libxl_types_*_json_internal.h so that they don't contaiminate public
> > > header.
> > 
> > "contaminate".
> > 
> > I'm a bit confused what is going into this file. It seems to be these
> > two:
> > 
> >     f.write("int %s_parse_json(libxl__gc *gc, const libxl__json_object *%s, %s)\n" % (ty.typename,"o",ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
> >     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)))
> > 
> 
> This hunk is in fact generating the implementation.

Sorry, somehow I thought this was the prototype.

>  I don't see the need
> to separate them to different implementation files.

Indeed, there is no reason to do so. My confusion.

> > The first looks internal (it takes const libxl__json_object which is
> > internal) but it is in the libxl_ namespace.
> > 
> > The second looks like it should be public in _libxl_json.h, shouldn't
> > it?
> > 
> 
> I agree with you that the public functions should to go _libxl_types.h
> -- it's done like that in this version already. So actually nothing to
> worry about with regard to header files.

Smashing. I'll have another look at this patch with that in mind once
I've dealt with my backlog from being away.

Ian.

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2014-06-24  9:10 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-17  9:32 [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
2014-06-17  9:32 ` [PATCH v7 1/9] xl / libxl: push parsing of SSID and CPU pool ID down to libxl Wei Liu
2014-06-17 22:46   ` Daniel De Graaf
2014-06-18  8:11     ` Wei Liu
2014-06-17  9:32 ` [PATCH v7 2/9] libxl: libxl_uuid_copy now takes a ctx argument Wei Liu
2014-06-17  9:32 ` [PATCH v7 3/9] libxl_internal: functions to check default values for builtin types Wei Liu
2014-06-18 14:07   ` Ian Campbell
2014-06-18 14:10     ` Wei Liu
2014-06-18 14:33       ` Ian Campbell
2014-06-18 14:33     ` Wei Liu
2014-06-18 14:48       ` Ian Campbell
2014-06-17  9:32 ` [PATCH v7 4/9] libxl/gentypes.py: don't generate default values Wei Liu
2014-06-18 14:13   ` Ian Campbell
2014-06-18 15:11     ` Wei Liu
2014-06-17  9:32 ` [PATCH v7 5/9] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct Wei Liu
2014-06-18 14:22   ` Ian Campbell
2014-06-18 14:45     ` Wei Liu
2014-06-18 14:51       ` Ian Campbell
2014-06-18 15:12         ` Wei Liu
2014-06-18 17:21     ` Wei Liu
2014-06-24  9:10       ` Ian Campbell
2014-06-17  9:32 ` [PATCH v7 6/9] libxl/gentest.py: test JSON parser Wei Liu
2014-06-17  9:32 ` [PATCH v7 7/9] libxl: copy function for builtin types Wei Liu
2014-06-17  9:32 ` [PATCH v7 8/9] libxl IDL: generate deep copy functions Wei Liu
2014-06-17  9:32 ` [PATCH v7 9/9] libxl/gentest.py: test " Wei Liu
2014-06-18 16:02 ` [PATCH v7 0/9] libxl: JSON infrastructure, fixes and prerequisite patches for new API Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.