The following series implements libxl_domain_config_init as per the libxl API requirement that each type has an init function. The first function does this in an open coded manner and is proposed for Xen 4.2. The second function is RFC only since it moves the definition of this type into the IDL and makes the required infrastructure updates to enable this. I think this is more 4.3 material at this stage.
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1333533071 -3600
# Node ID ac6f863df8f8c86dcc58df15f94333e6088e0bf4
# Parent d00faeaf21dd280500d2deace00683d884a2dc10
libxl: provide libxl_domain_config_init.
Currently this struct is too complicated for the IDL to represent (arrays) so
for now implement by hand.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r d00faeaf21dd -r ac6f863df8f8 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Wed Apr 04 10:49:08 2012 +0100
+++ b/tools/libxl/libxl.h Wed Apr 04 10:51:11 2012 +0100
@@ -462,6 +462,7 @@ int libxl_ctx_postfork(libxl_ctx *ctx);
typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
libxl_console_ready cb, void *priv, uint32_t *domid);
int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);
+void libxl_domain_config_init(libxl_domain_config *d_config);
void libxl_domain_config_dispose(libxl_domain_config *d_config);
int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
uint32_t domid, int fd);
diff -r d00faeaf21dd -r ac6f863df8f8 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Wed Apr 04 10:49:08 2012 +0100
+++ b/tools/libxl/libxl_create.c Wed Apr 04 10:51:11 2012 +0100
@@ -22,6 +22,13 @@
#include <xc_dom.h>
#include <xenguest.h>
+void libxl_domain_config_init(libxl_domain_config *d_config)
+{
+ memset(d_config, 0, sizeof(*d_config));
+ libxl_domain_create_info_init(&d_config->c_info);
+ libxl_domain_build_info_init(&d_config->b_info);
+}
+
void libxl_domain_config_dispose(libxl_domain_config *d_config)
{
int i;
diff -r d00faeaf21dd -r ac6f863df8f8 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Wed Apr 04 10:49:08 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Wed Apr 04 10:51:11 2012 +0100
@@ -535,8 +535,6 @@ static void parse_config_data(const char
exit(1);
}
- libxl_domain_create_info_init(c_info);
-
if (!xlu_cfg_get_string (config, "seclabel", &buf, 0)) {
e = libxl_flask_context_to_sid(ctx, (char *)buf, strlen(buf),
&c_info->ssidref);
@@ -582,7 +580,6 @@ static void parse_config_data(const char
exit(1);
}
- libxl_domain_build_info_init(b_info);
libxl_domain_build_info_init_type(b_info, c_info->type);
/* the following is the actual config parsing with overriding values in the
structures */
@@ -1505,7 +1502,7 @@ static int create_domain(struct domain_c
pid_t child_console_pid = -1;
struct save_file_header hdr;
- memset(&d_config, 0x00, sizeof(d_config));
+ libxl_domain_config_init(&d_config);
if (restore_file) {
uint8_t *optdata_begin = 0;
@@ -1822,7 +1819,7 @@ start:
/* Reparse the configuration in case it has changed */
libxl_domain_config_dispose(&d_config);
- memset(&d_config, 0, sizeof(d_config));
+ libxl_domain_config_init(&d_config);
parse_config_data(config_file, config_data, config_len,
&d_config);
Ian Campbell
2012-Apr-04 10:36 UTC
[PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1333535720 -3600
# Node ID dc3241cf1ed1b8e5709cc71c9ec8a93b2374cbd5
# Parent ac6f863df8f8c86dcc58df15f94333e6088e0bf4
RFC: libxl: move definition of libxl_domain_config into the IDL
This requires adding a new Array type to the IDL.
DO NOT APPLY. This is 4.3 material.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/libxl/gentest.py
--- a/tools/libxl/gentest.py Wed Apr 04 10:51:11 2012 +0100
+++ b/tools/libxl/gentest.py Wed Apr 04 11:35:20 2012 +0100
@@ -28,6 +28,18 @@ def gen_rand_init(ty, v, indent = " "
s = ""
if isinstance(ty, idl.Enumeration):
s += "%s = %s;\n" % (ty.pass_arg(v, parent is None),
randomize_enum(ty))
+ elif isinstance(ty, idl.Array):
+ if parent is None:
+ raise Exception("Array type must have a parent")
+ s += "%s = rand()%%8;\n" % (parent + ty.lenvar.name)
+ s += "%s = calloc(%s, sizeof(*%s));\n" % \
+ (v, parent + ty.lenvar.name, v)
+ s += "{\n"
+ s += " int i;\n"
+ s += " for (i=0; i<%s; i++)\n" % (parent +
ty.lenvar.name)
+ s += gen_rand_init(ty.elem_type, v+"[i]",
+ indent + " ", parent)
+ s += "}\n"
elif isinstance(ty, idl.KeyedUnion):
if parent is None:
raise Exception("KeyedUnion type must have a parent")
diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/libxl/gentypes.py
--- a/tools/libxl/gentypes.py Wed Apr 04 10:51:11 2012 +0100
+++ b/tools/libxl/gentypes.py Wed Apr 04 11:35:20 2012 +0100
@@ -11,8 +11,12 @@ def libxl_C_instance_of(ty, instancename
return libxl_C_type_define(ty)
else:
return libxl_C_type_define(ty) + " " + instancename
- else:
- return ty.typename + " " + instancename
+
+ s = ""
+ if isinstance(ty, idl.Array):
+ s += libxl_C_instance_of(ty.lenvar.type, ty.lenvar.name) +
";\n"
+
+ return s + ty.typename + " " + instancename
def libxl_C_type_define(ty, indent = ""):
s = ""
@@ -66,6 +70,17 @@ def libxl_C_type_dispose(ty, v, indent s +=
libxl_C_type_dispose(f.type, fexpr, indent + " ", nparent)
s += " break;\n"
s += "}\n"
+ elif isinstance(ty, idl.Array):
+ if parent is None:
+ raise Exception("Array type must have a parent")
+ s += "{\n"
+ s += " int i;\n"
+ s += " for (i=0; i<%s; i++)\n" % (parent +
ty.lenvar.name)
+ s += libxl_C_type_dispose(ty.elem_type, v+"[i]",
+ indent + " ", parent)
+ if ty.dispose_fn is not None:
+ s += " %s(%s);\n" % (ty.dispose_fn, ty.pass_arg(v,
parent is None))
+ s += "}\n"
elif isinstance(ty, idl.Struct) and (parent is None or ty.dispose_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)
@@ -164,7 +179,24 @@ def libxl_C_type_gen_json(ty, v, indent
s = ""
if parent is None:
s += "yajl_gen_status s;\n"
- if isinstance(ty, idl.Enumeration):
+
+ if isinstance(ty, idl.Array):
+ if parent is None:
+ raise Exception("Array type must have a parent")
+ s += "{\n"
+ s += " int i;\n"
+ s += " s = yajl_gen_array_open(hand);\n"
+ s += " if (s != yajl_gen_status_ok)\n"
+ s += " goto out;\n"
+ s += " for (i=0; i<%s; i++) {\n" % (parent +
ty.lenvar.name)
+ s += libxl_C_type_gen_json(ty.elem_type, v+"[i]",
+ indent + " ", parent)
+ s += " }\n"
+ s += " s = yajl_gen_array_close(hand);\n"
+ s += " if (s != yajl_gen_status_ok)\n"
+ s += " goto out;\n"
+ s += "}\n"
+ elif isinstance(ty, idl.Enumeration):
s += "s = libxl__yajl_gen_enum(hand, %s_to_string(%s));\n" %
(ty.typename, ty.pass_arg(v, parent is None))
s += "if (s != yajl_gen_status_ok)\n"
s += " goto out;\n"
diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/libxl/idl.py
--- a/tools/libxl/idl.py Wed Apr 04 10:51:11 2012 +0100
+++ b/tools/libxl/idl.py Wed Apr 04 11:35:20 2012 +0100
@@ -251,6 +251,17 @@ string = Builtin("char *", namespace = N
json_fn = "libxl__string_gen_json",
autogenerate_json = False)
+class Array(Type):
+ """An array of the same type"""
+ def __init__(self, elem_type, lenvar_name, **kwargs):
+ kwargs.setdefault(''dispose_fn'',
''free'')
+ Type.__init__(self, typename=elem_type.rawname + " *",
**kwargs)
+
+ lv_kwargs = dict([(x.lstrip(''lenvar_''),y) for (x,y)
in kwargs.items() if x.startswith(''lenvar_'')])
+
+ self.lenvar = Field(integer, lenvar_name, **lv_kwargs)
+ self.elem_type = elem_type
+
class OrderedDict(dict):
"""A dictionary which remembers insertion order.
diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/libxl/idl.txt
--- a/tools/libxl/idl.txt Wed Apr 04 10:51:11 2012 +0100
+++ b/tools/libxl/idl.txt Wed Apr 04 11:35:20 2012 +0100
@@ -145,12 +145,24 @@ idl.KeyedUnion
A subclass of idl.Aggregate which represents the C union type
where the currently valid member of the union can be determined based
- upon another member in the containing type.
+ upon another member in the containing type. An idl.KeyedUnion must
+ always be a member of a containing idl.Aggregate type.
- The KeyedUnion.keyvar contains an idl.type the member of the
+ The KeyedUnion.keyvar contains an idl.Type the member of the
containing type which determines the valid member of the union. The
must be an instance of the Enumeration type.
+idl.Array
+
+ A class representing an array or similar elements. An idl.Array must
+ always be an idl.Field of a containing idl.Aggregate.
+
+ idl.Array.elem_type contains an idl.Type which is the type of all
+ elements of the array.
+
+ idl.Array.len_var contains an idl.Field which is added to the parent
+ idl.Aggregate and will contain the length of the array.
+
Standard Types
--------------
diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Wed Apr 04 10:51:11 2012 +0100
+++ b/tools/libxl/libxl.h Wed Apr 04 11:35:20 2012 +0100
@@ -432,25 +432,6 @@ typedef struct {
#define LIBXL_VERSION 0
-typedef struct {
- libxl_domain_create_info c_info;
- libxl_domain_build_info b_info;
-
- int num_disks, num_vifs, num_pcidevs, num_vfbs, num_vkbs;
-
- libxl_device_disk *disks;
- libxl_device_nic *vifs;
- libxl_device_pci *pcidevs;
- libxl_device_vfb *vfbs;
- libxl_device_vkb *vkbs;
-
- libxl_action_on_shutdown on_poweroff;
- libxl_action_on_shutdown on_reboot;
- libxl_action_on_shutdown on_watchdog;
- libxl_action_on_shutdown on_crash;
-} libxl_domain_config;
-char *libxl_domain_config_to_json(libxl_ctx *ctx, libxl_domain_config *p);
-
/* context functions */
int libxl_ctx_alloc(libxl_ctx **pctx, int version,
unsigned flags /* none currently defined */,
@@ -462,8 +443,6 @@ int libxl_ctx_postfork(libxl_ctx *ctx);
typedef int (*libxl_console_ready)(libxl_ctx *ctx, uint32_t domid, void *priv);
int libxl_domain_create_new(libxl_ctx *ctx, libxl_domain_config *d_config,
libxl_console_ready cb, void *priv, uint32_t *domid);
int libxl_domain_create_restore(libxl_ctx *ctx, libxl_domain_config *d_config,
libxl_console_ready cb, void *priv, uint32_t *domid, int restore_fd);
-void libxl_domain_config_init(libxl_domain_config *d_config);
-void libxl_domain_config_dispose(libxl_domain_config *d_config);
int libxl_domain_suspend(libxl_ctx *ctx, libxl_domain_suspend_info *info,
uint32_t domid, int fd);
int libxl_domain_resume(libxl_ctx *ctx, uint32_t domid);
diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Wed Apr 04 10:51:11 2012 +0100
+++ b/tools/libxl/libxl_create.c Wed Apr 04 11:35:20 2012 +0100
@@ -22,41 +22,6 @@
#include <xc_dom.h>
#include <xenguest.h>
-void libxl_domain_config_init(libxl_domain_config *d_config)
-{
- memset(d_config, 0, sizeof(*d_config));
- libxl_domain_create_info_init(&d_config->c_info);
- libxl_domain_build_info_init(&d_config->b_info);
-}
-
-void libxl_domain_config_dispose(libxl_domain_config *d_config)
-{
- int i;
-
- for (i=0; i<d_config->num_disks; i++)
- libxl_device_disk_dispose(&d_config->disks[i]);
- free(d_config->disks);
-
- for (i=0; i<d_config->num_vifs; i++)
- libxl_device_nic_dispose(&d_config->vifs[i]);
- free(d_config->vifs);
-
- for (i=0; i<d_config->num_pcidevs; i++)
- libxl_device_pci_dispose(&d_config->pcidevs[i]);
- free(d_config->pcidevs);
-
- for (i=0; i<d_config->num_vfbs; i++)
- libxl_device_vfb_dispose(&d_config->vfbs[i]);
- free(d_config->vfbs);
-
- for (i=0; i<d_config->num_vkbs; i++)
- libxl_device_vkb_dispose(&d_config->vkbs[i]);
- free(d_config->vkbs);
-
- libxl_domain_create_info_dispose(&d_config->c_info);
- libxl_domain_build_info_dispose(&d_config->b_info);
-}
-
int libxl__domain_create_info_setdefault(libxl__gc *gc,
libxl_domain_create_info *c_info)
{
diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/libxl/libxl_json.c
--- a/tools/libxl/libxl_json.c Wed Apr 04 10:51:11 2012 +0100
+++ b/tools/libxl/libxl_json.c Wed Apr 04 11:35:20 2012 +0100
@@ -849,158 +849,6 @@ out:
return ret;
}
-yajl_gen_status libxl_domain_config_gen_json(yajl_gen hand,
- libxl_domain_config *p)
-{
- yajl_gen_status s;
- int i;
-
- s = yajl_gen_map_open(hand);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"c_info",
- sizeof("c_info")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = libxl_domain_create_info_gen_json(hand, &p->c_info);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"b_info",
- sizeof("b_info")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = libxl_domain_build_info_gen_json(hand, &p->b_info);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"disks",
- sizeof("disks")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = yajl_gen_array_open(hand);
- if (s != yajl_gen_status_ok)
- goto out;
- for (i = 0; i < p->num_disks; i++) {
- s = libxl_device_disk_gen_json(hand, &p->disks[i]);
- if (s != yajl_gen_status_ok)
- goto out;
- }
- s = yajl_gen_array_close(hand);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"vifs",
- sizeof("vifs")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = yajl_gen_array_open(hand);
- if (s != yajl_gen_status_ok)
- goto out;
- for (i = 0; i < p->num_vifs; i++) {
- s = libxl_device_nic_gen_json(hand, &p->vifs[i]);
- if (s != yajl_gen_status_ok)
- goto out;
- }
- s = yajl_gen_array_close(hand);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"pcidevs",
- sizeof("pcidevs")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = yajl_gen_array_open(hand);
- if (s != yajl_gen_status_ok)
- goto out;
- for (i = 0; i < p->num_pcidevs; i++) {
- s = libxl_device_pci_gen_json(hand, &p->pcidevs[i]);
- if (s != yajl_gen_status_ok)
- goto out;
- }
- s = yajl_gen_array_close(hand);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"vfbs",
- sizeof("vfbs")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = yajl_gen_array_open(hand);
- if (s != yajl_gen_status_ok)
- goto out;
- for (i = 0; i < p->num_vfbs; i++) {
- s = libxl_device_vfb_gen_json(hand, &p->vfbs[i]);
- if (s != yajl_gen_status_ok)
- goto out;
- }
- s = yajl_gen_array_close(hand);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"vkbs",
- sizeof("vkbs")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = yajl_gen_array_open(hand);
- if (s != yajl_gen_status_ok)
- goto out;
- for (i = 0; i < p->num_vkbs; i++) {
- s = libxl_device_vkb_gen_json(hand, &p->vkbs[i]);
- if (s != yajl_gen_status_ok)
- goto out;
- }
- s = yajl_gen_array_close(hand);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"on_poweroff",
- sizeof("on_poweroff")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = libxl_action_on_shutdown_gen_json(hand, &p->on_poweroff);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"on_reboot",
- sizeof("on_reboot")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = libxl_action_on_shutdown_gen_json(hand, &p->on_reboot);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"on_watchdog",
- sizeof("on_watchdog")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = libxl_action_on_shutdown_gen_json(hand, &p->on_watchdog);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_string(hand, (const unsigned char *)"on_crash",
- sizeof("on_crash")-1);
- if (s != yajl_gen_status_ok)
- goto out;
- s = libxl_action_on_shutdown_gen_json(hand, &p->on_crash);
- if (s != yajl_gen_status_ok)
- goto out;
-
- s = yajl_gen_map_close(hand);
- if (s != yajl_gen_status_ok)
- goto out;
- out:
- return s;
-}
-
-char *libxl_domain_config_to_json(libxl_ctx *ctx, libxl_domain_config *p)
-{
- return libxl__object_to_json(ctx, "libxl_domain_config",
-
(libxl__gen_json_callback)&libxl_domain_config_gen_json,
- (void *)p);
-}
-
/*
* Local variables:
* mode: C
diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Wed Apr 04 10:51:11 2012 +0100
+++ b/tools/libxl/libxl_types.idl Wed Apr 04 11:35:20 2012 +0100
@@ -356,6 +356,22 @@ libxl_device_pci = Struct("device_pci",
("power_mgmt", bool),
])
+libxl_domain_config = Struct("domain_config", [
+ ("c_info", libxl_domain_create_info),
+ ("b_info", libxl_domain_build_info),
+
+ ("disks", Array(libxl_device_disk, "num_disks")),
+ ("vifs", Array(libxl_device_nic, "num_vifs")),
+ ("pcidevs", Array(libxl_device_pci, "num_pcidevs")),
+ ("vfbs", Array(libxl_device_vfb, "num_vfbs")),
+ ("vkbs", Array(libxl_device_vkb, "num_vkbs")),
+
+ ("on_poweroff", libxl_action_on_shutdown),
+ ("on_reboot", libxl_action_on_shutdown),
+ ("on_watchdog", libxl_action_on_shutdown),
+ ("on_crash", libxl_action_on_shutdown),
+ ])
+
libxl_diskinfo = Struct("diskinfo", [
("backend", string),
("backend_id", uint32),
diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/ocaml/libs/xl/genwrap.py
--- a/tools/ocaml/libs/xl/genwrap.py Wed Apr 04 10:51:11 2012 +0100
+++ b/tools/ocaml/libs/xl/genwrap.py Wed Apr 04 11:35:20 2012 +0100
@@ -54,7 +54,8 @@ def ocaml_type_of(ty):
return "int%d" % ty.width
else:
return "int"
-
+ elif isinstance(ty,idl.Array):
+ return "%s list" % ocaml_type_of(ty.elem_type)
elif isinstance(ty,idl.Builtin):
if not builtins.has_key(ty.typename):
raise NotImplementedError("Unknown Builtin %s (%s)" %
(ty.typename, type(ty)))
@@ -268,6 +269,7 @@ if __name__ == ''__main__'':
"cpupoolinfo",
"domain_create_info",
"domain_build_info",
+ "domain_config",
"vcpuinfo",
"event",
]
diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/python/genwrap.py
--- a/tools/python/genwrap.py Wed Apr 04 10:51:11 2012 +0100
+++ b/tools/python/genwrap.py Wed Apr 04 11:35:20 2012 +0100
@@ -4,7 +4,7 @@ import sys,os
import idl
-(TYPE_DEFBOOL, TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING, TYPE_AGGREGATE) =
range(6)
+(TYPE_DEFBOOL, TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING, TYPE_ARRAY,
TYPE_AGGREGATE) = range(7)
def py_type(ty):
if ty == idl.bool:
@@ -18,6 +18,8 @@ def py_type(ty):
return TYPE_INT
else:
return TYPE_UINT
+ if isinstance(ty, idl.Array):
+ return TYPE_ARRAY
if isinstance(ty, idl.Aggregate):
return TYPE_AGGREGATE
if ty == idl.string:
@@ -74,7 +76,7 @@ def py_attrib_get(ty, f):
l.append('' return
genwrap__ull_get(self->obj.%s);''%f.name)
elif t == TYPE_STRING:
l.append('' return
genwrap__string_get(&self->obj.%s);''%f.name)
- elif t == TYPE_AGGREGATE:
+ elif t == TYPE_AGGREGATE or t == TYPE_ARRAY:
l.append('' PyErr_SetString(PyExc_NotImplementedError,
"Getting %s");''%ty.typename)
l.append('' return NULL;'')
else:
@@ -105,7 +107,7 @@ def py_attrib_set(ty, f):
l.append('' return ret;'')
elif t == TYPE_STRING:
l.append('' return genwrap__string_set(v,
&self->obj.%s);''%f.name)
- elif t == TYPE_AGGREGATE:
+ elif t == TYPE_AGGREGATE or t == TYPE_ARRAY:
l.append('' PyErr_SetString(PyExc_NotImplementedError,
"Setting %s");''%ty.typename)
l.append('' return -1;'')
else:
Ian Jackson
2012-Apr-24 17:57 UTC
Re: [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL
Ian Campbell writes ("[Xen-devel] [PATCH 2 of 2] RFC: libxl: move
definition of libxl_domain_config into the IDL"):> RFC: libxl: move definition of libxl_domain_config into the IDL
>
> This requires adding a new Array type to the IDL.
>
> DO NOT APPLY. This is 4.3 material.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> + idl.Array.len_var contains an idl.Field which is added to the parent
> + idl.Aggregate and will contain the length of the array.
Why does the Array not automatically invent a "num_<foo>" field
?
Surely there is no benefit to having non-systematically named (or
typed) array count fields ?
Ian.
Ian Jackson
2012-Apr-24 17:58 UTC
Re: [PATCH 1 of 2] libxl: provide libxl_domain_config_init
Ian Campbell writes ("[Xen-devel] [PATCH 1 of 2] libxl: provide
libxl_domain_config_init"):> libxl: provide libxl_domain_config_init.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Campbell
2012-Apr-25 08:11 UTC
Re: [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL
On Tue, 2012-04-24 at 18:57 +0100, Ian Jackson wrote:> Ian Campbell writes ("[Xen-devel] [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL"): > > RFC: libxl: move definition of libxl_domain_config into the IDL > > > > This requires adding a new Array type to the IDL. > > > > DO NOT APPLY. This is 4.3 material. > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > > + idl.Array.len_var contains an idl.Field which is added to the parent > > + idl.Aggregate and will contain the length of the array. > > Why does the Array not automatically invent a "num_<foo>" field ? > Surely there is no benefit to having non-systematically named (or > typed) array count fields ?That would be good but currently the Array type doesn''t see the name of the member in the containing struct: Struct("thing", [ ("disks", Array(libxl_device_disk, "num_disks")), ]) We have a similar problem with the KeyedUnion which similarly ought to be able to at least default keyvar_name to something. Ian.
Ian Campbell
2012-Apr-25 08:57 UTC
Re: [PATCH 1 of 2] libxl: provide libxl_domain_config_init
On Tue, 2012-04-24 at 18:58 +0100, Ian Jackson wrote:> Ian Campbell writes ("[Xen-devel] [PATCH 1 of 2] libxl: provide libxl_domain_config_init"): > > libxl: provide libxl_domain_config_init. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>Oops, I had an update which I''d forgotten to send out -- this is the delta, Sorry. 8<---------------------------------------------------- # HG changeset patch # User Ian Campbell <ian.campbell@citrix.com> # Date 1335344197 -3600 # Node ID bc54fd8d21b4bd50dc365905aff2f2eac7ac0807 # Parent b27dc6eecfe794fe1bed6d1188a71853818d77e9 libxl: use libxl_domain_config_init and not memset 0 I missed a couple of memsets in 25237:31489be80c51, we need to use libxl_domain_config_init everywhere and not memset since not all fields are initialised to zero now (the type field in particular). This fixes an abort with "xl list <dom>" for a specific domain due to assert(type == -1) in libxl_domain_build_info_init_type(). Signed-off-by: Ian Campbell <ian.campbell@citrix.com> diff -r b27dc6eecfe7 -r bc54fd8d21b4 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Wed Apr 25 09:54:39 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Wed Apr 25 09:56:37 2012 +0100 @@ -2464,7 +2464,7 @@ static void list_domains_details(const l if (rc) continue; CHK_ERRNO(asprintf(&config_file, "<domid %d data>", info[i].domid)); - memset(&d_config, 0x00, sizeof(d_config)); + libxl_domain_config_init(&d_config); parse_config_data(config_file, (char *)data, len, &d_config); printf_info(default_output_format, info[i].domid, &d_config); libxl_domain_config_dispose(&d_config); @@ -3546,7 +3546,7 @@ int main_config_update(int argc, char ** exit(1); } - memset(&d_config, 0x00, sizeof(d_config)); + libxl_domain_config_init(&d_config); parse_config_data(filename, config_data, config_len, &d_config);
Ian Jackson
2012-Apr-25 10:25 UTC
Re: [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL
Ian Campbell writes ("Re: [Xen-devel] [PATCH 2 of 2] RFC: libxl: move
definition of libxl_domain_config into the IDL"):> On Tue, 2012-04-24 at 18:57 +0100, Ian Jackson wrote:
...> > Why does the Array not automatically invent a
"num_<foo>" field ?
> > Surely there is no benefit to having non-systematically named (or
> > typed) array count fields ?
>
> That would be good but currently the Array type doesn''t see the
name of
> the member in the containing struct:
> Struct("thing", [
> ("disks", Array(libxl_device_disk, "num_disks")),
> ])
>
> We have a similar problem with the KeyedUnion which similarly ought to
> be able to at least default keyvar_name to something.
Ah. Can we make it an absolutely hard policy rule that the name _is_
some fixed variant on the name of the pointer member ? That way we
will be able to autogenerate it later without trouble.
The transformation should probably not involve adding "s" because then
people will be tempted to sometimes add "es" (or alternatively we have
to live with misspelled plurals).
Ian.
Ian Campbell
2012-Apr-25 10:28 UTC
Re: [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL
On Wed, 2012-04-25 at 11:25 +0100, Ian Jackson wrote:> Ian Campbell writes ("Re: [Xen-devel] [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL"): > > On Tue, 2012-04-24 at 18:57 +0100, Ian Jackson wrote: > ... > > > Why does the Array not automatically invent a "num_<foo>" field ? > > > Surely there is no benefit to having non-systematically named (or > > > typed) array count fields ? > > > > That would be good but currently the Array type doesn''t see the name of > > the member in the containing struct: > > Struct("thing", [ > > ("disks", Array(libxl_device_disk, "num_disks")), > > ]) > > > > We have a similar problem with the KeyedUnion which similarly ought to > > be able to at least default keyvar_name to something. > > Ah. Can we make it an absolutely hard policy rule that the name _is_ > some fixed variant on the name of the pointer member ? That way we > will be able to autogenerate it later without trouble.That seems reasonable.> The transformation should probably not involve adding "s" because then > people will be tempted to sometimes add "es" (or alternatively we have > to live with misspelled plurals).as does this. Ian.
Ian Jackson
2012-Apr-25 10:36 UTC
Re: [PATCH 1 of 2] libxl: provide libxl_domain_config_init
Ian Campbell writes ("Re: [Xen-devel] [PATCH 1 of 2] libxl: provide
libxl_domain_config_init"):> On Tue, 2012-04-24 at 18:58 +0100, Ian Jackson wrote:
> > Ian Campbell writes ("[Xen-devel] [PATCH 1 of 2] libxl: provide
libxl_domain_config_init"):
> > > libxl: provide libxl_domain_config_init.
> >
> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> > Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
>
> Oops, I had an update which I''d forgotten to send out -- this is
the
> delta, Sorry.
Ooops.
> libxl: use libxl_domain_config_init and not memset 0
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
> I missed a couple of memsets in 25237:31489be80c51, we need to use
> libxl_domain_config_init everywhere and not memset since not all fields are
> initialised to zero now (the type field in particular). This fixes an abort
> with "xl list <dom>" for a specific domain due to
assert(type == -1) in
> libxl_domain_build_info_init_type().
This will probably turn up as an automatic test failure.
Ian.
Dario Faggioli
2012-May-18 15:49 UTC
Re: [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL
On Wed, 2012-04-04 at 11:36 +0100, Ian Campbell wrote:> # HG changeset patch > # User Ian Campbell <ian.campbell@citrix.com> > # Date 1333535720 -3600 > # Node ID dc3241cf1ed1b8e5709cc71c9ec8a93b2374cbd5 > # Parent ac6f863df8f8c86dcc58df15f94333e6088e0bf4 > RFC: libxl: move definition of libxl_domain_config into the IDL > > This requires adding a new Array type to the IDL. >I''ve finally decided to take node distances into account for my NUMA series for 4.2, so I''m trying to use this patch for it (just the IDL array part, I''m leaving libxl_domain_config out... although that can of course be included as well if we want). Therefore, I''ll include this very own patch as a part of my series (almost ready, will post next week) with the following proposed modifications:> Signed-off-by: Ian Campbell <ian.campbell@citrix.com> >Tested-by: Dario Faggioli <dario.faggioli@citrix.com>> diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/libxl/gentypes.py > --- a/tools/libxl/gentypes.py Wed Apr 04 10:51:11 2012 +0100 > +++ b/tools/libxl/gentypes.py Wed Apr 04 11:35:20 2012 +0100 > > ... > > @@ -66,6 +70,17 @@ def libxl_C_type_dispose(ty, v, indent > s += libxl_C_type_dispose(f.type, fexpr, indent + " ", nparent) > s += " break;\n" > s += "}\n" > + elif isinstance(ty, idl.Array): > + if parent is None: > + raise Exception("Array type must have a parent") > + s += "{\n" > + s += " int i;\n" > + s += " for (i=0; i<%s; i++)\n" % (parent + ty.lenvar.name) > + s += libxl_C_type_dispose(ty.elem_type, v+"[i]", > + indent + " ", parent) > + if ty.dispose_fn is not None: > + s += " %s(%s);\n" % (ty.dispose_fn, ty.pass_arg(v, parent is None)) > + s += "}\n" >if ty.elem_type.dispose_fn is not None: s += " int i;\n" s += " for (i=0; i<%s; i++)\n" % (parent + ty.lenvar.name) s += libxl_C_type_dispose(ty.elem_type, v+"[i]", Otherwise I get something like the below, when creating an array of, say, uint32_t-s: int i; for (i=0; i<p->num_dists; i++) free(p->dists); Instead of just the free() part, which is what I need in this case.> diff -r ac6f863df8f8 -r dc3241cf1ed1 tools/libxl/idl.py > --- a/tools/libxl/idl.py Wed Apr 04 10:51:11 2012 +0100 > +++ b/tools/libxl/idl.py Wed Apr 04 11:35:20 2012 +0100 > @@ -251,6 +251,17 @@ string = Builtin("char *", namespace = N > json_fn = "libxl__string_gen_json", > autogenerate_json = False) > > +class Array(Type): > + """An array of the same type""" > + def __init__(self, elem_type, lenvar_name, **kwargs): > + kwargs.setdefault(''dispose_fn'', ''free'') > + Type.__init__(self, typename=elem_type.rawname + " *", **kwargs) >Type.__init__(self, namespace=elem_type.namespace, typename=elem_type.rawname + " *", **kwargs) As suggested by you (IanC) on IRC, to avoid getting stuff like `libxl_uint32_t'' and alike. Does that make sense? Regards, Dario -- <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel