* [PATCH v6 01/18] libxl: make cpupool_qualifier_to_cpupoolid a library function
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 02/18] xl / libxl: push parsing of SSID and CPU pool ID down to libxl Wei Liu
` (17 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl.h | 9 +++++++++
tools/libxl/libxl_utils.c | 31 ++++++++++++++++++++++++++++
tools/libxl/libxl_utils.h | 3 +++
tools/libxl/xl_cmdimpl.c | 49 +++++++++------------------------------------
4 files changed, 52 insertions(+), 40 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 80947c3..a57e2fe 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -67,6 +67,15 @@
* the same $(XEN_VERSION) (e.g. throughout a major release).
*/
+/* LIBXL_HAVE_CPUPOOL_QUALIFIER_TO_CPUPOOLID
+ *
+ * If this is defined, libxl has a library function called
+ * libxl_cpupool_qualifier_to_cpupoolid, which takes in a CPU pool
+ * qualifier in the form of number or string, then returns the ID of
+ * that CPU pool.
+ */
+#define LIBXL_HAVE_CPUPOOL_QUALIFIER_TO_CPUPOOLID 1
+
/*
* LIBXL_HAVE_FIRMWARE_PASSTHROUGH indicates the feature for
* passing in SMBIOS and ACPI firmware to HVM guests is present
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 1f334f2..476921e 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -108,6 +108,37 @@ int libxl_domain_qualifier_to_domid(libxl_ctx *ctx, const char *name,
return rv;
}
+static int qualifier_to_id(const char *p, uint32_t *id_r)
+{
+ int i, alldigit;
+
+ alldigit = 1;
+ for (i = 0; p[i]; i++) {
+ if (!isdigit((uint8_t)p[i])) {
+ alldigit = 0;
+ break;
+ }
+ }
+
+ if (i > 0 && alldigit) {
+ *id_r = strtoul(p, NULL, 10);
+ return 0;
+ } else {
+ /* check here if it's a uuid and do proper conversion */
+ }
+ return 1;
+}
+
+int libxl_cpupool_qualifier_to_cpupoolid(libxl_ctx *ctx, const char *p,
+ uint32_t *poolid_r,
+ int *was_name_r)
+{
+ int was_name;
+
+ was_name = qualifier_to_id(p, poolid_r);
+ if (was_name_r) *was_name_r = was_name;
+ return was_name ? libxl_name_to_cpupoolid(ctx, p, poolid_r) : 0;
+}
char *libxl_cpupoolid_to_name(libxl_ctx *ctx, uint32_t poolid)
{
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index e37fb89..8bfb81b 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -23,6 +23,9 @@ unsigned long libxl_get_required_shadow_memory(unsigned long maxmem_kb, unsigned
int libxl_name_to_domid(libxl_ctx *ctx, const char *name, uint32_t *domid);
int libxl_domain_qualifier_to_domid(libxl_ctx *ctx, const char *name, uint32_t *domid);
char *libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid);
+int libxl_cpupool_qualifier_to_cpupoolid(libxl_ctx *ctx, const char *p,
+ uint32_t *poolid_r,
+ int *was_name_r);
int libxl_name_to_cpupoolid(libxl_ctx *ctx, const char *name, uint32_t *poolid);
char *libxl_cpupoolid_to_name(libxl_ctx *ctx, uint32_t poolid);
int libxl_cpupoolid_is_valid(libxl_ctx *ctx, uint32_t poolid);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5195914..ac3188e 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -158,38 +158,6 @@ struct domain_create {
};
-
-static int qualifier_to_id(const char *p, uint32_t *id_r)
-{
- int i, alldigit;
-
- alldigit = 1;
- for (i = 0; p[i]; i++) {
- if (!isdigit((uint8_t)p[i])) {
- alldigit = 0;
- break;
- }
- }
-
- if (i > 0 && alldigit) {
- *id_r = strtoul(p, NULL, 10);
- return 0;
- } else {
- /* check here if it's a uuid and do proper conversion */
- }
- return 1;
-}
-
-static int cpupool_qualifier_to_cpupoolid(const char *p, uint32_t *poolid_r,
- int *was_name_r)
-{
- int was_name;
-
- was_name = qualifier_to_id(p, poolid_r);
- if (was_name_r) *was_name_r = was_name;
- return was_name ? libxl_name_to_cpupoolid(ctx, p, poolid_r) : 0;
-}
-
static uint32_t find_domain(const char *p) __attribute__((warn_unused_result));
static uint32_t find_domain(const char *p)
{
@@ -817,7 +785,7 @@ static void parse_config_data(const char *config_source,
if (!xlu_cfg_get_string (config, "pool", &buf, 0)) {
c_info->poolid = -1;
- cpupool_qualifier_to_cpupoolid(buf, &c_info->poolid, NULL);
+ 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");
@@ -5223,7 +5191,7 @@ static int sched_domain_output(libxl_scheduler sched, int (*output)(int),
int rc = 0;
if (cpupool) {
- if (cpupool_qualifier_to_cpupoolid(cpupool, &poolid, NULL) ||
+ if (libxl_cpupool_qualifier_to_cpupoolid(ctx, cpupool, &poolid, NULL) ||
!libxl_cpupoolid_is_valid(ctx, poolid)) {
fprintf(stderr, "unknown cpupool \'%s\'\n", cpupool);
return -ERROR_FAIL;
@@ -5343,7 +5311,8 @@ int main_sched_credit(int argc, char **argv)
uint32_t poolid = 0;
if (cpupool) {
- if (cpupool_qualifier_to_cpupoolid(cpupool, &poolid, NULL) ||
+ if (libxl_cpupool_qualifier_to_cpupoolid(ctx, cpupool,
+ &poolid, NULL) ||
!libxl_cpupoolid_is_valid(ctx, poolid)) {
fprintf(stderr, "unknown cpupool \'%s\'\n", cpupool);
return -ERROR_FAIL;
@@ -6852,7 +6821,7 @@ int main_cpupooldestroy(int argc, char **argv)
pool = argv[optind];
- if (cpupool_qualifier_to_cpupoolid(pool, &poolid, NULL) ||
+ if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, &poolid, NULL) ||
!libxl_cpupoolid_is_valid(ctx, poolid)) {
fprintf(stderr, "unknown cpupool \'%s\'\n", pool);
return -ERROR_FAIL;
@@ -6874,7 +6843,7 @@ int main_cpupoolrename(int argc, char **argv)
pool = argv[optind++];
- if (cpupool_qualifier_to_cpupoolid(pool, &poolid, NULL) ||
+ if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, &poolid, NULL) ||
!libxl_cpupoolid_is_valid(ctx, poolid)) {
fprintf(stderr, "unknown cpupool \'%s\'\n", pool);
return -ERROR_FAIL;
@@ -6912,7 +6881,7 @@ int main_cpupoolcpuadd(int argc, char **argv)
cpu = atoi(argv[optind]);
}
- if (cpupool_qualifier_to_cpupoolid(pool, &poolid, NULL) ||
+ if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, &poolid, NULL) ||
!libxl_cpupoolid_is_valid(ctx, poolid)) {
fprintf(stderr, "unknown cpupool \'%s\'\n", pool);
return -ERROR_FAIL;
@@ -6957,7 +6926,7 @@ int main_cpupoolcpuremove(int argc, char **argv)
cpu = atoi(argv[optind]);
}
- if (cpupool_qualifier_to_cpupoolid(pool, &poolid, NULL) ||
+ if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, &poolid, NULL) ||
!libxl_cpupoolid_is_valid(ctx, poolid)) {
fprintf(stderr, "unknown cpupool \'%s\'\n", pool);
return -ERROR_FAIL;
@@ -7001,7 +6970,7 @@ int main_cpupoolmigrate(int argc, char **argv)
return -ERROR_FAIL;
}
- if (cpupool_qualifier_to_cpupoolid(pool, &poolid, NULL) ||
+ if (libxl_cpupool_qualifier_to_cpupoolid(ctx, pool, &poolid, NULL) ||
!libxl_cpupoolid_is_valid(ctx, poolid)) {
fprintf(stderr, "unknown cpupool \'%s\'\n", pool);
return -ERROR_FAIL;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v6 02/18] xl / libxl: push parsing of SSID and CPU pool ID down to libxl
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
2014-06-09 12:43 ` [PATCH v6 01/18] libxl: make cpupool_qualifier_to_cpupoolid a library function Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-10 12:57 ` Ian Campbell
2014-06-09 12:43 ` [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning " Wei Liu
` (16 subsequent siblings)
18 siblings, 1 reply; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel
Cc: Wei Liu, ian.campbell, Dario Faggioli, ian.jackson,
Daniel De Graaf, Juergen Gross
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 have a field called X_name / X_label now, a
string is also copied there so that we can use it directly. 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 <juergen.gross@ts.fujitsu.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
tools/libxl/libxl.c | 19 ++++++--
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, 134 insertions(+), 86 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 900b8d4..5f320ca 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -516,12 +516,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);
@@ -568,7 +574,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;
@@ -587,7 +593,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;
}
@@ -615,6 +621,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);
@@ -4159,7 +4170,7 @@ retry_transaction:
abort_transaction = 1;
goto out;
}
- xcinfo2xlinfo(&info, &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);
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index a57e2fe..8b790f2 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 52f1aa9..0ea0464 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),
@@ -309,6 +313,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),
@@ -319,6 +324,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 ac3188e..a1cb5b8 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -727,35 +727,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);
@@ -783,14 +765,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)
@@ -1582,20 +1558,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); \
@@ -3307,15 +3273,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);
@@ -6780,27 +6739,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] 48+ messages in thread* Re: [PATCH v6 02/18] xl / libxl: push parsing of SSID and CPU pool ID down to libxl
2014-06-09 12:43 ` [PATCH v6 02/18] xl / libxl: push parsing of SSID and CPU pool ID down to libxl Wei Liu
@ 2014-06-10 12:57 ` Ian Campbell
2014-06-10 14:16 ` Wei Liu
0 siblings, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2014-06-10 12:57 UTC (permalink / raw)
To: Wei Liu
Cc: Juergen Gross, Dario Faggioli, ian.jackson, Daniel De Graaf, xen-devel
On Mon, 2014-06-09 at 13:43 +0100, 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 have a field called X_name / X_label now, a
> string is also copied there so that we can use it directly.
I'm not sure what you mean by this. Do you mean that if the caller uses
the numeric and not the string version we will populate the string side?
> 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 <juergen.gross@ts.fujitsu.com>
> Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
(I switched to Juergen's new address in the CC)
> ---
> tools/libxl/libxl.c | 19 ++++++--
> 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, 134 insertions(+), 86 deletions(-)
>
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 900b8d4..5f320ca 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -516,12 +516,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;
libxl_set_memory_target uses xcinfo2xlinfo but incorrectly fails to call
dispose on the result, so it will leak any ssid_label.
The other callers of this function all seem to return the result to the
application, which can be expected to be correct here.
Other than that:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 02/18] xl / libxl: push parsing of SSID and CPU pool ID down to libxl
2014-06-10 12:57 ` Ian Campbell
@ 2014-06-10 14:16 ` Wei Liu
0 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-10 14:16 UTC (permalink / raw)
To: Ian Campbell
Cc: Juergen Gross, Wei Liu, Dario Faggioli, ian.jackson, xen-devel,
Daniel De Graaf
On Tue, Jun 10, 2014 at 01:57:55PM +0100, Ian Campbell wrote:
> On Mon, 2014-06-09 at 13:43 +0100, 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 have a field called X_name / X_label now, a
> > string is also copied there so that we can use it directly.
>
> I'm not sure what you mean by this. Do you mean that if the caller uses
> the numeric and not the string version we will populate the string side?
>
I mean I added string field in libxl_dominfo etc, so that caller won't
have to do the translation.
I shall make this clearer in commit message.
> > 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 <juergen.gross@ts.fujitsu.com>
> > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>
> (I switched to Juergen's new address in the CC)
>
> > ---
> > tools/libxl/libxl.c | 19 ++++++--
> > 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, 134 insertions(+), 86 deletions(-)
> >
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index 900b8d4..5f320ca 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -516,12 +516,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;
>
> libxl_set_memory_target uses xcinfo2xlinfo but incorrectly fails to call
> dispose on the result, so it will leak any ssid_label.
>
I will fix this.
Wei.
> The other callers of this function all seem to return the result to the
> application, which can be expected to be correct here.
>
> Other than that:
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> Ian.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
2014-06-09 12:43 ` [PATCH v6 01/18] libxl: make cpupool_qualifier_to_cpupoolid a library function Wei Liu
2014-06-09 12:43 ` [PATCH v6 02/18] xl / libxl: push parsing of SSID and CPU pool ID down to libxl Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:53 ` Wei Liu
` (2 more replies)
2014-06-09 12:43 ` [PATCH v6 04/18] libxl: libxl_uuid_copy now takes a ctx argument Wei Liu
` (15 subsequent siblings)
18 siblings, 3 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Dario Faggioli, ian.jackson, ian.campbell
This patch introduces an array of libxl_bitmap called "vcpu_affinity" in
libxl IDL to preserve VCPU to PCPU mapping. This is necessary for libxl
to preserve all information to construct a domain.
Also define LIBXL_HAVE_AFFINITY_LIST in libxl.h to mark the change in
API.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
---
tools/libxl/libxl.h | 14 ++++++++++
tools/libxl/libxl_dom.c | 14 ++++++++++
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl_cmdimpl.c | 64 ++++++++++++-------------------------------
4 files changed, 46 insertions(+), 47 deletions(-)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 8b790f2..20aa875 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -331,6 +331,20 @@
#endif
/*
+ * LIBXL_HAVE_VCPU_AFFINITY_LIST
+ *
+ * If this is defined, then libxl_domain_build_info structure will
+ * contain vcpu_affinity, an array of libxl_bitmap that contains
+ * the necessary information to pin a VCPU to a PCPU. Libxl will try
+ * to pin VCPUs to PCPUs according to this list.
+ *
+ * The number of libxl_bitmap in the array equals to the maximum number
+ * of VCPUs. The size of each bitmap equals to the maximum number of
+ * PCPUs.
+ */
+#define LIBXL_HAVE_VCPU_AFFINITY_KEY_VALUE_LIST 1
+
+/*
* LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
*
* If this is defined, then the libxl_domain_build_info structure will
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 661999c..1312c44 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -263,6 +263,20 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
libxl_domain_set_nodeaffinity(ctx, domid, &info->nodemap);
libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, &info->cpumap);
+ /* If we have vcpu affinity list, pin vcpu to pcpu. */
+ if (d_config->b_info.num_vcpumaps) {
+ int i;
+
+ for (i = 0; i < d_config->b_info.num_vcpumaps; i++) {
+ if (libxl_set_vcpuaffinity(ctx, domid, i,
+ &d_config->b_info.vcpu_affinity[i])) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "setting affinity failed on vcpu `%d'.", i);
+ return ERROR_FAIL;
+ }
+ }
+ }
+
if (xc_domain_setmaxmem(ctx->xch, domid, info->target_memkb +
LIBXL_MAXMEM_CONSTANT) < 0) {
LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Couldn't set max memory");
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 0ea0464..4765fb6 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -305,6 +305,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
("avail_vcpus", libxl_bitmap),
("cpumap", libxl_bitmap),
("nodemap", libxl_bitmap),
+ ("vcpu_affinity", Array(libxl_bitmap, "num_vcpumaps")),
("numa_placement", libxl_defbool),
("tsc_mode", libxl_tsc_mode),
("max_memkb", MemKB),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index a1cb5b8..6901bac 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -88,9 +88,6 @@ xlchild children[child_max];
static const char *common_domname;
static int fd_lock = -1;
-/* Stash for specific vcpu to pcpu mappping */
-static int *vcpu_to_pcpu;
-
static const char savefileheader_magic[32]=
"Xen saved domain, xl format\n \0 \r";
@@ -804,27 +801,25 @@ static void parse_config_data(const char *config_source,
if (!xlu_cfg_get_list (config, "cpus", &cpus, 0, 1)) {
int n_cpus = 0;
+ b_info->num_vcpumaps = b_info->max_vcpus;
if (libxl_cpu_bitmap_alloc(ctx, &b_info->cpumap, 0)) {
fprintf(stderr, "Unable to allocate cpumap\n");
exit(1);
}
- /* Prepare the array for single vcpu to pcpu mappings */
- vcpu_to_pcpu = xmalloc(sizeof(int) * b_info->max_vcpus);
- memset(vcpu_to_pcpu, -1, sizeof(int) * b_info->max_vcpus);
+ b_info->vcpu_affinity =
+ xmalloc(b_info->num_vcpumaps * sizeof(libxl_bitmap));
+
+ for (i = 0; i < b_info->num_vcpumaps; i++) {
+ libxl_bitmap_init(&b_info->vcpu_affinity[i]);
+ if (libxl_cpu_bitmap_alloc(ctx, &b_info->vcpu_affinity[i], 0)) {
+ fprintf(stderr, "Unable to allocate cpumap for vcpu %d\n", i);
+ exit(1);
+ }
+ libxl_bitmap_set_any(&b_info->vcpu_affinity[i]);
+ }
- /*
- * Idea here is to let libxl think all the domain's vcpus
- * have cpu affinity with all the pcpus on the list.
- * It is then us, here in xl, that matches each single vcpu
- * to its pcpu (and that's why we need to stash such info in
- * the vcpu_to_pcpu array now) after the domain has been created.
- * Doing it like this saves the burden of passing to libxl
- * some big array hosting the single mappings. Also, using
- * the cpumap derived from the list ensures memory is being
- * allocated on the proper nodes anyway.
- */
libxl_bitmap_set_none(&b_info->cpumap);
while ((buf = xlu_cfg_get_listitem(cpus, n_cpus)) != NULL) {
i = atoi(buf);
@@ -833,12 +828,14 @@ static void parse_config_data(const char *config_source,
exit(1);
}
libxl_bitmap_set(&b_info->cpumap, i);
- if (n_cpus < b_info->max_vcpus)
- vcpu_to_pcpu[n_cpus] = i;
+ if (n_cpus < b_info->max_vcpus) {
+ libxl_bitmap_set_none(&b_info->vcpu_affinity[n_cpus]);
+ libxl_bitmap_set(&b_info->vcpu_affinity[n_cpus], i);
+ }
n_cpus++;
}
- /* We have a cpumap, disable automatic placement */
+ /* We have a list of cpumaps, disable automatic placement */
libxl_defbool_set(&b_info->numa_placement, false);
}
else if (!xlu_cfg_get_string (config, "cpus", &buf, 0)) {
@@ -2188,33 +2185,6 @@ start:
if ( ret )
goto error_out;
- /* If single vcpu to pcpu mapping was requested, honour it */
- if (vcpu_to_pcpu) {
- libxl_bitmap vcpu_cpumap;
-
- ret = libxl_cpu_bitmap_alloc(ctx, &vcpu_cpumap, 0);
- if (ret)
- goto error_out;
- for (i = 0; i < d_config.b_info.max_vcpus; i++) {
-
- if (vcpu_to_pcpu[i] != -1) {
- libxl_bitmap_set_none(&vcpu_cpumap);
- libxl_bitmap_set(&vcpu_cpumap, vcpu_to_pcpu[i]);
- } else {
- libxl_bitmap_set_any(&vcpu_cpumap);
- }
- if (libxl_set_vcpuaffinity(ctx, domid, i, &vcpu_cpumap)) {
- fprintf(stderr, "setting affinity failed on vcpu `%d'.\n", i);
- libxl_bitmap_dispose(&vcpu_cpumap);
- free(vcpu_to_pcpu);
- ret = ERROR_FAIL;
- goto error_out;
- }
- }
- libxl_bitmap_dispose(&vcpu_cpumap);
- free(vcpu_to_pcpu); vcpu_to_pcpu = NULL;
- }
-
ret = libxl_userdata_store(ctx, domid, "xl",
config_data, config_len);
if (ret) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl
2014-06-09 12:43 ` [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning " Wei Liu
@ 2014-06-09 12:53 ` Wei Liu
2014-06-10 6:59 ` Dario Faggioli
2014-06-10 14:01 ` Ian Campbell
2 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:53 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Dario Faggioli, ian.jackson, ian.campbell
In case people are wondering, the main change in this patch is that now
we use bitmap in IDL instead of key value list.
Wei.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl
2014-06-09 12:43 ` [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning " Wei Liu
2014-06-09 12:53 ` Wei Liu
@ 2014-06-10 6:59 ` Dario Faggioli
2014-06-10 8:09 ` Wei Liu
2014-06-10 14:01 ` Ian Campbell
2 siblings, 1 reply; 48+ messages in thread
From: Dario Faggioli @ 2014-06-10 6:59 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.jackson, ian.campbell, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 1535 bytes --]
On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> This patch introduces an array of libxl_bitmap called "vcpu_affinity" in
> libxl IDL to preserve VCPU to PCPU mapping. This is necessary for libxl
> to preserve all information to construct a domain.
>
> Also define LIBXL_HAVE_AFFINITY_LIST in libxl.h to mark the change in
> API.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
>
So, as far as this patch is concerned:
Acked-by: Dario Faggioli <dario.faggioli@citrix.com>
This clashes (not that badly, but it does) with my soft affinity series,
that I just resent yesterday night. It's not a big deal, we just need
two arrays of libxl_bitmap-s instead of just one: one for hard affinity,
and one for soft affinity. Probably that also means that
b_info->vcpu_affinity should have to be renamed to something like
b_info->vcpu_hard_affinity (or b_info->vcpu_hard_aff, or
b_info->hard_affinity, if we want to try keep it a bit shorter).
In any case, if this series goes in first, I'll rebase mine on top of
this one and respin it. It mine goes in first, Wei, I'm up for modifying
this patch of yours as described above, if you want/need it of course.
Just let me know.
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl
2014-06-10 6:59 ` Dario Faggioli
@ 2014-06-10 8:09 ` Wei Liu
2014-06-10 13:01 ` Ian Campbell
0 siblings, 1 reply; 48+ messages in thread
From: Wei Liu @ 2014-06-10 8:09 UTC (permalink / raw)
To: Dario Faggioli; +Cc: ian.jackson, Wei Liu, ian.campbell, xen-devel
On Tue, Jun 10, 2014 at 08:59:06AM +0200, Dario Faggioli wrote:
> On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > This patch introduces an array of libxl_bitmap called "vcpu_affinity" in
> > libxl IDL to preserve VCPU to PCPU mapping. This is necessary for libxl
> > to preserve all information to construct a domain.
> >
> > Also define LIBXL_HAVE_AFFINITY_LIST in libxl.h to mark the change in
> > API.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > Cc: Dario Faggioli <dario.faggioli@citrix.com>
> >
> So, as far as this patch is concerned:
>
> Acked-by: Dario Faggioli <dario.faggioli@citrix.com>
>
> This clashes (not that badly, but it does) with my soft affinity series,
> that I just resent yesterday night. It's not a big deal, we just need
> two arrays of libxl_bitmap-s instead of just one: one for hard affinity,
> and one for soft affinity. Probably that also means that
> b_info->vcpu_affinity should have to be renamed to something like
> b_info->vcpu_hard_affinity (or b_info->vcpu_hard_aff, or
> b_info->hard_affinity, if we want to try keep it a bit shorter).
>
I would go for vcpu_hard_affinity. When it comes to interface I prefer
it to be as explicit as possible.
> In any case, if this series goes in first, I'll rebase mine on top of
> this one and respin it. It mine goes in first, Wei, I'm up for modifying
> this patch of yours as described above, if you want/need it of course.
>
Thanks. I think that will do.
Wei.
> Just let me know.
>
> Regards,
> Dario
>
> --
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl
2014-06-10 8:09 ` Wei Liu
@ 2014-06-10 13:01 ` Ian Campbell
2014-06-10 13:09 ` Dario Faggioli
0 siblings, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2014-06-10 13:01 UTC (permalink / raw)
To: Wei Liu; +Cc: Dario Faggioli, ian.jackson, xen-devel
On Tue, 2014-06-10 at 09:09 +0100, Wei Liu wrote:
> On Tue, Jun 10, 2014 at 08:59:06AM +0200, Dario Faggioli wrote:
> > On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > > This patch introduces an array of libxl_bitmap called "vcpu_affinity" in
> > > libxl IDL to preserve VCPU to PCPU mapping. This is necessary for libxl
> > > to preserve all information to construct a domain.
> > >
> > > Also define LIBXL_HAVE_AFFINITY_LIST in libxl.h to mark the change in
> > > API.
> > >
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > Cc: Dario Faggioli <dario.faggioli@citrix.com>
> > >
> > So, as far as this patch is concerned:
> >
> > Acked-by: Dario Faggioli <dario.faggioli@citrix.com>
> >
> > This clashes (not that badly, but it does) with my soft affinity series,
> > that I just resent yesterday night. It's not a big deal, we just need
> > two arrays of libxl_bitmap-s instead of just one: one for hard affinity,
> > and one for soft affinity. Probably that also means that
> > b_info->vcpu_affinity should have to be renamed to something like
> > b_info->vcpu_hard_affinity (or b_info->vcpu_hard_aff, or
> > b_info->hard_affinity, if we want to try keep it a bit shorter).
> >
>
> I would go for vcpu_hard_affinity. When it comes to interface I prefer
> it to be as explicit as possible.
Renaming fields in the API is problematic, so we should make this be
correct when we first check it in, whichever order that is in.
Once that is resolved:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl
2014-06-10 13:01 ` Ian Campbell
@ 2014-06-10 13:09 ` Dario Faggioli
2014-06-10 13:46 ` Wei Liu
0 siblings, 1 reply; 48+ messages in thread
From: Dario Faggioli @ 2014-06-10 13:09 UTC (permalink / raw)
To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 2690 bytes --]
On mar, 2014-06-10 at 14:01 +0100, Ian Campbell wrote:
> On Tue, 2014-06-10 at 09:09 +0100, Wei Liu wrote:
> > On Tue, Jun 10, 2014 at 08:59:06AM +0200, Dario Faggioli wrote:
> > > On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > > > This patch introduces an array of libxl_bitmap called "vcpu_affinity" in
> > > > libxl IDL to preserve VCPU to PCPU mapping. This is necessary for libxl
> > > > to preserve all information to construct a domain.
> > > >
> > > > Also define LIBXL_HAVE_AFFINITY_LIST in libxl.h to mark the change in
> > > > API.
> > > >
> > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > > Cc: Dario Faggioli <dario.faggioli@citrix.com>
> > > >
> > > So, as far as this patch is concerned:
> > >
> > > Acked-by: Dario Faggioli <dario.faggioli@citrix.com>
> > >
> > > This clashes (not that badly, but it does) with my soft affinity series,
> > > that I just resent yesterday night. It's not a big deal, we just need
> > > two arrays of libxl_bitmap-s instead of just one: one for hard affinity,
> > > and one for soft affinity. Probably that also means that
> > > b_info->vcpu_affinity should have to be renamed to something like
> > > b_info->vcpu_hard_affinity (or b_info->vcpu_hard_aff, or
> > > b_info->hard_affinity, if we want to try keep it a bit shorter).
> > >
> >
> > I would go for vcpu_hard_affinity. When it comes to interface I prefer
> > it to be as explicit as possible.
>
> Renaming fields in the API is problematic, so we should make this be
> correct when we first check it in, whichever order that is in.
>
Good point.
Well, I may be wrong, but it looks to me that the more logical order
would be for Wei's series to go in first, and me to rebase mine on top
of that. If we do it the other way around, Wei's would have to call the
field vcpu_hard_affinity, without the distinction between hard and soft
affinity being present and explained anywhere in the tree.
Not a big deal, probably, if my series follows quickly enough, but
cetainly confusing for future archaeologists, I guess. :-)
This assuming both the series to be pretty, and almost equally, close to
green light.
In an case, it would still be useful for me to have the current version
(v7) of the soft affinity series reviewed, so that I'll know whether
such rebasing is the only thing I should change in v8. :-)
Let me know what you prefer.
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl
2014-06-10 13:09 ` Dario Faggioli
@ 2014-06-10 13:46 ` Wei Liu
0 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-10 13:46 UTC (permalink / raw)
To: Dario Faggioli; +Cc: ian.jackson, Wei Liu, Ian Campbell, xen-devel
On Tue, Jun 10, 2014 at 03:09:51PM +0200, Dario Faggioli wrote:
> On mar, 2014-06-10 at 14:01 +0100, Ian Campbell wrote:
> > On Tue, 2014-06-10 at 09:09 +0100, Wei Liu wrote:
> > > On Tue, Jun 10, 2014 at 08:59:06AM +0200, Dario Faggioli wrote:
> > > > On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > > > > This patch introduces an array of libxl_bitmap called "vcpu_affinity" in
> > > > > libxl IDL to preserve VCPU to PCPU mapping. This is necessary for libxl
> > > > > to preserve all information to construct a domain.
> > > > >
> > > > > Also define LIBXL_HAVE_AFFINITY_LIST in libxl.h to mark the change in
> > > > > API.
> > > > >
> > > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > > > Cc: Dario Faggioli <dario.faggioli@citrix.com>
> > > > >
> > > > So, as far as this patch is concerned:
> > > >
> > > > Acked-by: Dario Faggioli <dario.faggioli@citrix.com>
> > > >
> > > > This clashes (not that badly, but it does) with my soft affinity series,
> > > > that I just resent yesterday night. It's not a big deal, we just need
> > > > two arrays of libxl_bitmap-s instead of just one: one for hard affinity,
> > > > and one for soft affinity. Probably that also means that
> > > > b_info->vcpu_affinity should have to be renamed to something like
> > > > b_info->vcpu_hard_affinity (or b_info->vcpu_hard_aff, or
> > > > b_info->hard_affinity, if we want to try keep it a bit shorter).
> > > >
> > >
> > > I would go for vcpu_hard_affinity. When it comes to interface I prefer
> > > it to be as explicit as possible.
> >
> > Renaming fields in the API is problematic, so we should make this be
> > correct when we first check it in, whichever order that is in.
> >
> Good point.
>
> Well, I may be wrong, but it looks to me that the more logical order
> would be for Wei's series to go in first, and me to rebase mine on top
> of that. If we do it the other way around, Wei's would have to call the
> field vcpu_hard_affinity, without the distinction between hard and soft
> affinity being present and explained anywhere in the tree.
>
I don't think this is a big problem. I will change the name to
vcpu_hard_affinity and note this in commit message.
Wei.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl
2014-06-09 12:43 ` [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning " Wei Liu
2014-06-09 12:53 ` Wei Liu
2014-06-10 6:59 ` Dario Faggioli
@ 2014-06-10 14:01 ` Ian Campbell
2014-06-10 14:06 ` Wei Liu
2 siblings, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2014-06-10 14:01 UTC (permalink / raw)
To: Wei Liu; +Cc: Dario Faggioli, ian.jackson, xen-devel
On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 0ea0464..4765fb6 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -305,6 +305,7 @@ libxl_domain_build_info =
> Struct("domain_build_info",[
> ("avail_vcpus", libxl_bitmap),
> ("cpumap", libxl_bitmap),
> ("nodemap", libxl_bitmap),
> + ("vcpu_affinity", Array(libxl_bitmap, "num_vcpumaps")),
Looking at one of Dario's patches I became confused about how this new
field relates to the existing cpumap field.
Am I right that the new field is just a per-vcpu version of the old
(which only allows you to set the affinity of every vcpu together)?
Can this relationship be mentioned in the commit message and/or comments
please.
I also wonder if the name could be changed to better reflect this
relationship, but I can't actually think of a good name. cpumap_FOO
where FOO has connotations of per-vcpu/listiness.
I also just noticed that this change violates the following from
idl.txt:
idl.Array.len_var contains an idl.Field which is added to the parent
idl.Aggregate and will contain the length of the array. The field
MUST be named num_ARRAYNAME.
Ian.
> ("numa_placement", libxl_defbool),
> ("tsc_mode", libxl_tsc_mode),
> ("max_memkb", MemKB),
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl
2014-06-10 14:01 ` Ian Campbell
@ 2014-06-10 14:06 ` Wei Liu
2014-06-10 15:59 ` Dario Faggioli
0 siblings, 1 reply; 48+ messages in thread
From: Wei Liu @ 2014-06-10 14:06 UTC (permalink / raw)
To: Ian Campbell; +Cc: ian.jackson, Dario Faggioli, Wei Liu, xen-devel
On Tue, Jun 10, 2014 at 03:01:00PM +0100, Ian Campbell wrote:
> On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index 0ea0464..4765fb6 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -305,6 +305,7 @@ libxl_domain_build_info =
> > Struct("domain_build_info",[
> > ("avail_vcpus", libxl_bitmap),
> > ("cpumap", libxl_bitmap),
> > ("nodemap", libxl_bitmap),
> > + ("vcpu_affinity", Array(libxl_bitmap, "num_vcpumaps")),
>
> Looking at one of Dario's patches I became confused about how this new
> field relates to the existing cpumap field.
>
> Am I right that the new field is just a per-vcpu version of the old
> (which only allows you to set the affinity of every vcpu together)?
>
Yes, you're right. The old field denotes the PCPUs this domain is
allowed to run on. The new array specifies the PCPUs each VCPU can run
on.
> Can this relationship be mentioned in the commit message and/or comments
> please.
>
Sure.
> I also wonder if the name could be changed to better reflect this
> relationship, but I can't actually think of a good name. cpumap_FOO
> where FOO has connotations of per-vcpu/listiness.
>
> I also just noticed that this change violates the following from
> idl.txt:
> idl.Array.len_var contains an idl.Field which is added to the parent
> idl.Aggregate and will contain the length of the array. The field
> MUST be named num_ARRAYNAME.
>
I will change lenvar to num_ARRAYNAME.
Wei.
> Ian.
>
> > ("numa_placement", libxl_defbool),
> > ("tsc_mode", libxl_tsc_mode),
> > ("max_memkb", MemKB),
>
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning down to libxl
2014-06-10 14:06 ` Wei Liu
@ 2014-06-10 15:59 ` Dario Faggioli
0 siblings, 0 replies; 48+ messages in thread
From: Dario Faggioli @ 2014-06-10 15:59 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.jackson, Ian Campbell, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 3150 bytes --]
On mar, 2014-06-10 at 15:06 +0100, Wei Liu wrote:
> On Tue, Jun 10, 2014 at 03:01:00PM +0100, Ian Campbell wrote:
> > On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > > index 0ea0464..4765fb6 100644
> > > --- a/tools/libxl/libxl_types.idl
> > > +++ b/tools/libxl/libxl_types.idl
> > > @@ -305,6 +305,7 @@ libxl_domain_build_info =
> > > Struct("domain_build_info",[
> > > ("avail_vcpus", libxl_bitmap),
> > > ("cpumap", libxl_bitmap),
> > > ("nodemap", libxl_bitmap),
> > > + ("vcpu_affinity", Array(libxl_bitmap, "num_vcpumaps")),
> >
> > Looking at one of Dario's patches I became confused about how this new
> > field relates to the existing cpumap field.
> >
> > Am I right that the new field is just a per-vcpu version of the old
> > (which only allows you to set the affinity of every vcpu together)?
> >
>
> Yes, you're right. The old field denotes the PCPUs this domain is
> allowed to run on. The new array specifies the PCPUs each VCPU can run
> on.
>
Well, there's no "new" and "old". I mean, as can clearly be seen in the
hunk above, 'cpumap' is still there, and vcpu_affinity is being added.
Fact is, considering how xl parses XXX in "cpus=XXX", only one between
'cpumap' and 'vcpu_affinity' will contain actual information.
With my series on top of this one (or vice versa), we'll have:
("cpumap", libxl_bitmap)
("cpumap_soft", libxl_bitmap)
("vcpu_affinity", Array(libxl_bitmap, "num_vcpumaps"))
which is missing the soft affinity equivalent of 'vcpu_affinity'.
meaning that, either me or Wei, when rebasing on top of the other series
which went in first, will have to add it, meaning we'll end up with the
following:
("cpumap", libxl_bitmap)
("cpumap_soft", libxl_bitmap)
("vcpu_hard_affinity", Array(libxl_bitmap, "num_vcpumaps"))
("vcpu_soft_affinity", Array(libxl_bitmap, "num_vcpumaps"))
which is a lot of fields, and not particularly easy to understand, IMHO.
> > Can this relationship be mentioned in the commit message and/or comments
> > please.
> >
I think we could go farther than that... way farther. I mean, "cpumap"
contains the vcpu affinity to be applied to all the vcpus of the domain,
if the config file was something like "cpus="1,3-6". "vcpu_affinity"
contains a cpumap for each vcpu, if the config file was something like
"cpus=["1", "5"] (and all this repeated for soft affinity, with my
series).
Can't we kill "cpumap" (and "cpumap_soft" too, in my series) and use
"vcpu_affinity" (i.e., "vcpu_hard_affinity" and "vcpu_soft_affinity"
after my series) only? What would happen is, in case we find
"cpus="4-5", we set all the bitmaps of "vcpu_hard_affinity" to "4-5".
What do you think?
Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v6 04/18] libxl: libxl_uuid_copy now takes a ctx argument
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (2 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 03/18] xl / libxl: push VCPU affinity pinning " Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-10 13:05 ` Ian Campbell
2014-06-09 12:43 ` [PATCH v6 05/18] xl: remove parsing of "vncviewer" option in xl domain config file Wei Liu
` (14 subsequent siblings)
18 siblings, 1 reply; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 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>
---
tools/libxl/libxl.c | 2 +-
tools/libxl/libxl.h | 12 +++++++++++-
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, 25 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 5f320ca..0ce9153 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2006,7 +2006,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 20aa875..d14f25f 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
@@ -513,6 +515,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
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 ecc29c7..cb8546a 100644
--- a/tools/libxl/libxl_uuid.c
+++ b/tools/libxl/libxl_uuid.c
@@ -14,7 +14,7 @@
#include "libxl_osdeps.h" /* must come before any other headers */
-#include <libxl_uuid.h>
+#include <libxl_internal.h>
#include "libxl_internal.h"
@@ -35,7 +35,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);
}
@@ -82,7 +83,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 93c65a7..e68bfc1 100644
--- a/tools/libxl/libxl_uuid.h
+++ b/tools/libxl/libxl_uuid.h
@@ -56,7 +56,12 @@ typedef struct {
int libxl_uuid_is_nil(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(libxl_uuid *uuid1, libxl_uuid *uuid2);
uint8_t *libxl_uuid_bytearray(libxl_uuid *uuid);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v6 04/18] libxl: libxl_uuid_copy now takes a ctx argument
2014-06-09 12:43 ` [PATCH v6 04/18] libxl: libxl_uuid_copy now takes a ctx argument Wei Liu
@ 2014-06-10 13:05 ` Ian Campbell
2014-06-10 13:55 ` Wei Liu
0 siblings, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2014-06-10 13:05 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.jackson, xen-devel
On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 20aa875..d14f25f 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -319,13 +319,15 @@
>
> #include <xentoollog.h>
>
> +typedef struct libxl__ctx libxl_ctx;
Looks like you didn't move the existing one, but instead added a second?
> diff --git a/tools/libxl/libxl_uuid.c b/tools/libxl/libxl_uuid.c
> index ecc29c7..cb8546a 100644
> --- a/tools/libxl/libxl_uuid.c
> +++ b/tools/libxl/libxl_uuid.c
> @@ -14,7 +14,7 @@
>
> #include "libxl_osdeps.h" /* must come before any other headers */
>
> -#include <libxl_uuid.h>
> +#include <libxl_internal.h>
>
> #include "libxl_internal.h"
I'm getting deja vu ;-)
Other than those two things looks good. With them resolved:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v6 04/18] libxl: libxl_uuid_copy now takes a ctx argument
2014-06-10 13:05 ` Ian Campbell
@ 2014-06-10 13:55 ` Wei Liu
0 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-10 13:55 UTC (permalink / raw)
To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel
On Tue, Jun 10, 2014 at 02:05:11PM +0100, Ian Campbell wrote:
> On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index 20aa875..d14f25f 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -319,13 +319,15 @@
> >
> > #include <xentoollog.h>
> >
> > +typedef struct libxl__ctx libxl_ctx;
>
> Looks like you didn't move the existing one, but instead added a second?
>
> > diff --git a/tools/libxl/libxl_uuid.c b/tools/libxl/libxl_uuid.c
> > index ecc29c7..cb8546a 100644
> > --- a/tools/libxl/libxl_uuid.c
> > +++ b/tools/libxl/libxl_uuid.c
> > @@ -14,7 +14,7 @@
> >
> > #include "libxl_osdeps.h" /* must come before any other headers */
> >
> > -#include <libxl_uuid.h>
> > +#include <libxl_internal.h>
> >
> > #include "libxl_internal.h"
>
> I'm getting deja vu ;-)
>
> Other than those two things looks good. With them resolved:
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
Done. Thanks.
> Ian.
>
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v6 05/18] xl: remove parsing of "vncviewer" option in xl domain config file
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (3 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 04/18] libxl: libxl_uuid_copy now takes a ctx argument Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 06/18] libxl: fix JSON generator for uint64_t Wei Liu
` (13 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell
Print out a warning and suggest user use "-V" option when invoking "xl
create". Also remove that option in manpage. This will introduce a
minor functional regression but it's very easy to work around.
The rationale behind this change is that, this option is actually not
part of domain configuration. It just affects whether a vncviewer
should be automatically spawn, but has nothing to do with how a domain
should be constructed. And this option is also bogus, considering if you
migrate a domain to a remote host and the receiver spawns a vncviewer on
the receiving side then it either dies silently or occupies resource.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
docs/man/xl.cfg.pod.5 | 4 ----
tools/libxl/xl_cmdimpl.c | 21 ++++++++-------------
2 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index a94d037..c087cbc 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1114,10 +1114,6 @@ The default is cirrus.
Allow access to the display via the VNC protocol. This enables the
other VNC-related settings. The default is to enable this.
-=item B<vncviewer=BOOLEAN>
-
-Automatically spawn a vncviewer when creating/restoring a guest.
-
=item B<vnclisten="ADDRESS[:DISPLAYNUM]">
Specifies the IP address, and optionally VNC display number, to use.
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 6901bac..3a33cff 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -693,9 +693,7 @@ static void parse_top_level_sdl_options(XLU_Config *config,
static void parse_config_data(const char *config_source,
const char *config_data,
int config_len,
- libxl_domain_config *d_config,
- struct domain_create *dom_info)
-
+ libxl_domain_config *d_config)
{
const char *buf;
long l;
@@ -929,12 +927,9 @@ static void parse_config_data(const char *config_source,
if (!xlu_cfg_get_long(config, "rtc_timeoffset", &l, 0))
b_info->rtc_timeoffset = l;
- if (dom_info && !xlu_cfg_get_long(config, "vncviewer", &l, 0)) {
- /* Command line arguments must take precedence over what's
- * specified in the configuration file. */
- if (!dom_info->vnc)
- dom_info->vnc = l;
- }
+ if (!xlu_cfg_get_long(config, "vncviewer", &l, 0))
+ fprintf(stderr, "WARNING: ignoring \"vncviewer\" option. "
+ "Use \"-V\" option of \"xl create\" to automatically spawn vncviewer.\n");
xlu_cfg_get_defbool(config, "localtime", &b_info->localtime, 0);
@@ -2117,7 +2112,7 @@ static uint32_t create_domain(struct domain_create *dom_info)
if (!dom_info->quiet)
printf("Parsing config from %s\n", config_source);
- parse_config_data(config_source, config_data, config_len, &d_config, dom_info);
+ parse_config_data(config_source, config_data, config_len, &d_config);
if (migrate_fd >= 0) {
if (d_config.c_info.name) {
@@ -2292,7 +2287,7 @@ start:
libxl_domain_config_dispose(&d_config);
libxl_domain_config_init(&d_config);
parse_config_data(config_source, config_data, config_len,
- &d_config, dom_info);
+ &d_config);
/*
* XXX FIXME: If this sleep is not there then domain
@@ -3103,7 +3098,7 @@ static void list_domains_details(const libxl_dominfo *info, int nb_domain)
continue;
CHK_SYSCALL(asprintf(&config_source, "<domid %d data>", info[i].domid));
libxl_domain_config_init(&d_config);
- parse_config_data(config_source, (char *)data, len, &d_config, NULL);
+ parse_config_data(config_source, (char *)data, len, &d_config);
if (default_output_format == OUTPUT_FORMAT_JSON)
s = printf_info_one_json(hand, info[i].domid, &d_config);
else
@@ -4404,7 +4399,7 @@ int main_config_update(int argc, char **argv)
libxl_domain_config_init(&d_config);
- parse_config_data(filename, config_data, config_len, &d_config, NULL);
+ parse_config_data(filename, config_data, config_len, &d_config);
if (debug || dryrun_only)
printf_info(default_output_format, -1, &d_config);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v6 06/18] libxl: fix JSON generator for uint64_t
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (4 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 05/18] xl: remove parsing of "vncviewer" option in xl domain config file Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 07/18] libxl IDL: rename json_fn to json_gen_fn Wei Liu
` (12 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell
yajl_gen_integer cannot cope with uint64_t, because it takes a signed
long long. If we pass to it an uint64_t number which is between INT_MAX
and UINT_MAX, it generates a negative number. Later when we feed this
generated number into parser, the result gets signed extended, which is
wrong.
A new function called libxl__uint64_gen_json is introduced to handle
uint64_t. It utilises yajl_gen_number to generate numbers.
Also removed a duplicated definition of MemKB while I was there.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/libxl/idl.py | 2 +-
tools/libxl/libxl_json.c | 21 +++++++++++++++++++++
tools/libxl/libxl_json.h | 1 +
tools/libxl/libxl_types.idl | 4 +---
4 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/tools/libxl/idl.py b/tools/libxl/idl.py
index e4dc79b..69e08e1 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -266,7 +266,7 @@ integer = Number("int", namespace = None, signed = True)
uint8 = UInt(8)
uint16 = UInt(16)
uint32 = UInt(32)
-uint64 = UInt(64)
+uint64 = UInt(64, json_fn = "libxl__uint64_gen_json")
string = Builtin("char *", namespace = None, dispose_fn = "free",
json_fn = "libxl__string_gen_json",
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index ab964ab..30cfd20 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -781,6 +781,27 @@ out:
return ret;
}
+yajl_gen_status libxl__uint64_gen_json(yajl_gen hand, uint64_t val)
+{
+ char *num;
+ unsigned int len;
+ yajl_gen_status s;
+
+
+ len = asprintf(&num, "%"PRIu64, val);
+ if (len == -1) {
+ s = yajl_gen_in_error_state;
+ goto out;
+ }
+
+ s = yajl_gen_number(hand, num, len);
+
+ free(num);
+
+out:
+ return s;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_json.h b/tools/libxl/libxl_json.h
index a4dd8fc..a45d429 100644
--- a/tools/libxl/libxl_json.h
+++ b/tools/libxl/libxl_json.h
@@ -22,6 +22,7 @@
# include <yajl/yajl_version.h>
#endif
+yajl_gen_status libxl__uint64_gen_json(yajl_gen hand, uint64_t val);
yajl_gen_status libxl_defbool_gen_json(yajl_gen hand, libxl_defbool *p);
yajl_gen_status libxl_domid_gen_json(yajl_gen hand, libxl_domid *p);
yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, libxl_uuid *p);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 4765fb6..cbc16ce 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -22,7 +22,7 @@ libxl_hwcap = Builtin("hwcap", passby=PASS_BY_REFERENCE)
# Specific integer types
#
-MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
+MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT", json_fn = "libxl__uint64_gen_json")
#
# Constants / Enumerations
@@ -288,8 +288,6 @@ libxl_domain_restore_params = Struct("domain_restore_params", [
("checkpointed_stream", integer),
])
-MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT")
-
libxl_domain_sched_params = Struct("domain_sched_params",[
("sched", libxl_scheduler),
("weight", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT'}),
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v6 07/18] libxl IDL: rename json_fn to json_gen_fn
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (5 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 06/18] libxl: fix JSON generator for uint64_t Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 08/18] libxl_json: introduce libxl__object_from_json Wei Liu
` (11 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell
This json_fn is in fact used to generate string representation of a json
data structure. We will introduce another json function to parse json
data structure in later changeset, so rename json_fn to json_gen_fn to
clarify.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/gentest.py | 4 ++--
tools/libxl/gentypes.py | 12 ++++++------
tools/libxl/idl.py | 12 ++++++------
tools/libxl/idl.txt | 4 ++--
tools/libxl/libxl_types.idl | 6 +++---
tools/libxl/libxl_types_internal.idl | 2 +-
6 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/tools/libxl/gentest.py b/tools/libxl/gentest.py
index 722b7f4..eb9a21b 100644
--- a/tools/libxl/gentest.py
+++ b/tools/libxl/gentest.py
@@ -52,7 +52,7 @@ def gen_rand_init(ty, v, indent = " ", parent = None):
s += " break;\n"
s += "}\n"
elif isinstance(ty, idl.Struct) \
- and (parent is None or ty.json_fn is None):
+ and (parent is None or ty.json_gen_fn is None):
for f in [f for f in ty.fields if not f.const]:
(nparent,fexpr) = ty.member(v, f, parent is None)
s += gen_rand_init(f.type, fexpr, "", nparent)
@@ -243,7 +243,7 @@ int main(int argc, char **argv)
f.write(" printf(\"Testing TYPE_to_json()\\n\");\n")
f.write(" printf(\"----------------------\\n\");\n")
f.write(" printf(\"\\n\");\n")
- for ty in [t for t in types if t.json_fn is not None]:
+ for ty in [t for t in types if t.json_gen_fn is not None]:
arg = ty.typename + "_val"
f.write(" %s_rand_init(%s);\n" % (ty.typename, \
ty.pass_arg(arg, isref=False, passby=idl.PASS_BY_REFERENCE)))
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 917e2c2..61a2b3d 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -229,7 +229,7 @@ def libxl_C_type_gen_json(ty, v, indent = " ", parent = None):
s += " goto out;\n"
s += " break;\n"
s += "}\n"
- elif isinstance(ty, idl.Struct) and (parent is None or ty.json_fn is None):
+ elif isinstance(ty, idl.Struct) and (parent is None or ty.json_gen_fn is None):
s += "s = yajl_gen_map_open(hand);\n"
s += "if (s != yajl_gen_status_ok)\n"
s += " goto out;\n"
@@ -243,8 +243,8 @@ def libxl_C_type_gen_json(ty, v, indent = " ", parent = None):
s += "if (s != yajl_gen_status_ok)\n"
s += " goto out;\n"
else:
- if ty.json_fn is not None:
- s += "s = %s(hand, %s);\n" % (ty.json_fn, ty.pass_arg(v, parent is None))
+ if ty.json_gen_fn is not None:
+ s += "s = %s(hand, %s);\n" % (ty.json_gen_fn, ty.pass_arg(v, parent is None))
s += "if (s != yajl_gen_status_ok)\n"
s += " goto out;\n"
@@ -341,7 +341,7 @@ if __name__ == '__main__':
f.write("%svoid %s(%s, %s);\n" % (ty.hidden(), ty.init_fn + "_" + ku.keyvar.name,
ty.make_arg("p"),
ku.keyvar.type.make_arg(ku.keyvar.name)))
- if ty.json_fn is not None:
+ if ty.json_gen_fn is not None:
f.write("%schar *%s_to_json(libxl_ctx *ctx, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p")))
if isinstance(ty, idl.Enumeration):
f.write("%sconst char *%s_to_string(%s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p")))
@@ -369,7 +369,7 @@ if __name__ == '__main__':
""" % (header_json_define, header_json_define, " ".join(sys.argv)))
- for ty in [ty for ty in types if ty.json_fn is not None]:
+ for ty in [ty for ty in types if ty.json_gen_fn is not None]:
f.write("%syajl_gen_status %s_gen_json(yajl_gen hand, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
f.write("\n")
@@ -426,7 +426,7 @@ if __name__ == '__main__':
f.write("}\n")
f.write("\n")
- for ty in [t for t in types if t.json_fn is not None]:
+ for ty in [t for t in types if t.json_gen_fn is not None]:
f.write("yajl_gen_status %s_gen_json(yajl_gen hand, %s)\n" % (ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
f.write("{\n")
f.write(libxl_C_type_gen_json(ty, "p"))
diff --git a/tools/libxl/idl.py b/tools/libxl/idl.py
index 69e08e1..8b118dd 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -65,9 +65,9 @@ class Type(object):
self.autogenerate_init_fn = kwargs.setdefault('autogenerate_init_fn', False)
if self.typename is not None and not self.private:
- self.json_fn = kwargs.setdefault('json_fn', self.typename + "_gen_json")
+ self.json_gen_fn = kwargs.setdefault('json_gen_fn', self.typename + "_gen_json")
else:
- self.json_fn = kwargs.setdefault('json_fn', None)
+ self.json_gen_fn = kwargs.setdefault('json_gen_fn', None)
self.autogenerate_json = kwargs.setdefault('autogenerate_json', True)
@@ -118,7 +118,7 @@ class Number(Builtin):
kwargs.setdefault('namespace', None)
kwargs.setdefault('dispose_fn', None)
kwargs.setdefault('signed', False)
- kwargs.setdefault('json_fn', "yajl_gen_integer")
+ kwargs.setdefault('json_gen_fn', "yajl_gen_integer")
self.signed = kwargs['signed']
Builtin.__init__(self, ctype, **kwargs)
@@ -256,7 +256,7 @@ class KeyedUnion(Aggregate):
void = Builtin("void *", namespace = None)
bool = Builtin("bool", namespace = None,
- json_fn = "yajl_gen_bool",
+ json_gen_fn = "yajl_gen_bool",
autogenerate_json = False)
size_t = Number("size_t", namespace = None)
@@ -266,10 +266,10 @@ integer = Number("int", namespace = None, signed = True)
uint8 = UInt(8)
uint16 = UInt(16)
uint32 = UInt(32)
-uint64 = UInt(64, json_fn = "libxl__uint64_gen_json")
+uint64 = UInt(64, json_gen_fn = "libxl__uint64_gen_json")
string = Builtin("char *", namespace = None, dispose_fn = "free",
- json_fn = "libxl__string_gen_json",
+ json_gen_fn = "libxl__string_gen_json",
autogenerate_json = False)
class Array(Type):
diff --git a/tools/libxl/idl.txt b/tools/libxl/idl.txt
index 439aede..6a53dd8 100644
--- a/tools/libxl/idl.txt
+++ b/tools/libxl/idl.txt
@@ -60,14 +60,14 @@ Type.autogenerate_init_fn: (default: True if dir in [IN, BOTH])
Indicates if the above named Type.init_fn should be
autogenerated.
-Type.json_fn: (default: typename + "_gen_json" or None if type == None)
+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.autogenerate_json: (default: True)
- Indicates if the above named Type.json_fn should be autogenerated.
+ Indicates if the above named Type.json_gen_fn should be autogenerated.
Other simple type-Classes
-------------------------
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index cbc16ce..95d450f 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -7,8 +7,8 @@ namespace("libxl_")
libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
-libxl_domid = Builtin("domid", json_fn = "yajl_gen_integer", autogenerate_json = False)
-libxl_devid = Builtin("devid", json_fn = "yajl_gen_integer", autogenerate_json = False, signed = True, init_val="-1")
+libxl_domid = Builtin("domid", json_gen_fn = "yajl_gen_integer", autogenerate_json = False)
+libxl_devid = Builtin("devid", json_gen_fn = "yajl_gen_integer", autogenerate_json = False, signed = True, init_val="-1")
libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE)
libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE)
libxl_bitmap = Builtin("bitmap", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE)
@@ -22,7 +22,7 @@ libxl_hwcap = Builtin("hwcap", passby=PASS_BY_REFERENCE)
# Specific integer types
#
-MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT", json_fn = "libxl__uint64_gen_json")
+MemKB = UInt(64, init_val = "LIBXL_MEMKB_DEFAULT", json_gen_fn = "libxl__uint64_gen_json")
#
# Constants / Enumerations
diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl
index cb9444f..a964851 100644
--- a/tools/libxl/libxl_types_internal.idl
+++ b/tools/libxl/libxl_types_internal.idl
@@ -1,7 +1,7 @@
namespace("libxl__")
hidden(True)
-libxl_domid = Builtin("domid", namespace="libxl_", json_fn = "yajl_gen_integer")
+libxl_domid = Builtin("domid", namespace="libxl_", json_gen_fn = "yajl_gen_integer")
libxl__qmp_message_type = Enumeration("qmp_message_type", [
(1, "QMP"),
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v6 08/18] libxl_json: introduce libxl__object_from_json
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (6 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 07/18] libxl IDL: rename json_fn to json_gen_fn Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 09/18] libxl_json: introduce parser functions for builtin types Wei Liu
` (10 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell
Given a JSON string, we need to convert it to libxl_FOO struct.
The approach is:
JSON string -> libxl__json_object -> libxl_FOO struct
With this approach we can make use of libxl's infrastructure to do the
first half (JSON string -> libxl__json_object).
Second half is done by auto-generated code by libxl's IDL
infrastructure. IDL patch(es) will come later.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl_internal.h | 8 ++++++++
tools/libxl/libxl_json.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 082749e..89bbf7d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1640,6 +1640,14 @@ typedef struct libxl__json_object {
struct libxl__json_object *parent;
} libxl__json_object;
+typedef int (*libxl__json_parse_callback)(libxl__gc *gc,
+ libxl__json_object *o,
+ void *p);
+_hidden int libxl__object_from_json(libxl_ctx *ctx, const char *type,
+ libxl__json_parse_callback parse,
+ void *p,
+ const char *s);
+
typedef struct {
char *map_key;
libxl__json_object *obj;
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index 30cfd20..8440498 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -802,6 +802,36 @@ out:
return s;
}
+int libxl__object_from_json(libxl_ctx *ctx, const char *type,
+ libxl__json_parse_callback parse,
+ void *p, const char *s)
+{
+ GC_INIT(ctx);
+ libxl__json_object *o;
+ int rc;
+
+ o = libxl__json_parse(gc, s);
+ if (!o) {
+ LOG(ERROR,
+ "unable to generate libxl__json_object from JSON representation of %s.",
+ type);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = parse(gc, o, p);
+ if (rc) {
+ LOG(ERROR, "unable to convert libxl__json_object to %s. (rc=%d)", type, rc);
+ rc = ERROR_FAIL;
+ goto out;
+ }
+
+ rc = 0;
+out:
+ GC_FREE;
+ return rc;
+}
+
/*
* Local variables:
* mode: C
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v6 09/18] libxl_json: introduce parser functions for builtin types
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (7 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 08/18] libxl_json: introduce libxl__object_from_json Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 10/18] libxl/gentypes.py: special-case KeyedUnion map handle generation Wei Liu
` (9 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell
This changeset introduces following functions:
* libxl_defbool_parse_json
* libxl__bool_parse_json
* libxl_uuid_parse_json
* libxl_mac_parse_json
* libxl_bitmap_parse_json
* libxl_cpuid_policy_list_parse_json
* libxl_string_list_parse_json
* libxl_key_value_list_parse_json
* libxl_hwcap_parse_json
* libxl__int_parse_json
* libxl__uint{8,16,32,64}_parse_json
* libxl__string_parse_json
They will be used in later patch to convert the libxl__json_object
tree of a builtin type to libxl_FOO struct.
Also remove declaration of libxl_domid_gen_json as libxl_domid uses
yajl_gen_integer to generate JSON object.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Anthony Perard <anthony.perard@citrix.com>
---
tools/libxl/libxl_cpuid.c | 89 +++++++++++---
tools/libxl/libxl_json.c | 271 +++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_json.h | 36 +++++-
tools/libxl/libxl_nocpuid.c | 7 ++
4 files changed, 386 insertions(+), 17 deletions(-)
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 3787213..d9007b2 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -338,29 +338,29 @@ void libxl_cpuid_set(libxl_ctx *ctx, uint32_t domid,
(const char**)(cpuid[i].policy), cpuid_res);
}
+static const char *input_names[2] = { "leaf", "subleaf" };
+static const char *policy_names[4] = { "eax", "ebx", "ecx", "edx" };
+/*
+ * Aiming for:
+ * [
+ * { 'leaf': 'val-eax',
+ * 'subleaf': 'val-ecx',
+ * 'eax': 'filter',
+ * 'ebx': 'filter',
+ * 'ecx': 'filter',
+ * 'edx': 'filter' },
+ * { 'leaf': 'val-eax', ..., 'eax': 'filter', ... },
+ * ... etc ...
+ * ]
+ */
+
yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
libxl_cpuid_policy_list *pcpuid)
{
libxl_cpuid_policy_list cpuid = *pcpuid;
yajl_gen_status s;
- const char *input_names[2] = { "leaf", "subleaf" };
- const char *policy_names[4] = { "eax", "ebx", "ecx", "edx" };
int i, j;
- /*
- * Aiming for:
- * [
- * { 'leaf': 'val-eax',
- * 'subleaf': 'val-ecx',
- * 'eax': 'filter',
- * 'ebx': 'filter',
- * 'ecx': 'filter',
- * 'edx': 'filter' },
- * { 'leaf': 'val-eax', ..., 'eax': 'filter', ... },
- * ... etc ...
- * ]
- */
-
s = yajl_gen_array_open(hand);
if (s != yajl_gen_status_ok) goto out;
@@ -398,6 +398,63 @@ out:
return s;
}
+int libxl_cpuid_policy_list_parse_json(libxl__gc *gc,
+ const libxl__json_object *o,
+ libxl_cpuid_policy_list *p)
+{
+ int i, size;
+ libxl_cpuid_policy_list l;
+ flexarray_t *array;
+
+ if (!libxl__json_object_is_array(o))
+ return ERROR_FAIL;
+
+ array = libxl__json_object_get_array(o);
+ if (!array->count)
+ return 0;
+
+ size = array->count;
+ /* need one extra slot as sentinel */
+ l = *p = libxl__calloc(NOGC, size + 1, sizeof(libxl_cpuid_policy));
+
+ l[size].input[0] = XEN_CPUID_INPUT_UNUSED;
+ l[size].input[1] = XEN_CPUID_INPUT_UNUSED;
+
+ for (i = 0; i < size; i++) {
+ const libxl__json_object *t;
+ int j;
+
+ if (flexarray_get(array, i, (void**)&t) != 0)
+ return ERROR_FAIL;
+
+ if (!libxl__json_object_is_map(t))
+ return ERROR_FAIL;
+
+ for (j = 0; j < ARRAY_SIZE(l[0].input); j++) {
+ const libxl__json_object *r;
+
+ r = libxl__json_map_get(input_names[j], t, JSON_INTEGER);
+ if (!r)
+ l[i].input[j] = XEN_CPUID_INPUT_UNUSED;
+ else
+ l[i].input[j] = libxl__json_object_get_integer(r);
+ }
+
+ for (j = 0; j < ARRAY_SIZE(l[0].policy); j++) {
+ const libxl__json_object *r;
+
+ r = libxl__json_map_get(policy_names[j], t, JSON_STRING);
+ if (!r)
+ l[i].policy[j] = NULL;
+ else
+ l[i].policy[j] =
+ libxl__strdup(NOGC, libxl__json_object_get_string(r));
+ }
+ }
+
+ return 0;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index 8440498..fb9baf8 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -100,6 +100,42 @@ yajl_gen_status libxl_defbool_gen_json(yajl_gen hand,
return libxl__yajl_gen_asciiz(hand, libxl_defbool_to_string(*db));
}
+int libxl_defbool_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_defbool *p)
+{
+ const char *s;
+
+ if (!libxl__json_object_is_string(o))
+ return ERROR_FAIL;
+
+ s = libxl__json_object_get_string(o);
+
+ if (!strncmp(s, LIBXL__DEFBOOL_STR_DEFAULT,
+ strlen(LIBXL__DEFBOOL_STR_DEFAULT)))
+ p->val = LIBXL__DEFBOOL_DEFAULT;
+ else if (!strncmp(s, LIBXL__DEFBOOL_STR_TRUE,
+ strlen(LIBXL__DEFBOOL_STR_TRUE)))
+ p->val = LIBXL__DEFBOOL_TRUE;
+ else if (!strncmp(s, LIBXL__DEFBOOL_STR_FALSE,
+ strlen(LIBXL__DEFBOOL_STR_FALSE)))
+ p->val = LIBXL__DEFBOOL_FALSE;
+ else
+ return ERROR_FAIL;
+
+ return 0;
+}
+
+int libxl__bool_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ bool *p)
+{
+ if (!libxl__json_object_is_bool(o))
+ return ERROR_FAIL;
+
+ *p = libxl__json_object_get_bool(o);
+
+ return 0;
+}
+
yajl_gen_status libxl_uuid_gen_json(yajl_gen hand,
libxl_uuid *uuid)
{
@@ -108,6 +144,15 @@ yajl_gen_status libxl_uuid_gen_json(yajl_gen hand,
return yajl_gen_string(hand, (const unsigned char *)buf, LIBXL_UUID_FMTLEN);
}
+int libxl_uuid_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_uuid *p)
+{
+ if (!libxl__json_object_is_string(o))
+ return ERROR_FAIL;
+
+ return libxl_uuid_from_string(p, o->u.string);
+}
+
yajl_gen_status libxl_bitmap_gen_json(yajl_gen hand,
libxl_bitmap *bitmap)
{
@@ -128,6 +173,40 @@ out:
return s;
}
+int libxl_bitmap_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_bitmap *p)
+{
+ int i;
+ int size;
+ const libxl__json_object *t;
+ flexarray_t *array;
+
+ if (!libxl__json_object_is_array(o))
+ return ERROR_FAIL;
+
+ array = libxl__json_object_get_array(o);
+ if (!array->count) {
+ libxl_bitmap_init(p);
+ return 0;
+ }
+
+ t = libxl__json_array_get(o, array->count - 1);
+ if (!libxl__json_object_is_integer(t))
+ return ERROR_FAIL;
+ size = libxl__json_object_get_integer(t) + 1;
+
+ libxl_bitmap_alloc(CTX, p, size);
+
+ for (i = 0; (t = libxl__json_array_get(o, i)); i++) {
+ if (!libxl__json_object_is_integer(t))
+ return ERROR_FAIL;
+
+ libxl_bitmap_set(p, libxl__json_object_get_integer(t));
+ }
+
+ return 0;
+}
+
yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
libxl_key_value_list *pkvl)
{
@@ -155,6 +234,41 @@ out:
return s;
}
+int libxl_key_value_list_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_key_value_list *p)
+{
+ libxl__json_map_node *node = NULL;
+ flexarray_t *maps = NULL;
+ int i, size;
+ libxl_key_value_list kvl;
+
+ if (!libxl__json_object_is_map(o))
+ return ERROR_FAIL;
+
+ maps = libxl__json_object_get_map(o);
+ size = maps->count * 2;
+ kvl = *p = libxl__calloc(NOGC, size, sizeof(char *));
+
+ for (i = 0; i < maps->count; i++) {
+ int idx = i * 2;
+ if (flexarray_get(maps, i, (void**)&node) != 0)
+ return ERROR_FAIL;
+
+ if (!libxl__json_object_is_string(node->obj) &&
+ !libxl__json_object_is_null(node->obj))
+ return ERROR_FAIL;
+
+ kvl[idx] = libxl__strdup(NOGC, node->map_key);
+ if (libxl__json_object_is_string(node->obj))
+ kvl[idx+1] =
+ libxl__strdup(NOGC, libxl__json_object_get_string(node->obj));
+ else
+ kvl[idx+1] = NULL;
+ }
+
+ return 0;
+}
+
yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *pl)
{
libxl_string_list l = *pl;
@@ -176,6 +290,38 @@ out:
return s;
}
+int libxl_string_list_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_string_list *p)
+{
+ const libxl__json_object *t;
+ libxl_string_list l;
+ flexarray_t *array = NULL;
+ int i, size;
+
+ if (!libxl__json_object_is_array(o))
+ return ERROR_FAIL;
+
+ array = libxl__json_object_get_array(o);
+ size = array->count;
+
+ if (size == 0) {
+ *p = NULL;
+ return 0;
+ }
+
+ /* need one extra slot as sentinel */
+ l = *p = libxl__calloc(NOGC, size + 1, sizeof(char *));
+
+ for (i = 0; (t = libxl__json_array_get(o, i)); i++) {
+ if (!libxl__json_object_is_string(t))
+ return ERROR_FAIL;
+
+ l[i] = libxl__strdup(NOGC, libxl__json_object_get_string(t));
+ }
+
+ return 0;
+}
+
yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *mac)
{
char buf[LIBXL_MAC_FMTLEN+1];
@@ -183,6 +329,15 @@ yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *mac)
return yajl_gen_string(hand, (const unsigned char *)buf, LIBXL_MAC_FMTLEN);
}
+int libxl_mac_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_mac *p)
+{
+ if (!libxl__json_object_is_string(o))
+ return ERROR_FAIL;
+
+ return libxl__parse_mac(libxl__json_object_get_string(o), *p);
+}
+
yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand,
libxl_hwcap *p)
{
@@ -201,6 +356,27 @@ out:
return s;
}
+int libxl_hwcap_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_hwcap *p)
+{
+ int i;
+
+ if (!libxl__json_object_is_array(o))
+ return ERROR_FAIL;
+
+ for (i = 0; i<4; i++) {
+ const libxl__json_object *t;
+
+ t = libxl__json_array_get(o, i);
+ if (!t || !libxl__json_object_is_integer(t))
+ return ERROR_FAIL;
+
+ (*p)[i] = libxl__json_object_get_integer(t);
+ }
+
+ return 0;
+}
+
yajl_gen_status libxl__string_gen_json(yajl_gen hand,
const char *p)
{
@@ -210,6 +386,20 @@ yajl_gen_status libxl__string_gen_json(yajl_gen hand,
return yajl_gen_null(hand);
}
+int libxl__string_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ char **p)
+{
+ if (!libxl__json_object_is_string(o) && !libxl__json_object_is_null(o))
+ return ERROR_FAIL;
+
+ if (libxl__json_object_is_null(o))
+ *p = NULL;
+ else
+ *p = libxl__strdup(NOGC, libxl__json_object_get_string(o));
+
+ return 0;
+}
+
/*
* libxl__json_object helper functions
*/
@@ -832,6 +1022,87 @@ out:
return rc;
}
+int libxl__int_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p)
+{
+ long long i;
+
+ if (!libxl__json_object_is_integer(o))
+ return ERROR_FAIL;
+
+ i = libxl__json_object_get_integer(o);
+
+ if (i > INT_MAX || i < INT_MIN)
+ return ERROR_FAIL;
+
+ *((int *)p) = i;
+
+ return 0;
+}
+
+/* Macro to generate:
+ * libxl__uint8_parse_json
+ * libxl__uint16_parse_json
+ * libxl__uint32_parse_json
+ */
+#define PARSE_UINT(width) \
+ int libxl__uint ## width ## _parse_json(libxl__gc *gc, \
+ const libxl__json_object *o,\
+ void *p) \
+ { \
+ long long i; \
+ \
+ if (!libxl__json_object_is_integer(o)) \
+ return ERROR_FAIL; \
+ \
+ i = libxl__json_object_get_integer(o); \
+ \
+ if (i < 0 || i > UINT ## width ## _MAX) \
+ return ERROR_FAIL; \
+ \
+ *((uint ## width ## _t *)p) = i; \
+ \
+ return 0; \
+ }
+
+PARSE_UINT(8);
+PARSE_UINT(16);
+PARSE_UINT(32);
+
+int libxl__uint64_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p)
+{
+ if (!libxl__json_object_is_integer(o) &&
+ !libxl__json_object_is_number(o))
+ return ERROR_FAIL;
+
+ if (libxl__json_object_is_integer(o)) {
+ long long i = libxl__json_object_get_integer(o);
+
+ if (i < 0)
+ return ERROR_FAIL;
+
+ *((uint64_t *)p) = i;
+ } else {
+ const char *s;
+ unsigned long long i;
+ int saved_errno = errno;
+
+ s = libxl__json_object_get_number(o);
+
+ errno = 0;
+ i = strtoull(s, NULL, 10);
+
+ if (i == ULLONG_MAX && errno == ERANGE)
+ return ERROR_FAIL;
+
+ errno = saved_errno;
+ *((uint64_t *)p) = i;
+ }
+
+ return 0;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_json.h b/tools/libxl/libxl_json.h
index a45d429..b196c1c 100644
--- a/tools/libxl/libxl_json.h
+++ b/tools/libxl/libxl_json.h
@@ -22,18 +22,52 @@
# include <yajl/yajl_version.h>
#endif
+typedef struct libxl__gc libxl__gc;
+typedef struct libxl__json_object libxl__json_object;
+
yajl_gen_status libxl__uint64_gen_json(yajl_gen hand, uint64_t val);
yajl_gen_status libxl_defbool_gen_json(yajl_gen hand, libxl_defbool *p);
-yajl_gen_status libxl_domid_gen_json(yajl_gen hand, libxl_domid *p);
+int libxl_defbool_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_defbool *p);
+int libxl__bool_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ bool *p);
yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, libxl_uuid *p);
+int libxl_uuid_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_uuid *p);
yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *p);
+int libxl_mac_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_mac *p);
yajl_gen_status libxl_bitmap_gen_json(yajl_gen hand, libxl_bitmap *p);
+int libxl_bitmap_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_bitmap *p);
yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
libxl_cpuid_policy_list *p);
+int libxl_cpuid_policy_list_parse_json(libxl__gc *gc,
+ const libxl__json_object *o,
+ libxl_cpuid_policy_list *p);
yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *p);
+int libxl_string_list_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_string_list *p);
yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
libxl_key_value_list *p);
+int libxl_key_value_list_parse_json(libxl__gc *gc,
+ const libxl__json_object *o,
+ libxl_key_value_list *p);
yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand, libxl_hwcap *p);
+int libxl_hwcap_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_hwcap *p);
+int libxl__int_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p);
+int libxl__uint8_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p);
+int libxl__uint16_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p);
+int libxl__uint32_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p);
+int libxl__uint64_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p);
+int libxl__string_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ char **p);
#include <_libxl_types_json.h>
diff --git a/tools/libxl/libxl_nocpuid.c b/tools/libxl/libxl_nocpuid.c
index 5f7cb6a..eb525fc 100644
--- a/tools/libxl/libxl_nocpuid.c
+++ b/tools/libxl/libxl_nocpuid.c
@@ -44,6 +44,13 @@ yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
return 0;
}
+int libxl_cpuid_policy_list_parse_json(libxl__gc *gc,
+ const libxl__json_object *o,
+ libxl_cpuid_policy_list *p)
+{
+ return 0;
+}
+
/*
* Local variables:
* mode: C
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v6 10/18] libxl/gentypes.py: special-case KeyedUnion map handle generation
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (8 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 09/18] libxl_json: introduce parser functions for builtin types Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 11/18] libxl/gentypes.py: don't generate default values Wei Liu
` (8 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell
Generate JSON map handle according to KeyedUnion discriminator.
The original JSON output for a keyed union is like:
{
...
"u" : { FIELDS }
...
}
The discriminator is not generated, so that the parser won't be able to
figure out the fields in the incoming stream.
So we need to change this to something more sensible. For example, for
keyed union libxl_domain_type, which has a discriminator called "type",
we generate following for HVM guest:
{
...
"type.hvm" : { HVM FIELDS }
...
}
Parser then can know the type of this union and how to interpret the
incoming stream.
Note that we change the existing API here. However the original output is
quite broken anyway, we cannot make sensible use of it and I doubt that
there's existing user of existing API. So we are acutally fixing a
problem.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/libxl/gentypes.py | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 61a2b3d..01416e7 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -186,6 +186,26 @@ def libxl_C_type_member_init(ty, field):
s += "\n"
return s
+def libxl_C_type_gen_map_key(f, parent, indent = ""):
+ s = ""
+ if isinstance(f.type, idl.KeyedUnion):
+ s += "switch (%s) {\n" % (parent + f.type.keyvar.name)
+ for x in f.type.fields:
+ v = f.type.keyvar.name + "." + x.name
+ s += "case %s:\n" % x.enumname
+ s += " s = yajl_gen_string(hand, (const unsigned char *)\"%s\", sizeof(\"%s\")-1);\n" % (v, v)
+ s += " if (s != yajl_gen_status_ok)\n"
+ s += " goto out;\n"
+ s += " break;\n"
+ s += "}\n"
+ else:
+ s += "s = yajl_gen_string(hand, (const unsigned char *)\"%s\", sizeof(\"%s\")-1);\n" % (f.name, f.name)
+ s += "if (s != yajl_gen_status_ok)\n"
+ s += " goto out;\n"
+ if s != "":
+ s = indent + s
+ return s.replace("\n", "\n%s" % indent).rstrip(indent)
+
def libxl_C_type_gen_json(ty, v, indent = " ", parent = None):
s = ""
if parent is None:
@@ -235,9 +255,7 @@ 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 += "s = yajl_gen_string(hand, (const unsigned char *)\"%s\", sizeof(\"%s\")-1);\n" % (f.name, f.name)
- s += "if (s != yajl_gen_status_ok)\n"
- s += " goto out;\n"
+ s += libxl_C_type_gen_map_key(f, nparent)
s += libxl_C_type_gen_json(f.type, fexpr, "", nparent)
s += "s = yajl_gen_map_close(hand);\n"
s += "if (s != yajl_gen_status_ok)\n"
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v6 11/18] libxl/gentypes.py: don't generate default values
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (9 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 10/18] libxl/gentypes.py: special-case KeyedUnion map handle generation Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-10 13:25 ` Ian Campbell
2014-06-09 12:43 ` [PATCH v6 12/18] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct Wei Liu
` (7 subsequent siblings)
18 siblings, 1 reply; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell
If a type has init_val defined and a field of the type has the value of
init_val, there's no need to generate output for that field in JSON
output. When the parser consumes that generated JSON object, all default
values should be automatically filled in.
Also define init_val for enumeration type, string type and libxl_defbool
type.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/libxl/gentypes.py | 27 ++++++++++++++++++++++++---
tools/libxl/idl.py | 3 ++-
tools/libxl/libxl_types.idl | 3 ++-
3 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 01416e7..611c8af 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -135,7 +135,7 @@ def _libxl_C_type_init(ty, v, indent = " ", parent = None, subinit=False):
else:
s += _libxl_C_type_init(f.type, fexpr, "", nparent)
else:
- if ty.init_val is not None:
+ if ty.init_val is not None and ty.typename != "libxl_defbool":
s += "%s = %s;\n" % (ty.pass_arg(v, parent is None), ty.init_val)
elif ty.init_fn is not None:
s += "%s(%s);\n" % (ty.init_fn, ty.pass_arg(v, parent is None))
@@ -206,6 +206,13 @@ 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 libxl_C_type_gen_json(ty, v, indent = " ", parent = None):
s = ""
if parent is None:
@@ -255,8 +262,22 @@ 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)
+ init_val = get_init_val(f)
+ if init_val:
+ if f.type.typename != "libxl_defbool":
+ s += "if (%s != %s) {\n" % (fexpr, init_val)
+ else:
+ s += "if (%s.val != %s) {\n" % (fexpr, init_val)
+ indent1 = " "
+ else:
+ indent1 = ""
+
+ s += libxl_C_type_gen_map_key(f, nparent, indent1)
+ s += libxl_C_type_gen_json(f.type, fexpr, indent1, nparent)
+
+ if init_val:
+ 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..14ca165 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -142,6 +142,7 @@ class EnumerationValue(object):
class Enumeration(Type):
def __init__(self, typename, values, **kwargs):
kwargs.setdefault('dispose_fn', None)
+ kwargs.setdefault('init_val', '0')
Type.__init__(self, typename, **kwargs)
self.value_namespace = kwargs.setdefault('value_namespace',
@@ -270,7 +271,7 @@ 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",
- autogenerate_json = False)
+ autogenerate_json = False, init_val = "NULL")
class Array(Type):
"""An array of the same type"""
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 95d450f..d510e2d 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -5,7 +5,8 @@
namespace("libxl_")
-libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
+libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE,
+ init_val="LIBXL__DEFBOOL_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")
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v6 11/18] libxl/gentypes.py: don't generate default values
2014-06-09 12:43 ` [PATCH v6 11/18] libxl/gentypes.py: don't generate default values Wei Liu
@ 2014-06-10 13:25 ` Ian Campbell
2014-06-10 13:43 ` Wei Liu
0 siblings, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2014-06-10 13:25 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.jackson, xen-devel
On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> If a type has init_val defined and a field of the type has the value of
> init_val, there's no need to generate output for that field in JSON
> output. When the parser consumes that generated JSON object, all default
> values should be automatically filled in.
>
> Also define init_val for enumeration type, string type and libxl_defbool
> type.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> tools/libxl/gentypes.py | 27 ++++++++++++++++++++++++---
> tools/libxl/idl.py | 3 ++-
> tools/libxl/libxl_types.idl | 3 ++-
> 3 files changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
> index 01416e7..611c8af 100644
> --- a/tools/libxl/gentypes.py
> +++ b/tools/libxl/gentypes.py
> @@ -135,7 +135,7 @@ def _libxl_C_type_init(ty, v, indent = " ", parent = None, subinit=False):
> else:
> s += _libxl_C_type_init(f.type, fexpr, "", nparent)
> else:
> - if ty.init_val is not None:
> + if ty.init_val is not None and ty.typename != "libxl_defbool":
Why is libxl_defbool special here?
> s += "%s = %s;\n" % (ty.pass_arg(v, parent is None), ty.init_val)
> elif ty.init_fn is not None:
> s += "%s(%s);\n" % (ty.init_fn, ty.pass_arg(v, parent is None))
> @@ -206,6 +206,13 @@ 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 libxl_C_type_gen_json(ty, v, indent = " ", parent = None):
> s = ""
> if parent is None:
> @@ -255,8 +262,22 @@ 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)
> + init_val = get_init_val(f)
> + if init_val:
> + if f.type.typename != "libxl_defbool":
> + s += "if (%s != %s) {\n" % (fexpr, init_val)
> + else:
> + s += "if (%s.val != %s) {\n" % (fexpr, init_val)
> + indent1 = " "
> + else:
> + indent1 = ""
I don't think this is right. If a type doesn't has an init_val then its
init_val is 0 and you should compare as a boolean. e.g.
if init_val:
s += "if (%s != %s) {\n" % (fexpr, init_val)
else:
s += "if (%s) {\n" % (fexpr)
This gets rid of the special casing of libxl_defbool (where 0 ==
default) as well as removing a bunch of unnecessary p->foo = 0 from the
generated init fns too.
You could also possibly implement this by having get_init_val return an
explicit "0" as a fallback but I think that will resent in less clear
generated code.
> + s += libxl_C_type_gen_map_key(f, nparent, indent1)
> + s += libxl_C_type_gen_json(f.type, fexpr, indent1, nparent)
> +
> + if init_val:
> + 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..14ca165 100644
> --- a/tools/libxl/idl.py
> +++ b/tools/libxl/idl.py
> @@ -142,6 +142,7 @@ class EnumerationValue(object):
> class Enumeration(Type):
> def __init__(self, typename, values, **kwargs):
> kwargs.setdefault('dispose_fn', None)
> + kwargs.setdefault('init_val', '0')
I'm not sure about this, but I think you don't need it for the reasons
above.
> Type.__init__(self, typename, **kwargs)
>
> self.value_namespace = kwargs.setdefault('value_namespace',
> @@ -270,7 +271,7 @@ 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",
> - autogenerate_json = False)
> + autogenerate_json = False, init_val = "NULL")
If you do as I suggest above then you don't need this I think.
>
> -libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
> +libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE,
> + init_val="LIBXL__DEFBOOL_DEFAULT")
Or this.
BTW, I happened to notice that some enums in the IDL define their
(non-zero) default using the integer form, which isn't wrong but did
leap out at me from the generated code. If you fancy fixing that while
you are in the area that would be great. I noticed this on
libxl_action_on_shutdown and libxl_vga_interface_type.
See libxl_timer_mode for how to do it properly. (Maybe idl.py should do
the translation, but lets not go there...)
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 11/18] libxl/gentypes.py: don't generate default values
2014-06-10 13:25 ` Ian Campbell
@ 2014-06-10 13:43 ` Wei Liu
2014-06-10 14:31 ` Wei Liu
2014-06-10 14:56 ` Ian Campbell
0 siblings, 2 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-10 13:43 UTC (permalink / raw)
To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel
On Tue, Jun 10, 2014 at 02:25:14PM +0100, Ian Campbell wrote:
> On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > If a type has init_val defined and a field of the type has the value of
> > init_val, there's no need to generate output for that field in JSON
> > output. When the parser consumes that generated JSON object, all default
> > values should be automatically filled in.
> >
> > Also define init_val for enumeration type, string type and libxl_defbool
> > type.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > tools/libxl/gentypes.py | 27 ++++++++++++++++++++++++---
> > tools/libxl/idl.py | 3 ++-
> > tools/libxl/libxl_types.idl | 3 ++-
> > 3 files changed, 28 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
> > index 01416e7..611c8af 100644
> > --- a/tools/libxl/gentypes.py
> > +++ b/tools/libxl/gentypes.py
> > @@ -135,7 +135,7 @@ def _libxl_C_type_init(ty, v, indent = " ", parent = None, subinit=False):
> > else:
> > s += _libxl_C_type_init(f.type, fexpr, "", nparent)
> > else:
> > - if ty.init_val is not None:
> > + if ty.init_val is not None and ty.typename != "libxl_defbool":
>
> Why is libxl_defbool special here?
>
Because it's an opaque type and I didn't bother to modify the code
generator to have it generate some other functions to return value from
opaque type, as libxl_defbool is the only one that needs extra care so
far.
> > s += "%s = %s;\n" % (ty.pass_arg(v, parent is None), ty.init_val)
> > elif ty.init_fn is not None:
> > s += "%s(%s);\n" % (ty.init_fn, ty.pass_arg(v, parent is None))
> > @@ -206,6 +206,13 @@ 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 libxl_C_type_gen_json(ty, v, indent = " ", parent = None):
> > s = ""
> > if parent is None:
> > @@ -255,8 +262,22 @@ 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)
> > + init_val = get_init_val(f)
> > + if init_val:
> > + if f.type.typename != "libxl_defbool":
> > + s += "if (%s != %s) {\n" % (fexpr, init_val)
> > + else:
> > + s += "if (%s.val != %s) {\n" % (fexpr, init_val)
> > + indent1 = " "
> > + else:
> > + indent1 = ""
>
> I don't think this is right. If a type doesn't has an init_val then its
> init_val is 0 and you should compare as a boolean. e.g.
> if init_val:
> s += "if (%s != %s) {\n" % (fexpr, init_val)
> else:
> s += "if (%s) {\n" % (fexpr)
>
OK, I'm fine with this.
> This gets rid of the special casing of libxl_defbool (where 0 ==
> default) as well as removing a bunch of unnecessary p->foo = 0 from the
> generated init fns too.
>
Not true for libxl_defbool, as stated above, generating code like
if (some_defbool_field)
won't work.
> You could also possibly implement this by having get_init_val return an
> explicit "0" as a fallback but I think that will resent in less clear
> generated code.
>
> > + s += libxl_C_type_gen_map_key(f, nparent, indent1)
> > + s += libxl_C_type_gen_json(f.type, fexpr, indent1, nparent)
> > +
> > + if init_val:
> > + 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..14ca165 100644
> > --- a/tools/libxl/idl.py
> > +++ b/tools/libxl/idl.py
> > @@ -142,6 +142,7 @@ class EnumerationValue(object):
> > class Enumeration(Type):
> > def __init__(self, typename, values, **kwargs):
> > kwargs.setdefault('dispose_fn', None)
> > + kwargs.setdefault('init_val', '0')
>
> I'm not sure about this, but I think you don't need it for the reasons
> above.
>
IIRC with this line Enumeration type doesn't have a default value.
> > Type.__init__(self, typename, **kwargs)
> >
> > self.value_namespace = kwargs.setdefault('value_namespace',
> > @@ -270,7 +271,7 @@ 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",
> > - autogenerate_json = False)
> > + autogenerate_json = False, init_val = "NULL")
>
> If you do as I suggest above then you don't need this I think.
Yes, you're right on this one.
> >
> > -libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
> > +libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE,
> > + init_val="LIBXL__DEFBOOL_DEFAULT")
>
> Or this.
>
I would rather be explicit than implicit. LIBXL__DEFBOOL_DEFAULT happens
to be 0 and that's all. If we ever change our internal implementation
(however unlikely), generator will generate wrong thing. And I bet it
won't be very pleasant to debug this...
> BTW, I happened to notice that some enums in the IDL define their
> (non-zero) default using the integer form, which isn't wrong but did
> leap out at me from the generated code. If you fancy fixing that while
> you are in the area that would be great. I noticed this on
> libxl_action_on_shutdown and libxl_vga_interface_type.
>
I did that in my previous round but you said you wouldn't worry about
readability of generated code (albeit I also modified those "0"s).
So you think it's only necessary to have non-zero default using LIBXL_*
defines? I'm fine with this.
Wei.
> See libxl_timer_mode for how to do it properly. (Maybe idl.py should do
> the translation, but lets not go there...)
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 11/18] libxl/gentypes.py: don't generate default values
2014-06-10 13:43 ` Wei Liu
@ 2014-06-10 14:31 ` Wei Liu
2014-06-10 14:56 ` Ian Campbell
1 sibling, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-10 14:31 UTC (permalink / raw)
To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel
On Tue, Jun 10, 2014 at 02:43:00PM +0100, Wei Liu wrote:
[...]
> >
>
> IIRC with this line Enumeration type doesn't have a default value.
^^ without
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 11/18] libxl/gentypes.py: don't generate default values
2014-06-10 13:43 ` Wei Liu
2014-06-10 14:31 ` Wei Liu
@ 2014-06-10 14:56 ` Ian Campbell
2014-06-10 15:49 ` Wei Liu
1 sibling, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2014-06-10 14:56 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.jackson, xen-devel
On Tue, 2014-06-10 at 14:43 +0100, Wei Liu wrote:
> On Tue, Jun 10, 2014 at 02:25:14PM +0100, Ian Campbell wrote:
> > On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> > > If a type has init_val defined and a field of the type has the value of
> > > init_val, there's no need to generate output for that field in JSON
> > > output. When the parser consumes that generated JSON object, all default
> > > values should be automatically filled in.
> > >
> > > Also define init_val for enumeration type, string type and libxl_defbool
> > > type.
> > >
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > ---
> > > tools/libxl/gentypes.py | 27 ++++++++++++++++++++++++---
> > > tools/libxl/idl.py | 3 ++-
> > > tools/libxl/libxl_types.idl | 3 ++-
> > > 3 files changed, 28 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
> > > index 01416e7..611c8af 100644
> > > --- a/tools/libxl/gentypes.py
> > > +++ b/tools/libxl/gentypes.py
> > > @@ -135,7 +135,7 @@ def _libxl_C_type_init(ty, v, indent = " ", parent = None, subinit=False):
> > > else:
> > > s += _libxl_C_type_init(f.type, fexpr, "", nparent)
> > > else:
> > > - if ty.init_val is not None:
> > > + if ty.init_val is not None and ty.typename != "libxl_defbool":
> >
> > Why is libxl_defbool special here?
> >
>
> Because it's an opaque type and I didn't bother to modify the code
> generator to have it generate some other functions to return value from
> opaque type, as libxl_defbool is the only one that needs extra care so
> far.
Ah, ok. BTW I think "ty != libxl_debool" is valid. Or maybe
isinstance(), whichever one works is better than a string compare I
think.
>
> > > s += "%s = %s;\n" % (ty.pass_arg(v, parent is None), ty.init_val)
> > > elif ty.init_fn is not None:
> > > s += "%s(%s);\n" % (ty.init_fn, ty.pass_arg(v, parent is None))
> > > @@ -206,6 +206,13 @@ 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 libxl_C_type_gen_json(ty, v, indent = " ", parent = None):
> > > s = ""
> > > if parent is None:
> > > @@ -255,8 +262,22 @@ 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)
> > > + init_val = get_init_val(f)
> > > + if init_val:
> > > + if f.type.typename != "libxl_defbool":
> > > + s += "if (%s != %s) {\n" % (fexpr, init_val)
> > > + else:
> > > + s += "if (%s.val != %s) {\n" % (fexpr, init_val)
> > > + indent1 = " "
> > > + else:
> > > + indent1 = ""
> >
> > I don't think this is right. If a type doesn't has an init_val then its
> > init_val is 0 and you should compare as a boolean. e.g.
> > if init_val:
> > s += "if (%s != %s) {\n" % (fexpr, init_val)
> > else:
> > s += "if (%s) {\n" % (fexpr)
> >
>
> OK, I'm fine with this.
>
> > This gets rid of the special casing of libxl_defbool (where 0 ==
> > default) as well as removing a bunch of unnecessary p->foo = 0 from the
> > generated init fns too.
> >
>
> Not true for libxl_defbool, as stated above, generating code like
> if (some_defbool_field)
> won't work.
Oh yes. Ick.
Can you perhaps abstract this into:
def get_field_val(ftype, fexpr):
ftype == libxl_defbool:
return "%s.val" % fexpr
else:
return fexpr
?
>
> > You could also possibly implement this by having get_init_val return an
> > explicit "0" as a fallback but I think that will resent in less clear
> > generated code.
> >
> > > + s += libxl_C_type_gen_map_key(f, nparent, indent1)
> > > + s += libxl_C_type_gen_json(f.type, fexpr, indent1, nparent)
> > > +
> > > + if init_val:
> > > + 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..14ca165 100644
> > > --- a/tools/libxl/idl.py
> > > +++ b/tools/libxl/idl.py
> > > @@ -142,6 +142,7 @@ class EnumerationValue(object):
> > > class Enumeration(Type):
> > > def __init__(self, typename, values, **kwargs):
> > > kwargs.setdefault('dispose_fn', None)
> > > + kwargs.setdefault('init_val', '0')
> >
> > I'm not sure about this, but I think you don't need it for the reasons
> > above.
> >
>
> IIRC with this line Enumeration type doesn't have a default value.
^out (per your other mail)
It should be None, which I think is set by the following Type.__init__.
> > >
> > > -libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
> > > +libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE,
> > > + init_val="LIBXL__DEFBOOL_DEFAULT")
> >
> > Or this.
> >
>
> I would rather be explicit than implicit. LIBXL__DEFBOOL_DEFAULT happens
> to be 0 and that's all. If we ever change our internal implementation
> (however unlikely), generator will generate wrong thing. And I bet it
> won't be very pleasant to debug this...
The assumption that the default bit pattern for any type is all zeroes
is pretty firmly baked into lots of places. It is incredibly unlikely
that this is going to change.
> > BTW, I happened to notice that some enums in the IDL define their
> > (non-zero) default using the integer form, which isn't wrong but did
> > leap out at me from the generated code. If you fancy fixing that while
> > you are in the area that would be great. I noticed this on
> > libxl_action_on_shutdown and libxl_vga_interface_type.
> >
>
> I did that in my previous round but you said you wouldn't worry about
> readability of generated code (albeit I also modified those "0"s).
So I did, I started off mentioning the unlogged
s/1/LIBXL_VGA_INTERFACE_TYPE_CIRRUS/ but then didn't notice that we got
on to talking about 0s too. Sorry.
> So you think it's only necessary to have non-zero default using LIBXL_*
> defines? I'm fine with this.
If they have an init_val at all I think it should use the names not the
numbers. If the default is 0 then it needn't be specified explicitly
(and in that case it'll be if (x) not if (x==THING) in the generated
code)
But no need to fix this unless you want to, it's just a minor wrinkle in
the existing thing.
> Wei.
>
> > See libxl_timer_mode for how to do it properly. (Maybe idl.py should do
> > the translation, but lets not go there...)
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 11/18] libxl/gentypes.py: don't generate default values
2014-06-10 14:56 ` Ian Campbell
@ 2014-06-10 15:49 ` Wei Liu
2014-06-10 15:54 ` Ian Campbell
0 siblings, 1 reply; 48+ messages in thread
From: Wei Liu @ 2014-06-10 15:49 UTC (permalink / raw)
To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel
On Tue, Jun 10, 2014 at 03:56:10PM +0100, Ian Campbell wrote:
[...]
> > > > - if ty.init_val is not None:
> > > > + if ty.init_val is not None and ty.typename != "libxl_defbool":
> > >
> > > Why is libxl_defbool special here?
> > >
> >
> > Because it's an opaque type and I didn't bother to modify the code
> > generator to have it generate some other functions to return value from
> > opaque type, as libxl_defbool is the only one that needs extra care so
> > far.
>
> Ah, ok. BTW I think "ty != libxl_debool" is valid. Or maybe
> isinstance(), whichever one works is better than a string compare I
> think.
>
No problem.
> >
> > > > s += "%s = %s;\n" % (ty.pass_arg(v, parent is None), ty.init_val)
> > > > elif ty.init_fn is not None:
> > > > s += "%s(%s);\n" % (ty.init_fn, ty.pass_arg(v, parent is None))
> > > > @@ -206,6 +206,13 @@ 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 libxl_C_type_gen_json(ty, v, indent = " ", parent = None):
> > > > s = ""
> > > > if parent is None:
> > > > @@ -255,8 +262,22 @@ 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)
> > > > + init_val = get_init_val(f)
> > > > + if init_val:
> > > > + if f.type.typename != "libxl_defbool":
> > > > + s += "if (%s != %s) {\n" % (fexpr, init_val)
> > > > + else:
> > > > + s += "if (%s.val != %s) {\n" % (fexpr, init_val)
> > > > + indent1 = " "
> > > > + else:
> > > > + indent1 = ""
> > >
> > > I don't think this is right. If a type doesn't has an init_val then its
> > > init_val is 0 and you should compare as a boolean. e.g.
> > > if init_val:
> > > s += "if (%s != %s) {\n" % (fexpr, init_val)
> > > else:
> > > s += "if (%s) {\n" % (fexpr)
> > >
> >
> > OK, I'm fine with this.
> >
> > > This gets rid of the special casing of libxl_defbool (where 0 ==
> > > default) as well as removing a bunch of unnecessary p->foo = 0 from the
> > > generated init fns too.
> > >
> >
> > Not true for libxl_defbool, as stated above, generating code like
> > if (some_defbool_field)
> > won't work.
>
> Oh yes. Ick.
>
> Can you perhaps abstract this into:
>
> def get_field_val(ftype, fexpr):
> ftype == libxl_defbool:
> return "%s.val" % fexpr
> else:
> return fexpr
>
> ?
Good idea.
> >
> > > You could also possibly implement this by having get_init_val return an
> > > explicit "0" as a fallback but I think that will resent in less clear
> > > generated code.
> > >
> > > > + s += libxl_C_type_gen_map_key(f, nparent, indent1)
> > > > + s += libxl_C_type_gen_json(f.type, fexpr, indent1, nparent)
> > > > +
> > > > + if init_val:
> > > > + 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..14ca165 100644
> > > > --- a/tools/libxl/idl.py
> > > > +++ b/tools/libxl/idl.py
> > > > @@ -142,6 +142,7 @@ class EnumerationValue(object):
> > > > class Enumeration(Type):
> > > > def __init__(self, typename, values, **kwargs):
> > > > kwargs.setdefault('dispose_fn', None)
> > > > + kwargs.setdefault('init_val', '0')
> > >
> > > I'm not sure about this, but I think you don't need it for the reasons
> > > above.
> > >
> >
> > IIRC with this line Enumeration type doesn't have a default value.
> ^out (per your other mail)
>
> It should be None, which I think is set by the following Type.__init__.
>
Yes, it's None if it's not explicitly set.
If I make get_init_val function return 0 when it encounters None then I
don't need this hunk anymore.
> > > >
> > > > -libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE)
> > > > +libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE,
> > > > + init_val="LIBXL__DEFBOOL_DEFAULT")
> > >
> > > Or this.
> > >
> >
> > I would rather be explicit than implicit. LIBXL__DEFBOOL_DEFAULT happens
> > to be 0 and that's all. If we ever change our internal implementation
> > (however unlikely), generator will generate wrong thing. And I bet it
> > won't be very pleasant to debug this...
>
> The assumption that the default bit pattern for any type is all zeroes
> is pretty firmly baked into lots of places. It is incredibly unlikely
> that this is going to change.
>
Ack.
> > > BTW, I happened to notice that some enums in the IDL define their
> > > (non-zero) default using the integer form, which isn't wrong but did
> > > leap out at me from the generated code. If you fancy fixing that while
> > > you are in the area that would be great. I noticed this on
> > > libxl_action_on_shutdown and libxl_vga_interface_type.
> > >
> >
> > I did that in my previous round but you said you wouldn't worry about
> > readability of generated code (albeit I also modified those "0"s).
>
> So I did, I started off mentioning the unlogged
> s/1/LIBXL_VGA_INTERFACE_TYPE_CIRRUS/ but then didn't notice that we got
> on to talking about 0s too. Sorry.
>
> > So you think it's only necessary to have non-zero default using LIBXL_*
> > defines? I'm fine with this.
>
> If they have an init_val at all I think it should use the names not the
> numbers. If the default is 0 then it needn't be specified explicitly
> (and in that case it'll be if (x) not if (x==THING) in the generated
> code)
>
> But no need to fix this unless you want to, it's just a minor wrinkle in
> the existing thing.
>
This is rather trivial after all. I can fix this in my next version.
Wei.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 11/18] libxl/gentypes.py: don't generate default values
2014-06-10 15:49 ` Wei Liu
@ 2014-06-10 15:54 ` Ian Campbell
2014-06-10 15:59 ` Wei Liu
0 siblings, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2014-06-10 15:54 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.jackson, xen-devel
On Tue, 2014-06-10 at 16:49 +0100, Wei Liu wrote:
> On Tue, Jun 10, 2014 at 03:56:10PM +0100, Ian Campbell wrote:
> [...]
> > > > > - if ty.init_val is not None:
> > > > > + if ty.init_val is not None and ty.typename != "libxl_defbool":
> > > >
> > > > Why is libxl_defbool special here?
> > > >
> > >
> > > Because it's an opaque type and I didn't bother to modify the code
> > > generator to have it generate some other functions to return value from
> > > opaque type, as libxl_defbool is the only one that needs extra care so
> > > far.
> >
> > Ah, ok. BTW I think "ty != libxl_debool" is valid. Or maybe
> > isinstance(), whichever one works is better than a string compare I
> > think.
> >
>
> No problem.
>
> > >
> > > > > s += "%s = %s;\n" % (ty.pass_arg(v, parent is None), ty.init_val)
> > > > > elif ty.init_fn is not None:
> > > > > s += "%s(%s);\n" % (ty.init_fn, ty.pass_arg(v, parent is None))
> > > > > @@ -206,6 +206,13 @@ 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 libxl_C_type_gen_json(ty, v, indent = " ", parent = None):
> > > > > s = ""
> > > > > if parent is None:
> > > > > @@ -255,8 +262,22 @@ 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)
> > > > > + init_val = get_init_val(f)
> > > > > + if init_val:
> > > > > + if f.type.typename != "libxl_defbool":
> > > > > + s += "if (%s != %s) {\n" % (fexpr, init_val)
> > > > > + else:
> > > > > + s += "if (%s.val != %s) {\n" % (fexpr, init_val)
> > > > > + indent1 = " "
> > > > > + else:
> > > > > + indent1 = ""
> > > >
> > > > I don't think this is right. If a type doesn't has an init_val then its
> > > > init_val is 0 and you should compare as a boolean. e.g.
> > > > if init_val:
> > > > s += "if (%s != %s) {\n" % (fexpr, init_val)
> > > > else:
> > > > s += "if (%s) {\n" % (fexpr)
> > > >
> > >
> > > OK, I'm fine with this.
> > >
> > > > This gets rid of the special casing of libxl_defbool (where 0 ==
> > > > default) as well as removing a bunch of unnecessary p->foo = 0 from the
> > > > generated init fns too.
> > > >
> > >
> > > Not true for libxl_defbool, as stated above, generating code like
> > > if (some_defbool_field)
> > > won't work.
> >
> > Oh yes. Ick.
> >
> > Can you perhaps abstract this into:
> >
> > def get_field_val(ftype, fexpr):
> > ftype == libxl_defbool:
> > return "%s.val" % fexpr
> > else:
> > return fexpr
> >
> > ?
>
> Good idea.
>
> > >
> > > > You could also possibly implement this by having get_init_val return an
> > > > explicit "0" as a fallback but I think that will resent in less clear
> > > > generated code.
> > > >
> > > > > + s += libxl_C_type_gen_map_key(f, nparent, indent1)
> > > > > + s += libxl_C_type_gen_json(f.type, fexpr, indent1, nparent)
> > > > > +
> > > > > + if init_val:
> > > > > + 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..14ca165 100644
> > > > > --- a/tools/libxl/idl.py
> > > > > +++ b/tools/libxl/idl.py
> > > > > @@ -142,6 +142,7 @@ class EnumerationValue(object):
> > > > > class Enumeration(Type):
> > > > > def __init__(self, typename, values, **kwargs):
> > > > > kwargs.setdefault('dispose_fn', None)
> > > > > + kwargs.setdefault('init_val', '0')
> > > >
> > > > I'm not sure about this, but I think you don't need it for the reasons
> > > > above.
> > > >
> > >
> > > IIRC with this line Enumeration type doesn't have a default value.
> > ^out (per your other mail)
> >
> > It should be None, which I think is set by the following Type.__init__.
> >
>
> Yes, it's None if it's not explicitly set.
>
> If I make get_init_val function return 0 when it encounters None then I
> don't need this hunk anymore.
I think it should return None in this case and then you use
if init_val:
s += "if (%s != %s) {\n" % (fexpr, init_val)
else:
s += "if (%s) {\n" % (fexpr)
as discussed previously. Or maybe you could make get_init_val return the
complete appropriate expression and call it get_default_expr (or
something better than that)
> > > > BTW, I happened to notice that some enums in the IDL define their
> > > > (non-zero) default using the integer form, which isn't wrong but did
> > > > leap out at me from the generated code. If you fancy fixing that while
> > > > you are in the area that would be great. I noticed this on
> > > > libxl_action_on_shutdown and libxl_vga_interface_type.
> > > >
> > >
> > > I did that in my previous round but you said you wouldn't worry about
> > > readability of generated code (albeit I also modified those "0"s).
> >
> > So I did, I started off mentioning the unlogged
> > s/1/LIBXL_VGA_INTERFACE_TYPE_CIRRUS/ but then didn't notice that we got
> > on to talking about 0s too. Sorry.
> >
> > > So you think it's only necessary to have non-zero default using LIBXL_*
> > > defines? I'm fine with this.
> >
> > If they have an init_val at all I think it should use the names not the
> > numbers. If the default is 0 then it needn't be specified explicitly
> > (and in that case it'll be if (x) not if (x==THING) in the generated
> > code)
> >
> > But no need to fix this unless you want to, it's just a minor wrinkle in
> > the existing thing.
> >
>
> This is rather trivial after all. I can fix this in my next version.
Thanks.
Ian.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 11/18] libxl/gentypes.py: don't generate default values
2014-06-10 15:54 ` Ian Campbell
@ 2014-06-10 15:59 ` Wei Liu
0 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-10 15:59 UTC (permalink / raw)
To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel
On Tue, Jun 10, 2014 at 04:54:26PM +0100, Ian Campbell wrote:
[...]
> > > > IIRC with this line Enumeration type doesn't have a default value.
> > > ^out (per your other mail)
> > >
> > > It should be None, which I think is set by the following Type.__init__.
> > >
> >
> > Yes, it's None if it's not explicitly set.
> >
> > If I make get_init_val function return 0 when it encounters None then I
> > don't need this hunk anymore.
>
> I think it should return None in this case and then you use
> if init_val:
> s += "if (%s != %s) {\n" % (fexpr, init_val)
> else:
> s += "if (%s) {\n" % (fexpr)
>
> as discussed previously. Or maybe you could make get_init_val return the
> complete appropriate expression and call it get_default_expr (or
> something better than that)
>
Oh I was talking nonsense in my previous email. get_init_val is in fact
returning the expression and your suggestion to change it to
get_default_expr is good. Anyway, I will figure this out. Sorry for the
confusion.
Wei.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v6 12/18] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (10 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 11/18] libxl/gentypes.py: don't generate default values Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-10 13:32 ` Ian Campbell
2014-06-09 12:43 ` [PATCH v6 13/18] libxl/gentest.py: test JSON parser Wei Liu
` (6 subsequent siblings)
18 siblings, 1 reply; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 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.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/libxl/gentypes.py | 131 ++++++++++++++++++++++++++++++++++
tools/libxl/idl.py | 15 ++++
tools/libxl/idl.txt | 7 +-
tools/libxl/libxl.h | 14 ++++
tools/libxl/libxl_types.idl | 29 ++++----
tools/libxl/libxl_types_internal.idl | 4 +-
6 files changed, 186 insertions(+), 14 deletions(-)
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index 611c8af..ca49965 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -304,6 +304,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 = %s;\n" % (fexpr, 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
@@ -382,6 +492,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)))
@@ -411,6 +523,10 @@ if __name__ == '__main__':
for ty in [ty for ty in types if ty.json_gen_fn is not None]:
f.write("%syajl_gen_status %s_gen_json(yajl_gen hand, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
+ for ty in [ty for ty in types if ty.json_parse_fn is not None]:
+ f.write("%sint %s_parse_json(libxl__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()
@@ -478,4 +594,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 14ca165..1405764 100644
--- a/tools/libxl/idl.py
+++ b/tools/libxl/idl.py
@@ -66,8 +66,12 @@ class Type(object):
if self.typename is not None and not self.private:
self.json_gen_fn = kwargs.setdefault('json_gen_fn', self.typename + "_gen_json")
+ self.json_parse_type = kwargs.setdefault('json_parse_type', "JSON_ANY")
+ self.json_parse_fn = kwargs.setdefault('json_parse_fn', self.typename + "_parse_json")
else:
self.json_gen_fn = kwargs.setdefault('json_gen_fn', None)
+ self.json_parse_type = kwargs.setdefault('json_parse_type', None)
+ self.json_parse_fn = kwargs.setdefault('json_parse_fn', None)
self.autogenerate_json = kwargs.setdefault('autogenerate_json', True)
@@ -119,6 +123,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)
@@ -126,6 +133,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
@@ -143,6 +151,7 @@ class Enumeration(Type):
def __init__(self, typename, values, **kwargs):
kwargs.setdefault('dispose_fn', None)
kwargs.setdefault('init_val', '0')
+ kwargs.setdefault('json_parse_type', "JSON_STRING")
Type.__init__(self, typename, **kwargs)
self.value_namespace = kwargs.setdefault('value_namespace',
@@ -172,6 +181,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:
@@ -258,6 +268,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)
@@ -271,12 +283,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, init_val = "NULL")
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 6a53dd8..484d5d7 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.
Other simple type-Classes
-------------------------
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index d14f25f..1fad0d2 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_types.idl b/tools/libxl/libxl_types.idl
index d510e2d..ac203bb 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -5,19 +5,24 @@
namespace("libxl_")
-libxl_defbool = Builtin("defbool", passby=PASS_BY_REFERENCE,
+libxl_defbool = Builtin("defbool", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE,
init_val="LIBXL__DEFBOOL_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_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)
+libxl_mac = Builtin("mac", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE)
+libxl_bitmap = Builtin("bitmap", json_parse_type="JSON_ARRAY", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE)
+libxl_cpuid_policy_list = Builtin("cpuid_policy_list", json_parse_type="JSON_ARRAY",
+ dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE)
+
+libxl_string_list = Builtin("string_list", json_parse_type="JSON_ARRAY",
+ dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE)
+libxl_key_value_list = Builtin("key_value_list", json_parse_type="JSON_MAP",
+ dispose_fn="libxl_key_value_list_dispose", passby=PASS_BY_REFERENCE)
+libxl_hwcap = Builtin("hwcap", json_parse_type="JSON_ARRAY", passby=PASS_BY_REFERENCE)
#
# Specific integer types
@@ -583,7 +588,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] 48+ messages in thread* Re: [PATCH v6 12/18] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct
2014-06-09 12:43 ` [PATCH v6 12/18] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct Wei Liu
@ 2014-06-10 13:32 ` Ian Campbell
0 siblings, 0 replies; 48+ messages in thread
From: Ian Campbell @ 2014-06-10 13:32 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.jackson, xen-devel
On Mon, 2014-06-09 at 13:43 +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.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
One small thought:
> + 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
You could do the assign and the declaration on one line.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v6 13/18] libxl/gentest.py: test JSON parser
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (11 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 12/18] libxl IDL: generate code to parse libxl__json_object to libxl_FOO struct Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 14/18] libxl: introduce libxl_key_value_list_length Wei Liu
` (5 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 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] 48+ messages in thread* [PATCH v6 14/18] libxl: introduce libxl_key_value_list_length
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (12 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 13/18] libxl/gentest.py: test JSON parser Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 15/18] libxl: introduce libxl_cpuid_policy_list_length Wei Liu
` (4 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl.c | 13 +++++++++++++
tools/libxl/libxl.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0ce9153..a5ed1e2 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -216,6 +216,19 @@ int libxl_string_list_length(const libxl_string_list *psl)
return i;
}
+int libxl_key_value_list_length(libxl_key_value_list *pkvl)
+{
+ int i = 0;
+ libxl_key_value_list kvl = *pkvl;
+
+ if (kvl) {
+ while (kvl[2 * i]) /* Only checks keys */
+ i++;
+ }
+
+ return i;
+}
+
void libxl_key_value_list_dispose(libxl_key_value_list *pkvl)
{
int i;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 1fad0d2..3e3d913 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -567,6 +567,7 @@ int libxl_string_list_length(const libxl_string_list *sl);
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);
typedef uint32_t libxl_hwcap[8];
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v6 15/18] libxl: introduce libxl_cpuid_policy_list_length
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (13 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 14/18] libxl: introduce libxl_key_value_list_length Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 16/18] libxl: copy function for builtin types Wei Liu
` (3 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, ian.jackson, ian.campbell
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/libxl.h | 1 +
tools/libxl/libxl_cpuid.c | 13 +++++++++++++
2 files changed, 14 insertions(+)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 3e3d913..5f2642b 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -587,6 +587,7 @@ void libxl_bitmap_dispose(libxl_bitmap *map);
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);
#define LIBXL_PCI_FUNC_ALL (~0U)
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index d9007b2..1f2bcd5 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -455,6 +455,19 @@ int libxl_cpuid_policy_list_parse_json(libxl__gc *gc,
return 0;
}
+int libxl_cpuid_policy_list_length(libxl_cpuid_policy_list *pl)
+{
+ int i = 0;
+ libxl_cpuid_policy_list l = *pl;
+
+ if (l) {
+ while (l[i].input[0] != XEN_CPUID_INPUT_UNUSED)
+ i++;
+ }
+
+ return i;
+}
+
/*
* Local variables:
* mode: C
--
1.7.10.4
^ permalink raw reply related [flat|nested] 48+ messages in thread* [PATCH v6 16/18] libxl: copy function for builtin types
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (14 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 15/18] libxl: introduce libxl_cpuid_policy_list_length Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 17/18] libxl IDL: generate deep copy functions Wei Liu
` (2 subsequent siblings)
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 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 | 14 +++++++--
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(+), 2 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a5ed1e2..2a4d710 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;
@@ -5688,6 +5739,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 5f2642b..41d1b18 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -556,20 +556,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;
@@ -588,6 +597,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)
@@ -632,8 +644,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_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] 48+ messages in thread* [PATCH v6 17/18] libxl IDL: generate deep copy functions
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (15 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 16/18] libxl: copy function for builtin types Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-09 12:43 ` [PATCH v6 18/18] libxl/gentest.py: test " Wei Liu
2014-06-10 14:14 ` [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Ian Campbell
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 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 ca49965..5d4ec53 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"""
@@ -481,6 +536,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):
@@ -560,6 +618,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 1405764..1b25ac9 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)
@@ -121,6 +128,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")
@@ -134,6 +142,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
@@ -151,6 +160,7 @@ class Enumeration(Type):
def __init__(self, typename, values, **kwargs):
kwargs.setdefault('dispose_fn', None)
kwargs.setdefault('init_val', '0')
+ kwargs.setdefault('copy_fn', None)
kwargs.setdefault('json_parse_type', "JSON_STRING")
Type.__init__(self, typename, **kwargs)
@@ -267,6 +277,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",
@@ -281,7 +292,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 484d5d7..8b54aeb 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 ac203bb..68a564d 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -5,24 +5,29 @@
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,
init_val="LIBXL__DEFBOOL_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)
-libxl_mac = Builtin("mac", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE)
-libxl_bitmap = Builtin("bitmap", json_parse_type="JSON_ARRAY", dispose_fn="libxl_bitmap_dispose", passby=PASS_BY_REFERENCE)
+ 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, copy_fn="libxl_uuid_copy")
+libxl_mac = Builtin("mac", json_parse_type="JSON_STRING", passby=PASS_BY_REFERENCE, copy_fn="libxl_mac_copy")
+libxl_bitmap = Builtin("bitmap", json_parse_type="JSON_ARRAY", dispose_fn="libxl_bitmap_dispose",
+ copy_fn='libxl_bitmap_copy_alloc', passby=PASS_BY_REFERENCE)
libxl_cpuid_policy_list = Builtin("cpuid_policy_list", json_parse_type="JSON_ARRAY",
- dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE)
+ dispose_fn="libxl_cpuid_dispose", passby=PASS_BY_REFERENCE,
+ copy_fn="libxl_cpuid_policy_list_copy")
libxl_string_list = Builtin("string_list", json_parse_type="JSON_ARRAY",
- dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE)
+ dispose_fn="libxl_string_list_dispose", passby=PASS_BY_REFERENCE,
+ copy_fn="libxl_string_list_copy")
libxl_key_value_list = Builtin("key_value_list", json_parse_type="JSON_MAP",
- dispose_fn="libxl_key_value_list_dispose", passby=PASS_BY_REFERENCE)
-libxl_hwcap = Builtin("hwcap", json_parse_type="JSON_ARRAY", passby=PASS_BY_REFERENCE)
+ dispose_fn="libxl_key_value_list_dispose", passby=PASS_BY_REFERENCE,
+ copy_fn="libxl_key_value_list_copy")
+libxl_hwcap = Builtin("hwcap", json_parse_type="JSON_ARRAY", passby=PASS_BY_REFERENCE,
+ 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] 48+ messages in thread* [PATCH v6 18/18] libxl/gentest.py: test deep copy functions
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (16 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 17/18] libxl IDL: generate deep copy functions Wei Liu
@ 2014-06-09 12:43 ` Wei Liu
2014-06-10 14:14 ` [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Ian Campbell
18 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-09 12:43 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] 48+ messages in thread* Re: [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API
2014-06-09 12:43 [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Wei Liu
` (17 preceding siblings ...)
2014-06-09 12:43 ` [PATCH v6 18/18] libxl/gentest.py: test " Wei Liu
@ 2014-06-10 14:14 ` Ian Campbell
2014-06-10 20:10 ` Boris Ostrovsky
18 siblings, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2014-06-10 14:14 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.jackson, xen-devel
On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> Wei Liu (18):
> A libxl: make cpupool_qualifier_to_cpupoolid a library function
Applied.
> A xl: remove parsing of "vncviewer" option in xl domain config file
Applied.
> M libxl: fix JSON generator for uint64_t
Acked + applied.
> A libxl IDL: rename json_fn to json_gen_fn
> A libxl_json: introduce libxl__object_from_json
> A libxl_json: introduce parser functions for builtin types
Applied all three.
> M libxl/gentypes.py: special-case KeyedUnion map handle generation
Acked + applied.
> C libxl IDL: generate code to parse libxl__json_object to libxl_FOO
> struct
> A libxl/gentest.py: test JSON parser
I skipped reviewing these ones this time around. Will come back to them.
> A libxl: introduce libxl_key_value_list_length
> A libxl: introduce libxl_cpuid_policy_list_length
Applied.
> A libxl: copy function for builtin types
This one didn't apply due to some missing prerequisites. At this point I
stopped trying to apply things because I figured the rest wouldn't apply
either.
Ian.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API
2014-06-10 14:14 ` [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API Ian Campbell
@ 2014-06-10 20:10 ` Boris Ostrovsky
2014-06-10 20:51 ` Boris Ostrovsky
0 siblings, 1 reply; 48+ messages in thread
From: Boris Ostrovsky @ 2014-06-10 20:10 UTC (permalink / raw)
To: Ian Campbell; +Cc: ian.jackson, Wei Liu, xen-devel
On 06/10/2014 10:14 AM, Ian Campbell wrote:
> On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
>> Wei Liu (18):
>> A libxl: make cpupool_qualifier_to_cpupoolid a library function
> Applied.
>> A xl: remove parsing of "vncviewer" option in xl domain config file
> Applied.
>
>> M libxl: fix JSON generator for uint64_t
> Acked + applied.
>
>> A libxl IDL: rename json_fn to json_gen_fn
>> A libxl_json: introduce libxl__object_from_json
>> A libxl_json: introduce parser functions for builtin types
The last one breaks on older gcc:
In file included from flexarray.c:16:
libxl_internal.h:136: error: redefinition of typedef ‘libxl__gc’
libxl_json.h:25: note: previous declaration of ‘libxl__gc’ was here
In file included from flexarray.c:16:
libxl_internal.h:1641: error: redefinition of typedef ‘libxl__json_object’
libxl_json.h:26: note: previous declaration of ‘libxl__json_object’ was here
make[4]: *** [flexarray.o] Error 1
Looks like at some point gcc started allowing multiple typedefs. So, for
example:
FC-64 <build@build-mk2:/tmp> cat foo.c; gcc --version |head -1; gcc foo.c
typedef int foo;
typedef int foo;
main(){}
gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
foo.c:2: error: redefinition of typedef ‘foo’
foo.c:1: note: previous declaration of ‘foo’ was here
FC-64 <build@build-mk2:/tmp>
but
ostr@workbase> cat foo.c; gcc --version |head -1; gcc foo.c
typedef int foo;
typedef int foo;
main(){}
gcc (GCC) 4.7.2 20121109 (Red Hat 4.7.2-8)
ostr@workbase>
-boris
> Applied all three.
>
>> M libxl/gentypes.py: special-case KeyedUnion map handle generation
> Acked + applied.
>
>> C libxl IDL: generate code to parse libxl__json_object to libxl_FOO
>> struct
>> A libxl/gentest.py: test JSON parser
> I skipped reviewing these ones this time around. Will come back to them.
>
>> A libxl: introduce libxl_key_value_list_length
>> A libxl: introduce libxl_cpuid_policy_list_length
> Applied.
>
>> A libxl: copy function for builtin types
> This one didn't apply due to some missing prerequisites. At this point I
> stopped trying to apply things because I figured the rest wouldn't apply
> either.
>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API
2014-06-10 20:10 ` Boris Ostrovsky
@ 2014-06-10 20:51 ` Boris Ostrovsky
2014-06-10 21:21 ` Wei Liu
0 siblings, 1 reply; 48+ messages in thread
From: Boris Ostrovsky @ 2014-06-10 20:51 UTC (permalink / raw)
To: Ian Campbell; +Cc: Wei Liu, ian.jackson, xen-devel
On 06/10/2014 04:10 PM, Boris Ostrovsky wrote:
> On 06/10/2014 10:14 AM, Ian Campbell wrote:
>> On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
>>> Wei Liu (18):
>>> A libxl: make cpupool_qualifier_to_cpupoolid a library function
>> Applied.
>>> A xl: remove parsing of "vncviewer" option in xl domain config file
>> Applied.
>>
>>> M libxl: fix JSON generator for uint64_t
>> Acked + applied.
>>
>>> A libxl IDL: rename json_fn to json_gen_fn
>>> A libxl_json: introduce libxl__object_from_json
>>> A libxl_json: introduce parser functions for builtin types
>
> The last one breaks on older gcc:
>
> In file included from flexarray.c:16:
> libxl_internal.h:136: error: redefinition of typedef ‘libxl__gc’
> libxl_json.h:25: note: previous declaration of ‘libxl__gc’ was here
> In file included from flexarray.c:16:
> libxl_internal.h:1641: error: redefinition of typedef
> ‘libxl__json_object’
> libxl_json.h:26: note: previous declaration of ‘libxl__json_object’
> was here
> make[4]: *** [flexarray.o] Error 1
>
>
> Looks like at some point gcc started allowing multiple typedefs. So,
> for example:
And this is apparently that point (for version 4.6, I believe):
https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=ce3765bf44e49ef0568a1ad4a0b7f807591d6412
-boris
>
>
> FC-64 <build@build-mk2:/tmp> cat foo.c; gcc --version |head -1; gcc foo.c
> typedef int foo;
> typedef int foo;
> main(){}
> gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
> foo.c:2: error: redefinition of typedef ‘foo’
> foo.c:1: note: previous declaration of ‘foo’ was here
> FC-64 <build@build-mk2:/tmp>
>
>
> but
>
> ostr@workbase> cat foo.c; gcc --version |head -1; gcc foo.c
> typedef int foo;
> typedef int foo;
> main(){}
>
> gcc (GCC) 4.7.2 20121109 (Red Hat 4.7.2-8)
> ostr@workbase>
>
>
> -boris
>
>
>> Applied all three.
>>
>>> M libxl/gentypes.py: special-case KeyedUnion map handle generation
>> Acked + applied.
>>
>>> C libxl IDL: generate code to parse libxl__json_object to libxl_FOO
>>> struct
>>> A libxl/gentest.py: test JSON parser
>> I skipped reviewing these ones this time around. Will come back to them.
>>
>>> A libxl: introduce libxl_key_value_list_length
>>> A libxl: introduce libxl_cpuid_policy_list_length
>> Applied.
>>
>>> A libxl: copy function for builtin types
>> This one didn't apply due to some missing prerequisites. At this point I
>> stopped trying to apply things because I figured the rest wouldn't apply
>> either.
>>
>> Ian.
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API
2014-06-10 20:51 ` Boris Ostrovsky
@ 2014-06-10 21:21 ` Wei Liu
2014-06-10 21:38 ` Boris Ostrovsky
2014-06-11 8:47 ` Ian Campbell
0 siblings, 2 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-10 21:21 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: Wei Liu, ian.jackson, Ian Campbell, xen-devel
On Tue, Jun 10, 2014 at 04:51:35PM -0400, Boris Ostrovsky wrote:
> On 06/10/2014 04:10 PM, Boris Ostrovsky wrote:
> >On 06/10/2014 10:14 AM, Ian Campbell wrote:
> >>On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> >>>Wei Liu (18):
> >>> A libxl: make cpupool_qualifier_to_cpupoolid a library function
> >>Applied.
> >>> A xl: remove parsing of "vncviewer" option in xl domain config file
> >>Applied.
> >>
> >>> M libxl: fix JSON generator for uint64_t
> >>Acked + applied.
> >>
> >>> A libxl IDL: rename json_fn to json_gen_fn
> >>> A libxl_json: introduce libxl__object_from_json
> >>> A libxl_json: introduce parser functions for builtin types
> >
> >The last one breaks on older gcc:
> >
> >In file included from flexarray.c:16:
> >libxl_internal.h:136: error: redefinition of typedef ‘libxl__gc’
> >libxl_json.h:25: note: previous declaration of ‘libxl__gc’ was here
> >In file included from flexarray.c:16:
> >libxl_internal.h:1641: error: redefinition of typedef ‘libxl__json_object’
> >libxl_json.h:26: note: previous declaration of ‘libxl__json_object’ was
> >here
> >make[4]: *** [flexarray.o] Error 1
> >
> >
> >Looks like at some point gcc started allowing multiple typedefs. So, for
> >example:
>
> And this is apparently that point (for version 4.6, I believe):
>
> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=ce3765bf44e49ef0568a1ad4a0b7f807591d6412
>
>
> -boris
Thanks for reporting. Now that I notice these functions really belong to
libxl_internal.h.
Does this patch fix it for you?
Wei.
---8<---
From 5a57f827f644f716b3748beec0214bbefddda4b6 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Tue, 10 Jun 2014 22:16:14 +0100
Subject: [PATCH] libxl: move some internal functions to libxl_internal.h
In 752f181f ("libxl_json: introduce parser functions for builtin types")
a bunch of parser functions are added to libxl_json.h, which breaks
GCC < 4.6.
These functions are internal and libxl_json.h is public header, so move
them to libxl_internal.h.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
tools/libxl/libxl_internal.h | 32 ++++++++++++++++++++++++++++++++
tools/libxl/libxl_json.h | 35 -----------------------------------
2 files changed, 32 insertions(+), 35 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 80ea883..a0d4f24 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3147,6 +3147,38 @@ void libxl__numa_candidate_put_nodemap(libxl__gc *gc,
*/
#define CTYPE(isfoo,c) (isfoo((unsigned char)(c)))
+int libxl_defbool_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_defbool *p);
+int libxl__bool_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ bool *p);
+int libxl_mac_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_mac *p);
+int libxl_bitmap_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_bitmap *p);
+int libxl_uuid_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_uuid *p);
+int libxl_cpuid_policy_list_parse_json(libxl__gc *gc,
+ const libxl__json_object *o,
+ libxl_cpuid_policy_list *p);
+int libxl_string_list_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_string_list *p);
+int libxl_key_value_list_parse_json(libxl__gc *gc,
+ const libxl__json_object *o,
+ libxl_key_value_list *p);
+int libxl_hwcap_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ libxl_hwcap *p);
+int libxl__int_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p);
+int libxl__uint8_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p);
+int libxl__uint16_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p);
+int libxl__uint32_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p);
+int libxl__uint64_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ void *p);
+int libxl__string_parse_json(libxl__gc *gc, const libxl__json_object *o,
+ char **p);
#endif
diff --git a/tools/libxl/libxl_json.h b/tools/libxl/libxl_json.h
index b196c1c..e4c0f6c 100644
--- a/tools/libxl/libxl_json.h
+++ b/tools/libxl/libxl_json.h
@@ -22,52 +22,17 @@
# include <yajl/yajl_version.h>
#endif
-typedef struct libxl__gc libxl__gc;
-typedef struct libxl__json_object libxl__json_object;
-
yajl_gen_status libxl__uint64_gen_json(yajl_gen hand, uint64_t val);
yajl_gen_status libxl_defbool_gen_json(yajl_gen hand, libxl_defbool *p);
-int libxl_defbool_parse_json(libxl__gc *gc, const libxl__json_object *o,
- libxl_defbool *p);
-int libxl__bool_parse_json(libxl__gc *gc, const libxl__json_object *o,
- bool *p);
yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, libxl_uuid *p);
-int libxl_uuid_parse_json(libxl__gc *gc, const libxl__json_object *o,
- libxl_uuid *p);
yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *p);
-int libxl_mac_parse_json(libxl__gc *gc, const libxl__json_object *o,
- libxl_mac *p);
yajl_gen_status libxl_bitmap_gen_json(yajl_gen hand, libxl_bitmap *p);
-int libxl_bitmap_parse_json(libxl__gc *gc, const libxl__json_object *o,
- libxl_bitmap *p);
yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
libxl_cpuid_policy_list *p);
-int libxl_cpuid_policy_list_parse_json(libxl__gc *gc,
- const libxl__json_object *o,
- libxl_cpuid_policy_list *p);
yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *p);
-int libxl_string_list_parse_json(libxl__gc *gc, const libxl__json_object *o,
- libxl_string_list *p);
yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
libxl_key_value_list *p);
-int libxl_key_value_list_parse_json(libxl__gc *gc,
- const libxl__json_object *o,
- libxl_key_value_list *p);
yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand, libxl_hwcap *p);
-int libxl_hwcap_parse_json(libxl__gc *gc, const libxl__json_object *o,
- libxl_hwcap *p);
-int libxl__int_parse_json(libxl__gc *gc, const libxl__json_object *o,
- void *p);
-int libxl__uint8_parse_json(libxl__gc *gc, const libxl__json_object *o,
- void *p);
-int libxl__uint16_parse_json(libxl__gc *gc, const libxl__json_object *o,
- void *p);
-int libxl__uint32_parse_json(libxl__gc *gc, const libxl__json_object *o,
- void *p);
-int libxl__uint64_parse_json(libxl__gc *gc, const libxl__json_object *o,
- void *p);
-int libxl__string_parse_json(libxl__gc *gc, const libxl__json_object *o,
- char **p);
#include <_libxl_types_json.h>
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 48+ messages in thread* Re: [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API
2014-06-10 21:21 ` Wei Liu
@ 2014-06-10 21:38 ` Boris Ostrovsky
2014-06-10 21:42 ` Wei Liu
2014-06-11 8:47 ` Ian Campbell
1 sibling, 1 reply; 48+ messages in thread
From: Boris Ostrovsky @ 2014-06-10 21:38 UTC (permalink / raw)
To: Wei Liu; +Cc: ian.jackson, Ian Campbell, xen-devel
On 06/10/2014 05:21 PM, Wei Liu wrote:
> On Tue, Jun 10, 2014 at 04:51:35PM -0400, Boris Ostrovsky wrote:
>> On 06/10/2014 04:10 PM, Boris Ostrovsky wrote:
>>> On 06/10/2014 10:14 AM, Ian Campbell wrote:
>>>> On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
>>>>> Wei Liu (18):
>>>>> A libxl: make cpupool_qualifier_to_cpupoolid a library function
>>>> Applied.
>>>>> A xl: remove parsing of "vncviewer" option in xl domain config file
>>>> Applied.
>>>>
>>>>> M libxl: fix JSON generator for uint64_t
>>>> Acked + applied.
>>>>
>>>>> A libxl IDL: rename json_fn to json_gen_fn
>>>>> A libxl_json: introduce libxl__object_from_json
>>>>> A libxl_json: introduce parser functions for builtin types
>>> The last one breaks on older gcc:
>>>
>>> In file included from flexarray.c:16:
>>> libxl_internal.h:136: error: redefinition of typedef ‘libxl__gc’
>>> libxl_json.h:25: note: previous declaration of ‘libxl__gc’ was here
>>> In file included from flexarray.c:16:
>>> libxl_internal.h:1641: error: redefinition of typedef ‘libxl__json_object’
>>> libxl_json.h:26: note: previous declaration of ‘libxl__json_object’ was
>>> here
>>> make[4]: *** [flexarray.o] Error 1
>>>
>>>
>>> Looks like at some point gcc started allowing multiple typedefs. So, for
>>> example:
>> And this is apparently that point (for version 4.6, I believe):
>>
>> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=ce3765bf44e49ef0568a1ad4a0b7f807591d6412
>>
>>
>> -boris
> Thanks for reporting. Now that I notice these functions really belong to
> libxl_internal.h.
>
> Does this patch fix it for you?
Yes, it allows me to build libxl. I can't test it right now but will do
it tomorrow.
Thanks.
-boris
>
> Wei.
>
> ---8<---
> From 5a57f827f644f716b3748beec0214bbefddda4b6 Mon Sep 17 00:00:00 2001
> From: Wei Liu <wei.liu2@citrix.com>
> Date: Tue, 10 Jun 2014 22:16:14 +0100
> Subject: [PATCH] libxl: move some internal functions to libxl_internal.h
>
> In 752f181f ("libxl_json: introduce parser functions for builtin types")
> a bunch of parser functions are added to libxl_json.h, which breaks
> GCC < 4.6.
>
> These functions are internal and libxl_json.h is public header, so move
> them to libxl_internal.h.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> tools/libxl/libxl_internal.h | 32 ++++++++++++++++++++++++++++++++
> tools/libxl/libxl_json.h | 35 -----------------------------------
> 2 files changed, 32 insertions(+), 35 deletions(-)
>
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 80ea883..a0d4f24 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3147,6 +3147,38 @@ void libxl__numa_candidate_put_nodemap(libxl__gc *gc,
> */
> #define CTYPE(isfoo,c) (isfoo((unsigned char)(c)))
>
> +int libxl_defbool_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + libxl_defbool *p);
> +int libxl__bool_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + bool *p);
> +int libxl_mac_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + libxl_mac *p);
> +int libxl_bitmap_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + libxl_bitmap *p);
> +int libxl_uuid_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + libxl_uuid *p);
> +int libxl_cpuid_policy_list_parse_json(libxl__gc *gc,
> + const libxl__json_object *o,
> + libxl_cpuid_policy_list *p);
> +int libxl_string_list_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + libxl_string_list *p);
> +int libxl_key_value_list_parse_json(libxl__gc *gc,
> + const libxl__json_object *o,
> + libxl_key_value_list *p);
> +int libxl_hwcap_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + libxl_hwcap *p);
> +int libxl__int_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + void *p);
> +int libxl__uint8_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + void *p);
> +int libxl__uint16_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + void *p);
> +int libxl__uint32_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + void *p);
> +int libxl__uint64_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + void *p);
> +int libxl__string_parse_json(libxl__gc *gc, const libxl__json_object *o,
> + char **p);
>
> #endif
>
> diff --git a/tools/libxl/libxl_json.h b/tools/libxl/libxl_json.h
> index b196c1c..e4c0f6c 100644
> --- a/tools/libxl/libxl_json.h
> +++ b/tools/libxl/libxl_json.h
> @@ -22,52 +22,17 @@
> # include <yajl/yajl_version.h>
> #endif
>
> -typedef struct libxl__gc libxl__gc;
> -typedef struct libxl__json_object libxl__json_object;
> -
> yajl_gen_status libxl__uint64_gen_json(yajl_gen hand, uint64_t val);
> yajl_gen_status libxl_defbool_gen_json(yajl_gen hand, libxl_defbool *p);
> -int libxl_defbool_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - libxl_defbool *p);
> -int libxl__bool_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - bool *p);
> yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, libxl_uuid *p);
> -int libxl_uuid_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - libxl_uuid *p);
> yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *p);
> -int libxl_mac_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - libxl_mac *p);
> yajl_gen_status libxl_bitmap_gen_json(yajl_gen hand, libxl_bitmap *p);
> -int libxl_bitmap_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - libxl_bitmap *p);
> yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
> libxl_cpuid_policy_list *p);
> -int libxl_cpuid_policy_list_parse_json(libxl__gc *gc,
> - const libxl__json_object *o,
> - libxl_cpuid_policy_list *p);
> yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *p);
> -int libxl_string_list_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - libxl_string_list *p);
> yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
> libxl_key_value_list *p);
> -int libxl_key_value_list_parse_json(libxl__gc *gc,
> - const libxl__json_object *o,
> - libxl_key_value_list *p);
> yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand, libxl_hwcap *p);
> -int libxl_hwcap_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - libxl_hwcap *p);
> -int libxl__int_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - void *p);
> -int libxl__uint8_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - void *p);
> -int libxl__uint16_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - void *p);
> -int libxl__uint32_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - void *p);
> -int libxl__uint64_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - void *p);
> -int libxl__string_parse_json(libxl__gc *gc, const libxl__json_object *o,
> - char **p);
>
> #include <_libxl_types_json.h>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API
2014-06-10 21:38 ` Boris Ostrovsky
@ 2014-06-10 21:42 ` Wei Liu
0 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-10 21:42 UTC (permalink / raw)
To: Boris Ostrovsky; +Cc: ian.jackson, Wei Liu, Ian Campbell, xen-devel
On Tue, Jun 10, 2014 at 05:38:13PM -0400, Boris Ostrovsky wrote:
> On 06/10/2014 05:21 PM, Wei Liu wrote:
> >On Tue, Jun 10, 2014 at 04:51:35PM -0400, Boris Ostrovsky wrote:
> >>On 06/10/2014 04:10 PM, Boris Ostrovsky wrote:
> >>>On 06/10/2014 10:14 AM, Ian Campbell wrote:
> >>>>On Mon, 2014-06-09 at 13:43 +0100, Wei Liu wrote:
> >>>>>Wei Liu (18):
> >>>>> A libxl: make cpupool_qualifier_to_cpupoolid a library function
> >>>>Applied.
> >>>>> A xl: remove parsing of "vncviewer" option in xl domain config file
> >>>>Applied.
> >>>>
> >>>>> M libxl: fix JSON generator for uint64_t
> >>>>Acked + applied.
> >>>>
> >>>>> A libxl IDL: rename json_fn to json_gen_fn
> >>>>> A libxl_json: introduce libxl__object_from_json
> >>>>> A libxl_json: introduce parser functions for builtin types
> >>>The last one breaks on older gcc:
> >>>
> >>>In file included from flexarray.c:16:
> >>>libxl_internal.h:136: error: redefinition of typedef ‘libxl__gc’
> >>>libxl_json.h:25: note: previous declaration of ‘libxl__gc’ was here
> >>>In file included from flexarray.c:16:
> >>>libxl_internal.h:1641: error: redefinition of typedef ‘libxl__json_object’
> >>>libxl_json.h:26: note: previous declaration of ‘libxl__json_object’ was
> >>>here
> >>>make[4]: *** [flexarray.o] Error 1
> >>>
> >>>
> >>>Looks like at some point gcc started allowing multiple typedefs. So, for
> >>>example:
> >>And this is apparently that point (for version 4.6, I believe):
> >>
> >>https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=ce3765bf44e49ef0568a1ad4a0b7f807591d6412
> >>
> >>
> >>-boris
> >Thanks for reporting. Now that I notice these functions really belong to
> >libxl_internal.h.
> >
> >Does this patch fix it for you?
>
> Yes, it allows me to build libxl. I can't test it right now but will do it
> tomorrow.
>
FWIW I tested this patch with gcc 4.4 it worked. The core bit is it
avoids having duplicate libxl__gc and libxl__json_object.
If you're thinking about testing new functionalities then there's
probably nothing to test at the moment. The user of this infrastructure
is just not yet in tree. :-)
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API
2014-06-10 21:21 ` Wei Liu
2014-06-10 21:38 ` Boris Ostrovsky
@ 2014-06-11 8:47 ` Ian Campbell
2014-06-11 8:53 ` Wei Liu
1 sibling, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2014-06-11 8:47 UTC (permalink / raw)
To: Wei Liu; +Cc: Boris Ostrovsky, ian.jackson, xen-devel
On Tue, 2014-06-10 at 22:21 +0100, Wei Liu wrote:
> ---8<---
> From 5a57f827f644f716b3748beec0214bbefddda4b6 Mon Sep 17 00:00:00 2001
> From: Wei Liu <wei.liu2@citrix.com>
> Date: Tue, 10 Jun 2014 22:16:14 +0100
> Subject: [PATCH] libxl: move some internal functions to libxl_internal.h
>
> In 752f181f ("libxl_json: introduce parser functions for builtin types")
> a bunch of parser functions are added to libxl_json.h, which breaks
> GCC < 4.6.
>
> These functions are internal and libxl_json.h is public header, so move
> them to libxl_internal.h.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked + applied.
FYI there is a bunch of existing internal (ish) stuff in there which I
was planning to move to (new) libxl_json_internal.h at some point. Due
to: http://lists.xen.org/archives/html/xen-devel/2014-06/msg00069.html
Ian.
^ permalink raw reply [flat|nested] 48+ messages in thread* Re: [PATCH v6 00/18] libxl: JSON infrastructure, fixes and prerequisite patches for new API
2014-06-11 8:47 ` Ian Campbell
@ 2014-06-11 8:53 ` Wei Liu
0 siblings, 0 replies; 48+ messages in thread
From: Wei Liu @ 2014-06-11 8:53 UTC (permalink / raw)
To: Ian Campbell; +Cc: ian.jackson, Boris Ostrovsky, Wei Liu, xen-devel
On Wed, Jun 11, 2014 at 09:47:06AM +0100, Ian Campbell wrote:
>
> On Tue, 2014-06-10 at 22:21 +0100, Wei Liu wrote:
>
> > ---8<---
> > From 5a57f827f644f716b3748beec0214bbefddda4b6 Mon Sep 17 00:00:00 2001
> > From: Wei Liu <wei.liu2@citrix.com>
> > Date: Tue, 10 Jun 2014 22:16:14 +0100
> > Subject: [PATCH] libxl: move some internal functions to libxl_internal.h
> >
> > In 752f181f ("libxl_json: introduce parser functions for builtin types")
> > a bunch of parser functions are added to libxl_json.h, which breaks
> > GCC < 4.6.
> >
> > These functions are internal and libxl_json.h is public header, so move
> > them to libxl_internal.h.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>
> Acked + applied.
>
> FYI there is a bunch of existing internal (ish) stuff in there which I
> was planning to move to (new) libxl_json_internal.h at some point. Due
> to: http://lists.xen.org/archives/html/xen-devel/2014-06/msg00069.html
>
I was thinking about the same thing at some point last night but I
didn't want to stay up any longer than necessary. :-P
Wei.
> Ian.
>
>
^ permalink raw reply [flat|nested] 48+ messages in thread