[PATCH 0 of 2] libxl: add libxl_domain_config_init

[PATCH 0 of 2] libxl: add libxl_domain_config_init

[Ian Campbell]

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.

[PATCH 1 of 2] libxl: provide libxl_domain_config_init

[Ian Campbell]

# 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);

Re: [PATCH 1 of 2] libxl: provide libxl_domain_config_init

[Ian Jackson]

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>

Re: [PATCH 1 of 2] libxl: provide libxl_domain_config_init

[Ian Campbell]

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);

Re: [PATCH 1 of 2] libxl: provide libxl_domain_config_init

[Ian Jackson]

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.

[PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL

[Ian Campbell]

# 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:

Re: [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL

[Ian Jackson]

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.

Re: [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL

[Ian Campbell]

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.

Re: [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL

[Ian Jackson]

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.

Re: [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL

[Ian Campbell]

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.

Re: [PATCH 2 of 2] RFC: libxl: move definition of libxl_domain_config into the IDL

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 3452 bytes --]

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)


[-- 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