libxl python binding updates

This patch-series brings the libxl python binding up to the state where it can create domains. It also includes several bug-fixes and exporting of additional libxl functionality such as PCI passthrough.

The changes are all contained within the tools/python tree except for changes to the libxl IDL and associated processing code. The final patch in the series exports libxl_domain_config structure via the IDL and removes the redundant (identical) definitions and declarations from libxl.

Gianni

 tools/python/genwrap.py           |   5 ++++-
 tools/python/xen/lowlevel/xl/xl.c |  12 +++++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

# HG changeset patch
# User Gianni Tedesco <[email protected]> # Date 1294761665 0
# Node ID 9ae3901ab42fd0458cf6150014e1f376a25493b5 # Parent 719ba64edad1d7226cd927bcfa65fff087544995
pyxl: Fix reference counting of Py_(None|True|False)

The incorrect refcounting causes the python interpreter to crash

diff -r 719ba64edad1 -r 9ae3901ab42f tools/python/genwrap.py
@@ -51,7 +51,10 @@ def py_attrib_get(ty, f):
     l.append('static PyObject *py_%s_%s_get(Py_%s *self, void *priv)'%(ty.rawname, f.name, ty.rawname))
     l.append('{')
     if t == TYPE_BOOL:
-        l.append('    return (self->obj.%s) ? Py_True : Py_False;'%f.name)
+        l.append('    PyObject *ret;')
+        l.append('    ret = (self->obj.%s) ? Py_True : Py_False;'%f.name)
+        l.append('    Py_INCREF(ret);')
+        l.append('    return ret;')
     elif t == TYPE_INT:
         l.append('    return genwrap__ll_get(self->obj.%s);'%f.name)
     elif t == TYPE_UINT:

diff -r 719ba64edad1 -r 9ae3901ab42f tools/python/xen/lowlevel/xl/xl.c
@@ -97,8 +97,10 @@ int genwrap__string_set(PyObject *v, cha
 
 PyObject *genwrap__string_get(char **str)
 {
-    if ( NULL == *str )
+    if ( NULL == *str ) {
+        Py_INCREF(Py_None);
         return Py_None;
+    }
     return PyString_FromString(*str);
 }
 

@@ -377,6 +379,7 @@ static PyObject *pyxl_list_domains(XlObj
         if ( NULL == di )
             goto err_mem;
         memcpy(&di->obj, cur, sizeof(di->obj));
+        /* SetItem steals a reference */
         PyList_SetItem(list, i, (PyObject *)di);
     }
 

@@ -413,6 +416,7 @@ static PyObject *pyxl_domain_shutdown(Xl
         PyErr_SetString(xl_error_obj, "cannot shutdown domain");
         return NULL;
     }
+    Py_INCREF(Py_None);
     return Py_None;
 }
 

@@ -425,6 +429,7 @@ static PyObject *pyxl_domain_destroy(XlO
         PyErr_SetString(xl_error_obj, "cannot destroy domain");
         return NULL;
     }
+    Py_INCREF(Py_None);
     return Py_None;
 }
 

@@ -437,6 +442,7 @@ static PyObject *pyxl_domain_pause(XlObj
         PyErr_SetString(xl_error_obj, "cannot pause domain");
         return NULL;
     }
+    Py_INCREF(Py_None);
     return Py_None;
 }
 

@@ -449,6 +455,7 @@ static PyObject *pyxl_domain_unpause(XlO
         PyErr_SetString(xl_error_obj, "cannot unpause domain");
         return NULL;
     }
+    Py_INCREF(Py_None);
     return Py_None;
 }
 

@@ -462,6 +469,7 @@ static PyObject *pyxl_domain_rename(XlOb
         PyErr_SetString(xl_error_obj, "cannot rename domain");
         return NULL;
     }
+    Py_INCREF(Py_None);
     return Py_None;
 }
 

@@ -481,6 +489,7 @@ static PyObject *pyxl_pci_add(XlObject *
         PyErr_SetString(xl_error_obj, "cannot add pci device");
         return NULL;
     }
+    Py_INCREF(Py_None);
     return Py_None;
 }
 

@@ -501,6 +510,7 @@ static PyObject *pyxl_pci_del(XlObject *
         PyErr_SetString(xl_error_obj, "cannot remove pci device");
         return NULL;
     }
+    Py_INCREF(Py_None);
     return Py_None;
 }

 tools/python/xen/lowlevel/xl/xl.c |  4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

# HG changeset patch
# User Gianni Tedesco <[email protected]> # Date 1294761666 0
# Node ID b06b8e4174e1660b775bfee70f132542afe0a62e # Parent 9ae3901ab42fd0458cf6150014e1f376a25493b5
pyxl: Un-muddle libxl_domain_(pause|unpause) wrappers

diff -r 9ae3901ab42f -r b06b8e4174e1 tools/python/xen/lowlevel/xl/xl.c
@@ -546,9 +546,9 @@ static PyMethodDef pyxl_methods[] = {
          "Shutdown a domain"},
     {"domain_destroy", (PyCFunction)pyxl_domain_destroy, METH_VARARGS,
          "Destroy a domain"},
-    {"domain_pause", (PyCFunction)pyxl_domain_unpause, METH_VARARGS,
+    {"domain_pause", (PyCFunction)pyxl_domain_pause, METH_VARARGS,
          "Pause a domain"},
-    {"domain_unpause", (PyCFunction)pyxl_domain_pause, METH_VARARGS,
+    {"domain_unpause", (PyCFunction)pyxl_domain_unpause, METH_VARARGS,
          "Unpause a domain"},
     {"domain_rename", (PyCFunction)pyxl_domain_rename, METH_VARARGS,
          "Rename a domain"},

 tools/python/xen/lowlevel/xl/xl.c |  32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

# HG changeset patch
# User Gianni Tedesco <[email protected]> # Date 1294761666 0
# Node ID 26fc86ef80278a10cd9954d7ce7e4221f5015baf # Parent b06b8e4174e1660b775bfee70f132542afe0a62e
pyxl: Export relevant integer constants from python wrapper

diff -r b06b8e4174e1 -r 26fc86ef8027 tools/python/xen/lowlevel/xl/xl.c
@@ -651,6 +651,8 @@ static PyTypeObject PyXlType = {
 
 static PyMethodDef xl_methods[] = { { NULL } };
 
+#define  _INT_CONST(m, c) PyModule_AddIntConstant(m, #c, c)
+#define  _INT_CONST_LIBXL(m, c) PyModule_AddIntConstant(m, #c, LIBXL_ ## c)
 PyMODINIT_FUNC initxl(void)
 {
     PyObject *m;

@@ -671,6 +673,36 @@ PyMODINIT_FUNC initxl(void)
     Py_INCREF(xl_error_obj);
     PyModule_AddObject(m, "Error", xl_error_obj);
 
+    _INT_CONST(m, SHUTDOWN_poweroff);
+    _INT_CONST(m, SHUTDOWN_reboot);
+    _INT_CONST(m, SHUTDOWN_suspend);
+    _INT_CONST(m, SHUTDOWN_crash);
+    _INT_CONST(m, SHUTDOWN_watchdog);
+
+    _INT_CONST(m, XENFV);
+    _INT_CONST(m, XENPV);
+
+    _INT_CONST_LIBXL(m, CONSTYPE_SERIAL);
+    _INT_CONST_LIBXL(m, CONSTYPE_PV);
+
+    _INT_CONST_LIBXL(m, CONSBACK_XENCONSOLED);
+    _INT_CONST_LIBXL(m, CONSBACK_IOEMU);
+
+    _INT_CONST(m, PHYSTYPE_QCOW);
+    _INT_CONST(m, PHYSTYPE_QCOW2);
+    _INT_CONST(m, PHYSTYPE_VHD);
+    _INT_CONST(m, PHYSTYPE_AIO);
+    _INT_CONST(m, PHYSTYPE_FILE);
+    _INT_CONST(m, PHYSTYPE_PHY);
+
+    _INT_CONST(m, NICTYPE_IOEMU);
+    _INT_CONST(m, NICTYPE_VIF);
+
+    _INT_CONST_LIBXL(m, EVENT_DOMAIN_DEATH);
+    _INT_CONST_LIBXL(m, EVENT_DISK_EJECT);
+
+    _INT_CONST(m, POWER_BUTTON);
+    _INT_CONST(m, SLEEP_BUTTON);
     genwrap__init(m);
 }

 tools/python/genwrap.py           |  2 +-
 tools/python/xen/lowlevel/xl/xl.c |  2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

# HG changeset patch
# User Gianni Tedesco <[email protected]> # Date 1294761667 0
# Node ID 791046eea8ded39ad09f1cf4b8482580e2a42e6a # Parent 26fc86ef80278a10cd9954d7ce7e4221f5015baf
pyxl: Allow subclassing of auto-generated python types

diff -r 26fc86ef8027 -r 791046eea8de tools/python/genwrap.py
@@ -151,7 +151,7 @@ static PyTypeObject Py%s_Type= {
     NULL,                         /* tp_getattro       */
     NULL,                         /* tp_setattro       */
     NULL,                         /* tp_as_buffer      */
-    Py_TPFLAGS_DEFAULT,           /* tp_flags          */
+    Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE, /* tp_flags          */
     "%s",                         /* tp_doc            */
     NULL,                         /* tp_traverse       */
     NULL,                         /* tp_clear          */

diff -r 26fc86ef8027 -r 791046eea8de tools/python/xen/lowlevel/xl/xl.c
@@ -628,7 +628,7 @@ static PyTypeObject PyXlType = {
     NULL,                         /* tp_getattro       */
     NULL,                         /* tp_setattro       */
     NULL,                         /* tp_as_buffer      */
-    Py_TPFLAGS_DEFAULT,           /* tp_flags          */
+    Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE, /* tp_flags          */
     "libxenlight connection",     /* tp_doc            */
     NULL,                         /* tp_traverse       */
     NULL,                         /* tp_clear          */

 tools/python/xen/lowlevel/xl/xl.c |  74 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 74 insertions(+), 0 deletions(-)

# HG changeset patch
# User Gianni Tedesco <[email protected]> # Date 1294761667 0
# Node ID fb64e8d377d84f99b0a0bac3d7f04dd62fea25b9 # Parent 791046eea8ded39ad09f1cf4b8482580e2a42e6a
pyxl: Export PCI passthrough related libxl functions

diff -r 791046eea8de -r fb64e8d377d8 tools/python/xen/lowlevel/xl/xl.c
@@ -537,6 +537,75 @@ static PyObject *pyxl_pci_parse(XlObject
     return (PyObject *)pci;
 }
 
+static PyObject *pyxl_pci_list_assignable(XlObject *self, PyObject *args)
+{
+    libxl_device_pci *dev;
+    PyObject *list;
+    int nr_dev, i;
+
+    if ( libxl_device_pci_list_assignable(&self->ctx, &dev, &nr_dev) ) {
+        PyErr_SetString(xl_error_obj, "Cannot list assignable devices");
+        return NULL;
+    }
+
+    list = PyList_New(nr_dev);
+    if ( NULL == list )
+        return NULL;
+
+    for(i = 0; i < nr_dev; i++) {
+        Py_device_pci *pd;
+        pd = Pydevice_pci_New();
+        if ( NULL == pd )
+            goto err_mem;
+        memcpy(&pd->obj, &dev[i], sizeof(pd->obj));
+        /* SetItem steals a reference */
+        PyList_SetItem(list, i, (PyObject *)pd);
+    }
+
+    free(dev);
+    return list;
+err_mem:
+    Py_DECREF(list);
+    PyErr_SetString(PyExc_MemoryError, "Allocating PCI device list");
+    return NULL;
+}
+
+static PyObject *pyxl_pci_list(XlObject *self, PyObject *args)
+{
+    libxl_device_pci *dev;
+    PyObject *list;
+    int nr_dev, i, domid;
+
+    if ( !PyArg_ParseTuple(args, "i", &domid) )
+        return NULL;
+
+    if ( libxl_device_pci_list_assigned(&self->ctx, &dev, domid, &nr_dev) ) {
+        PyErr_SetString(xl_error_obj, "Cannot list assignable devices");
+        return NULL;
+    }
+
+    list = PyList_New(nr_dev);
+    if ( NULL == list )
+        return NULL;
+
+    for(i = 0; i < nr_dev; i++) {
+        Py_device_pci *pd;
+        pd = Pydevice_pci_New();
+        if ( NULL == pd )
+            goto err_mem;
+        memcpy(&pd->obj, &dev[i], sizeof(pd->obj));
+        /* SetItem steals a reference */
+        PyList_SetItem(list, i, (PyObject *)pd);
+    }
+
+    free(dev);
+    return list;
+err_mem:
+    Py_DECREF(list);
+    PyErr_SetString(PyExc_MemoryError, "Allocating PCI device list");
+    return NULL;
+}
+
 static PyMethodDef pyxl_methods[] = {
     {"list_domains", (PyCFunction)pyxl_list_domains, METH_NOARGS,
          "List domains"},

@@ -558,6 +627,11 @@ static PyMethodDef pyxl_methods[] = {
          "Remove a pass-through PCI device"},
     {"device_pci_parse_bdf", (PyCFunction)pyxl_pci_parse, METH_VARARGS,
          "Parse pass-through PCI device spec (BDF)"},
+    {"device_pci_list", (PyCFunction)pyxl_pci_list, METH_VARARGS,
+        "List PCI devices assigned to a domain"},
+    {"device_pci_list_assignable",
+        (PyCFunction)pyxl_pci_list_assignable, METH_NOARGS,
+        "List assignable PCI devices"},
     { NULL, NULL, 0, NULL }
 };

 tools/python/xen/lowlevel/xl/xl.c |  23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

# HG changeset patch
# User Gianni Tedesco <[email protected]> # Date 1294761667 0
# Node ID f0cb6ba7a4cf52b63cca5595a68568657976d57d # Parent fb64e8d377d84f99b0a0bac3d7f04dd62fea25b9
pyxl: Updates to builtin-type marshalling functions

Allow setting a string field to None as a way to zero it out. Implement setting/getting libx_file_references as strings. Produce relevant Exceptions marshallers which remain unimplemented.

diff -r fb64e8d377d8 -r f0cb6ba7a4cf tools/python/xen/lowlevel/xl/xl.c
@@ -76,7 +76,7 @@ int genwrap__obj_init(PyObject *self, Py
 int genwrap__string_set(PyObject *v, char **str)
 {
     char *tmp;
-    if ( NULL == v ) {
+    if ( NULL == v || Py_None == v ) {
         free(*str);
         *str = NULL;
         return 0;

@@ -211,6 +211,7 @@ static PyObject *fixed_bytearray_get(con
 
 int attrib__libxl_cpuid_policy_list_set(PyObject *v, libxl_cpuid_policy_list *pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Setting cpuid_policy_list");
     return -1;
 }
 

@@ -233,21 +234,29 @@ int attrib__libxl_cpuarray_set(PyObject 
 
 int attrib__libxl_domain_build_state_ptr_set(PyObject *v, libxl_domain_build_state **pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Setting domain_build_state_ptr");
     return -1;
 }
 
 int attrib__libxl_file_reference_set(PyObject *v, libxl_file_reference *pptr)
 {
-    return -1;
+    return genwrap__string_set(v, &pptr->path);
 }
 
 int attrib__libxl_hwcap_set(PyObject *v, libxl_hwcap *pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Setting hwcap");
     return -1;
 }
 
 int attrib__libxl_key_value_list_set(PyObject *v, libxl_key_value_list *pptr)
 {
+    if ( *pptr ) {
+        libxl_key_value_list_destroy(pptr);
+        *pptr = NULL;
+    }
+    if ( v == Py_None )
+        return 0;
     return -1;
 }
 

@@ -258,6 +267,7 @@ int attrib__libxl_mac_set(PyObject *v, l
 
 int attrib__libxl_string_list_set(PyObject *v, libxl_string_list *pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Setting string_list");
     return -1;
 }
 

@@ -268,11 +278,13 @@ int attrib__libxl_uuid_set(PyObject *v, 
 
 int attrib__struct_in_addr_set(PyObject *v, struct in_addr *pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Setting in_addr");
     return -1;
 }
 
 PyObject *attrib__libxl_cpuid_policy_list_get(libxl_cpuid_policy_list *pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Getting cpuid_policy_list");
     return NULL;
 }
 

@@ -314,21 +326,24 @@ PyObject *attrib__libxl_cpuarray_get(lib
 
 PyObject *attrib__libxl_domain_build_state_ptr_get(libxl_domain_build_state **pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Getting domain_build_state_ptr");
     return NULL;
 }
 
 PyObject *attrib__libxl_file_reference_get(libxl_file_reference *pptr)
 {
-    return NULL;
+    return genwrap__string_get(&pptr->path);
 }
 
 PyObject *attrib__libxl_hwcap_get(libxl_hwcap *pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Getting hwcap");
     return NULL;
 }
 
 PyObject *attrib__libxl_key_value_list_get(libxl_key_value_list *pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Getting key_value_list");
     return NULL;
 }
 

@@ -339,6 +354,7 @@ PyObject *attrib__libxl_mac_get(libxl_ma
 
 PyObject *attrib__libxl_string_list_get(libxl_string_list *pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Getting string_list");
     return NULL;
 }
 

@@ -349,6 +365,7 @@ PyObject *attrib__libxl_uuid_get(libxl_u
 
 PyObject *attrib__struct_in_addr_get(struct in_addr *pptr)
 {
+    PyErr_SetString(PyExc_NotImplementedError, "Getting in_addr");
     return NULL;
 }

 tools/libxl/libxl.idl   |    2 +-
 tools/python/genwrap.py |  184 ++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 155 insertions(+), 31 deletions(-)

# HG changeset patch
# User Gianni Tedesco <[email protected]>
# Date 1294761668 0
# Node ID 735df86eab9a758431f9a3927db64e02baae82d6
# Parent f0cb6ba7a4cf52b63cca5595a68568657976d57d
pyxl: Recursively scan type-tree to produce complete binding boilerplate

Currently the python wrapper generator ignores non-toplevel types in the IDL. The KeyedUnion implementation is trivial but for structures embedded in structures the code becomes more complex. The problem is that when assigning to a structure (a.b = x) either we:
- copy the c structure from x in to a.b which is unexpected for python programmers since everything is by reference not by value.
- keep a reference to x which 'shadows' a.b and do the copy just before we pass the wrapped C structure across the libxl C API boundary.

The latter approach is chosen. The tree of shadow references is maintained in the auto-generated code and Py_foo_Unshadow() functions are also generated and exposed for the hand-written part of the xl wrapper to use.

Finally, for anonymous types, a name mangling system is used to translate names like x.u.hvm.acpi. To support such constructs in python we would need to export python types for x.u.hvm such as 'xl.anonymous_hvm_0'. Instead of dealing with this we simply flatten the type tree so that such a field is named x.u_hvm_apic.

diff -r f0cb6ba7a4cf -r 735df86eab9a tools/libxl/libxl.idl
@@ -115,7 +115,7 @@ libxl_domain_build_info = Struct("domain
                                         ("bootloader_args", string),
                                         ("cmdline", string),
                                         ("ramdisk", libxl_file_reference),
-                                        ("features", string, True),
+                                        ("features", string),
                                         ])),
                  ])),
     ],

Gianni Tedesco writes ("[Xen-devel] [PATCH 07 of 10] pyxl: Recursively scan type-tree to produce complete binding boilerplate"):

I applied patches 1-6. They touch only pyxl and didn't break the build.

However patch 7 contains this:

...

We discussed this off-list at the time I was writing it. I think with one of the other guys. We decided that const char * in the interface was a bad idea since it imposed an allocation policy where no reason for it actually existed in the code. I could have worked around this by casting everything in the python bindings but we felt that was an ugly solution and not a good precedent to set.

You are right that it changes the code and may incorrectly free the "" allocated in libxl_dm.c - I could strdup() in that instance that but the interface is changed so we have a problem. Alternatively I can change 2 lines in genwrap.py to emit correct code for this.

Your chainsaw.... my baby... :(

Gianni

--
# HG changeset patch
# Parent 760d458df83d738019b404a2dba12959d79ac1dd
pyxl: Recursively scan type-tree to produce complete binding boilerplate

Currently the python wrapper generator ignores non-toplevel types in the IDL. The KeyedUnion implementation is trivial but for structures embedded in structures the code becomes more complex. The problem is that when assigning to a structure (a.b = x) either we:
- copy the c structure from x in to a.b which is unexpected for python programmers since everything is by reference not by value.
- keep a reference to x which 'shadows' a.b and do the copy just before we pass the wrapped C structure across the libxl C API boundary.

The latter approach is chosen. The tree of shadow references is maintained in the auto-generated code and Py_foo_Unshadow() functions are also generated and exposed for the hand-written part of the xl wrapper to use.

Finally, for anonymous types, a name mangling system is used to translate names like x.u.hvm.acpi. To support such constructs in python we would need to export python types for x.u.hvm such as 'xl.anonymous_hvm_0'. Instead of dealing with this we simply flatten the type tree so that such a field is named x.u_hvm_apic.

diff -r 760d458df83d tools/python/genwrap.py
@@ -4,7 +4,7 @@ import sys,os
 
 import libxltypes
 
-(TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING) = range(4)
+(TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING) = xrange(4)
 
 def py_type(ty):
     if ty == libxltypes.bool or isinstance(ty, libxltypes.BitField) and ty.width == 1:

@@ -18,11 +18,16 @@ def py_type(ty):
         return TYPE_STRING
     return None
 
+def shadow_fields(ty):
+    return filter(lambda x:x.shadow_type is True, ty.fields)
+
 def py_wrapstruct(ty):
     l = []
     l.append('typedef struct {')
     l.append('    PyObject_HEAD;')
     l.append('    %s obj;'%ty.typename);
+    for f in shadow_fields(ty):
+        l.append('    Py_%s *%s_ref;'%(f.type.rawname, f.python_name))
     l.append('}Py_%s;'%ty.rawname)
     l.append('')
     return "\n".join(l) + "\n"

@@ -36,33 +41,59 @@ def py_decls(ty):
     l = []
     l.append('_hidden Py_%s *Py%s_New(void);\n'%(ty.rawname, ty.rawname))
     l.append('_hidden int Py%s_Check(PyObject *self);\n'%ty.rawname)
+    if len(shadow_fields(ty)) > 0:
+        l.append('_hidden void Py%s_Unshadow(Py_%s *self);\n'%(ty.rawname, ty.rawname))
     for f in ty.fields:
         if py_type(f.type) is not None:
             continue
-        l.append('_hidden PyObject *attrib__%s_get(%s *%s);'%(\
-                 fsanitize(f.type.typename), f.type.typename, f.name))
-        l.append('_hidden int attrib__%s_set(PyObject *v, %s *%s);'%(\
-                 fsanitize(f.type.typename), f.type.typename, f.name))
+        l.append('_hidden PyObject *attrib__%s_get(%s *val);'%(\
+                 fsanitize(f.type.typename), f.type.typename))
+        l.append('_hidden int attrib__%s_set(PyObject *v, %s *val);'%(\
+                 fsanitize(f.type.typename), f.type.typename))
     return '\n'.join(l) + "\n"
 
+def union_check(f, ret, prefix = 'self->obj.'):
+    u_check = []
+    if len(f.condlist):
+        cond = '||'.join(map(lambda (expr,name):('!(' + expr + ')')%('%s%s'%(prefix,name)), f.condlist))
+        u_check.append('    if ( %s ) {'%cond)
+        u_check.append('        PyErr_SetString(PyExc_AttributeError, "Union key not selected");')
+        u_check.append('        return %s;'%ret)
+        u_check.append('    }')
+    return u_check
+
 def py_attrib_get(ty, f):
     t = py_type(f.type)
     l = []
-    l.append('static PyObject *py_%s_%s_get(Py_%s *self, void *priv)'%(ty.rawname, f.name, ty.rawname))
+    l.append('static PyObject *py_%s_%s_get(Py_%s *self, void *priv)'%(ty.rawname, f.python_name, ty.rawname))
     l.append('{')
+
+    u_check = union_check(f, 'NULL')
+
     if t == TYPE_BOOL:
         l.append('    PyObject *ret;')
+        l.extend(u_check)
         l.append('    ret = (self->obj.%s) ? Py_True : Py_False;'%f.name)
         l.append('    Py_INCREF(ret);')
         l.append('    return ret;')
     elif t == TYPE_INT:
+        l.extend(u_check)
         l.append('    return genwrap__ll_get(self->obj.%s);'%f.name)
     elif t == TYPE_UINT:
+        l.extend(u_check)
         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)
+        l.extend(u_check)
+        l.append('    return genwrap__string_get((char **)&self->obj.%s);'%f.name)
+    elif f.shadow_type is True:
+        l.append('    PyObject *ret;')
+        l.extend(u_check)
+        l.append('    ret = (self->%s_ref == NULL) ? Py_None : (PyObject *)self->%s_ref;'%(f.python_name, f.python_name))
+        l.append('    Py_INCREF(ret);')
+        l.append('    return ret;')
     else:
         tn = f.type.typename
+        l.extend(u_check)
         l.append('    return attrib__%s_get((%s *)&self->obj.%s);'%(fsanitize(tn), tn, f.name))
     l.append('}')
     return '\n'.join(l) + "\n\n"

@@ -70,14 +101,17 @@ def py_attrib_get(ty, f):
 def py_attrib_set(ty, f):
     t = py_type(f.type)
     l = []
-    l.append('static int py_%s_%s_set(Py_%s *self, PyObject *v, void *priv)'%(ty.rawname, f.name, ty.rawname))
+    l.append('static int py_%s_%s_set(Py_%s *self, PyObject *v, void *priv)'%(ty.rawname, f.python_name, ty.rawname))
     l.append('{')
+    u_check = union_check(f, '-1')
     if t == TYPE_BOOL:
+        l.extend(u_check)
         l.append('    self->obj.%s = (NULL == v || Py_None == v || Py_False == v) ? 0 : 1;'%f.name)
         l.append('    return 0;')
     elif t == TYPE_UINT or t == TYPE_INT:
         l.append('    %slong long tmp;'%(t == TYPE_UINT and 'unsigned ' or ''))
         l.append('    int ret;')
+        l.extend(u_check)
         if t == TYPE_UINT:
             l.append('    ret = genwrap__ull_set(v, &tmp, (%s)~0);'%f.type.typename)
         else:

@@ -86,47 +120,87 @@ def py_attrib_set(ty, f):
         l.append('        self->obj.%s = tmp;'%f.name)
         l.append('    return ret;')
     elif t == TYPE_STRING:
-        l.append('    return genwrap__string_set(v, &self->obj.%s);'%f.name)
+        l.extend(u_check)
+        l.append('    return genwrap__string_set(v, (char **)&self->obj.%s);'%f.name)
+    elif f.shadow_type is True:
+        l.extend(u_check)
+        l.append('    if ( !Py%s_Check(v) ) {'%f.type.rawname)
+        l.append('        PyErr_SetString(PyExc_TypeError, "Expected xl.%s");'%f.type.rawname)
+        l.append('        return -1;')
+        l.append('    }')
+        l.append('    if ( self->%s_ref ) {'%f.python_name)
+        l.append('        Py_DECREF(self->%s_ref);'%f.python_name)
+        l.append('    }')
+        l.append('    self->%s_ref = (Py_%s *)v;'%(f.python_name, f.type.rawname))
+        l.append('    Py_INCREF(self->%s_ref);'%f.python_name)
+        l.append('    return 0;')
     else:
         tn = f.type.typename
+        l.extend(u_check)
         l.append('    return attrib__%s_set(v, (%s *)&self->obj.%s);'%(fsanitize(tn), tn, f.name))
     l.append('}')
     return '\n'.join(l) + "\n\n"
 
+def sync_func(ty):
+    pf = shadow_fields(ty)
+    if len(pf) == 0:
+        return ''
+    l = []
+
+    l.append('void Py%s_Unshadow(Py_%s*self)\n'%(ty.rawname, ty.rawname))
+    l.append('{')
+    for f in pf:
+        l.append('    if ( self->%s_ref ) {'%f.python_name)
+        l.append('        Py_%s *x = (Py_%s *)self->%s_ref;'%(f.type.rawname, f.type.rawname, f.python_name))
+        if len(shadow_fields(f.type)):
+            l.append('        Py%s_Unshadow(x);'%f.type.rawname)
+        l.append('        memcpy(&self->obj.%s, &x->obj, sizeof(self->obj.%s));'%(f.name, f.name))
+        l.append('    }else{')
+        l.append('        memset(&self->obj.%s, 0, sizeof(self->obj.%s));'%(f.name, f.name))
+        l.append('    }')
+    l.append('}')
+    l.append('')
+    return '\n'.join(l)
+
 def py_object_def(ty):
     l = []
-    if ty.destructor_fn is not None:
-        dtor = '    %s(&self->obj);\n'%ty.destructor_fn
-    else:
-        dtor = ''
-
-    funcs="""static void Py%(rawname)s_dealloc(Py_%(rawname)s *self)
-{
-%(dtor)s    self->ob_type->tp_free((PyObject *)self);
-}
-
-static int Py%(rawname)s_init(Py_%(rawname)s *self, PyObject *args, PyObject *kwds)
+    funcs="""static int Py%s_init(Py_%s *self, PyObject *args, PyObject *kwds)
 {
     memset(&self->obj, 0, sizeof(self->obj));
     return genwrap__obj_init((PyObject *)self, args, kwds);
 }
 
-static PyObject *Py%(rawname)s_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
+static PyObject *Py%s_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 {
-    Py_%(rawname)s *self = (Py_%(rawname)s *)type->tp_alloc(type, 0);
+    Py_%s *self = (Py_%s *)type->tp_alloc(type, 0);
     if (self == NULL)
         return NULL;
     memset(&self->obj, 0, sizeof(self->obj));
     return (PyObject *)self;
 }
 
-"""%{'rawname': ty.rawname, 'dtor': dtor}
+"""%tuple(ty.rawname for x in range(5))
+
+    funcs += sync_func(ty)
+
+
+    l.append('static void Py%s_dealloc(Py_%s *self)'%(ty.rawname, ty.rawname))
+    l.append('{')
+    if ty.destructor_fn is not None:
+        l.append('    %s(&self->obj);'%ty.destructor_fn)
+    for f in shadow_fields(ty):
+        l.append('    if ( self->%s_ref ) {'%f.python_name)
+        l.append('        Py_DECREF(self->%s_ref);'%f.python_name)
+        l.append('    }')
+    l.append('self->ob_type->tp_free((PyObject *)self);')
+    l.append('}')
+    l.append('')
 
     l.append('static PyGetSetDef Py%s_getset[] = {'%ty.rawname)
     for f in ty.fields:
-        l.append('    { .name = "%s", '%f.name)
-        l.append('      .get = (getter)py_%s_%s_get, '%(ty.rawname, f.name))
-        l.append('      .set = (setter)py_%s_%s_set },'%(ty.rawname, f.name))
+        l.append('    { .name = "%s", '%f.python_name)
+        l.append('      .get = (getter)py_%s_%s_get, '%(ty.rawname, f.python_name))
+        l.append('      .set = (setter)py_%s_%s_set },'%(ty.rawname, f.python_name))
     l.append('    { .name = NULL }')
     l.append('};')
     struct="""

@@ -196,12 +270,62 @@ def py_initfuncs(types):
     l.append('}')
     return '\n'.join(l) + "\n\n"
 
-def tree_frob(types):
-    ret = types[:]
-    for ty in ret:
-        ty.fields = filter(lambda f:f.name is not None and f.type.typename is not None, ty.fields)
+def dbg_tree(str, indent=0):
+    do_dbg = True
+    if not do_dbg:
+        return
+    print '%s%s'%(''.join('  ' for i in xrange(indent)), str)
+
+# We don't have a good translation for anonymous structures so we just
+# flatten them out recursively and replace '.' with '_'. For example
+# domain_build_info.u.hvm.pae becomes domain_build_info.u_hvm_pae
+def flatten_type(ty, path = None, depth = 0, condvar = []):
+    if not isinstance(ty, libxltypes.Aggregate):
+        return ty.fields
+
+    if path is None:
+        path = ''
+    else:
+        path = path + '.'
+
+    ret = []
+    for f in ty.fields:
+        f.name = path + f.name
+        f.python_name = f.name.replace('.', '_')
+        if isinstance(f.type, libxltypes.Aggregate) and \
+                f.type.typename is not None:
+            f.shadow_type = True
+        else:
+            f.shadow_type = False
+
+        if isinstance(f.type, libxltypes.KeyedUnion):
+            f.type.keyvar_name = path + f.type.keyvar_name
+
+        f.condlist = condvar[:]
+
+        if isinstance(f.type, libxltypes.Aggregate) and \
+                f.type.typename is None:
+            dbg_tree('(%s)'%(f.name), depth + 1)
+            if isinstance(ty, libxltypes.KeyedUnion):
+                condvar.append((f.keyvar_expr, ty.keyvar_name))
+            ret.extend(flatten_type(f.type, f.name, depth + 1, condvar))
+            if isinstance(ty, libxltypes.KeyedUnion):
+                condvar.pop()
+        else:
+            dbg_tree('%s - %s'%(f.name, f.python_name), depth + 1)
+            ret.append(f)
+
+
     return ret
 
+def frob_type(type):
+    dbg_tree('[%s]'%type.typename)
+    type.fields = flatten_type(type)
+    return type
+
+def frob_types(types):
+    return map(frob_type, types)
+
 if __name__ == '__main__':
     if len(sys.argv) < 4:
         print >>sys.stderr, "Usage: genwrap.py <idl> <decls> <defns>"

@@ -210,7 +334,7 @@ if __name__ == '__main__':
     idl = sys.argv[1]
     (_,types) = libxltypes.parse(idl)
 
-    types = tree_frob(types)
+    types = frob_types(types)
 
     decls = sys.argv[2]
     f = open(decls, 'w')

@@ -250,7 +374,7 @@ _hidden int genwrap__ll_set(PyObject *v,
 
 """ % " ".join(sys.argv))
     for ty in types:
-        f.write('/* Internal APU for %s wrapper */\n'%ty.typename)
+        f.write('/* Internal API for %s wrapper */\n'%ty.typename)
         f.write(py_wrapstruct(ty))
         f.write(py_decls(ty))
         f.write('\n')

Gianni Tedesco writes ("Re: [Xen-devel] [PATCH 07 of 10] pyxl: Recursively scan type-tree to produce complete binding boilerplate"):

Right.

However: There was no explanation of this change in your patch comment. It should probably have been a separate patch anyway. Furthermore, if you had agreement from anyone else that this change should be made despite the freeze I'm afraid you didn't mention it.

All of these are possibilities but I think this series should probably wait for 4.2 now.

There is no point saying we're having a freeze if we always let new stuff in anyway. I think this is beyond the scope of the exception we should make at this stage. Feel free to try to convince me otherwise.

Sorry.

As we have just discussed if you want to resubmit the rest of the series in a way that doesn't change the autogenerated code, then I'm happy to apply it on the basis that it's (a) a bugfix to the python bindings (which are broken right now) and (b) low risk if it really doesn't change the autogenerated code.

If you need to do something which changes the autogenerated code then I'll be wanting a second opinion at least.

Ian.

Specifically, I'd like some a discussion of each change to the autogenerated output, and why it is necessary/justifiable/safe.

Ian.

OK, well the code I resubmitted just then no longer makes that change to auto-generated code. Btw. we are talking about whether that field gets free()'d in the destructor function.

The problem is that this field is used once in libxl and it's assigned as "", a static const char *. In the original version of the patch I submitted the change leads to free() being called on that which is erroneous. Changing it to not be const is a problem because callers will need to know to strdup() now but the compiler wont be able to tell them that.

So, on the flip-side, my solution (patch v2) has just been to cast this (const char *) to (char *) when we modify it from python bindings. This is mostly fine correct the bindings handle all allocations. The problem is that we just call the destructor function when the object in question's (domain_build_info) goes away.

A workaround is to assign None to the field after it has been set to a string but before the variable goes out of scope and gets collected. It's a leak, but it's a small one. It would be a matter of a few more lines of code in gewrap.py to make that go away. Perhaps I should make these changes and re-submit end portion of series?

Gianni

I might have verbally misled Gianni to think that a change like this would be appropriate.
Bu thinking twice about it, even though it appears correct and it is just one line, it doesn't meet the criteria.

Well, this would have been back in November/December time, maybe earlier. I had even forgot all about it. It may have been appropriate then but not at this late stage.

Gianni

 tools/libxl/gentypes.py           |   30 +++++++-
 tools/libxl/libxltypes.py         |    6 +
 tools/python/genwrap.py           |  156 +++++++++++++++++++++++++++++++------
 tools/python/xen/lowlevel/xl/xl.c |   19 ++++
 4 files changed, 183 insertions(+), 28 deletions(-)

# HG changeset patch
# User Gianni Tedesco <[email protected]> # Date 1294761668 0
# Node ID 9c57b1622bbcc6252e59d18dca5e33673d6fc236 # Parent 735df86eab9a758431f9a3927db64e02baae82d6
xl: support array types in IDL

This is required to auto-generate language bindings for libxl_domain_config. An Array() types is implemented which causes the IDL header generator to output a pointer and a count variable for each instance. C destructor functions are also correctly generated.

They python wrapper part of this patch builds on the 'shadow' references introduced in the previous patch 'pyxl: Recursively scan type-tree to produce complete binding boilerplate' This means that array fields remain as python lists and present with the expected 'by-reference' semantics. The C array is built during the 'unshadow' process before the wrapped structure is passed accross the C API boundary in to libxl.

diff -r 735df86eab9a -r 9c57b1622bbc tools/libxl/gentypes.py
@@ -32,6 +32,16 @@ def libxl_C_instance_of(ty, instancename
     else:
         return libxl_C_type_of(ty) + " " + instancename
 
+def flatten_arrays(ty):
+    ret = []
+    for f in ty.fields:
+        if isinstance(f.type, libxltypes.Array):
+            ret.append(libxltypes.Field(libxltypes.integer, "num_%s"%f.name))
+            ret.append(libxltypes.Field(libxltypes.Reference(f.type.array_type), f.name))
+        else:
+            ret.append(f)
+    return ret
+    
 def libxl_C_type_define(ty, indent = ""):
     s = ""
     if isinstance(ty, libxltypes.Aggregate):

@@ -43,7 +53,7 @@ def libxl_C_type_define(ty, indent = "")
         else:
             s += "typedef %s {\n" % ty.kind
 
-        for f in ty.fields:
+        for f in flatten_arrays(ty):
             if f.comment is not None:
                 s += format_comment(4, f.comment)
             x = libxl_C_instance_of(f.type, f.name)

@@ -59,6 +69,15 @@ def libxl_C_type_define(ty, indent = "")
         raise NotImplementedError("%s" % type(ty))
     return s.replace("\n", "\n%s" % indent)
 
+def contains_array_dtor(ty, parent = None):
+    if isinstance(ty, libxltypes.Struct) and (parent is None or ty.destructor_fn is None):
+        for f in [f for f in ty.fields if not f.const]:
+            if isinstance(f.type, libxltypes.Array) and f.type.array_type.destructor_fn:
+                return True
+        if contains_array_dtor(f.type, True):
+            return True
+    return False
+
 def libxl_C_type_destroy(ty, v, reference, indent = "    ", parent = None):
     if reference:
         deref = v + "->"

@@ -85,6 +104,13 @@ def libxl_C_type_destroy(ty, v, referenc
             s += "%s(%s);\n" % (ty.destructor_fn, makeref + v)
     elif isinstance(ty, libxltypes.Struct) and (parent is None or ty.destructor_fn is None):
         for f in [f for f in ty.fields if not f.const]:
+            if isinstance(f.type, libxltypes.Array):
+                at = f.type.array_type
+                if at.destructor_fn:
+                    s += "for(i = 0; i < %s; i++)\n"%(deref + "num_" + f.name)
+                    s += "%s%s(&%s%s[i]);\n"%(indent, at.destructor_fn, deref, f.name)
+                    s += "free(%s%s);\n"%(deref, f.name)
+                continue
 
             if f.name is None: # Anonynous struct
                 s += libxl_C_type_destroy(f.type, deref, False, "", deref)

@@ -158,6 +184,8 @@ if __name__ == '__main__':
     for ty in [t for t in types if t.destructor_fn is not None and t.autogenerate_destructor]:
         f.write("void %s(%s *p)\n" % (ty.destructor_fn, ty.typename))
         f.write("{\n")
+        if contains_array_dtor(ty):
+            f.write("    int i;\n")
         f.write(libxl_C_type_destroy(ty, "p", True))
         f.write("    memset(p, LIBXL_DTOR_POISON, sizeof(*p));\n")
         f.write("}\n")

diff -r 735df86eab9a -r 9c57b1622bbc tools/libxl/libxltypes.py
@@ -69,6 +69,12 @@ class Field(object):
         self.comment = kwargs.setdefault('comment', None)
         self.keyvar_expr = kwargs.setdefault('keyvar_expr', None)
 
+class Array(Type):
+    """A counted array of elements"""
+    def __init__(self, type):
+        Type.__init__(self, None)
+        self.array_type = type
+
 class Aggregate(Type):
     """A type containing a collection of other types"""
     def __init__(self, kind, typename, fields, **kwargs):

diff -r 735df86eab9a -r 9c57b1622bbc tools/python/xen/lowlevel/xl/xl.c
@@ -73,6 +73,25 @@ int genwrap__obj_init(PyObject *self, Py
     return 0;
 }
 
+int genwrap__list_check(PyObject *list, int(*cbfn)(PyObject *obj))
+{
+    Py_ssize_t i, len;
+
+    if ( list == Py_None )
+        return 1;
+
+    if ( !PyList_Check(list) )
+        return 0;
+
+    len = PyList_Size(list);
+    for(i = 0; i < len; i++) {
+        if ( !(*cbfn)(PyList_GetItem(list, i)) )
+            return 0;
+    }
+
+    return 1;
+}
+
 int genwrap__string_set(PyObject *v, char **str)
 {
     char *tmp;

 tools/libxl/libxltypes.py |  4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

# HG changeset patch
# User Gianni Tedesco <[email protected]> # Date 1294761669 0
# Node ID 3bbba45fac5c7f614d75076aaf3e567799b2b4a7 # Parent 9c57b1622bbcc6252e59d18dca5e33673d6fc236
xl: Implement enum types in IDL

diff -r 9c57b1622bbc -r 3bbba45fac5c tools/libxl/libxltypes.py
@@ -44,6 +44,10 @@ class Number(Builtin):
         self.signed = kwargs['signed']
         Builtin.__init__(self, ctype, **kwargs)
 
+class Enum(Number):
+    def __init__(self, w, **kwargs):
+        Number.__init__(self, 'enum %s'%w, **kwargs)
+
 class UInt(Number):
     def __init__(self, w, **kwargs):
         kwargs.setdefault('namespace', None)

 tools/libxl/libxl.h               |  45 ++++++------------------
 tools/libxl/libxl.idl             |  16 +++++++++
 tools/libxl/libxl_create.c        |  33 ------------------
 tools/python/xen/lowlevel/xl/xl.c |  68 +++++++++++++++++++++++++++++++++++++-
 4 files changed, 94 insertions(+), 68 deletions(-)

# HG changeset patch
# User Gianni Tedesco <[email protected]> # Date 1294761669 0
# Node ID beded83ed66984ee588e18c6160c546895443607 # Parent 3bbba45fac5c7f614d75076aaf3e567799b2b4a7
pyxl: Export libxl_domain_create_new() to python binding

This also moves struct libxl_domain_config to be an IDL generated type.

diff -r 3bbba45fac5c -r beded83ed669 tools/libxl/libxl.h
@@ -205,6 +205,18 @@ typedef struct libxl__cpuid_policy libxl
 typedef libxl_cpuid_policy * libxl_cpuid_policy_list;
 void libxl_cpuid_destroy(libxl_cpuid_policy_list *cpuid_list);
 
+enum libxl_action_on_shutdown {
+    LIBXL_ACTION_DESTROY,
+
+    LIBXL_ACTION_RESTART,
+    LIBXL_ACTION_RESTART_RENAME,
+
+    LIBXL_ACTION_PRESERVE,
+
+    LIBXL_ACTION_COREDUMP_DESTROY,
+    LIBXL_ACTION_COREDUMP_RESTART,
+};
+
 #define LIBXL_PCI_FUNC_ALL (~0U)
 
 #include "_libxl_types.h"

@@ -241,38 +253,6 @@ enum {
 
 #define LIBXL_VERSION 0
 
-enum libxl_action_on_shutdown {
-    LIBXL_ACTION_DESTROY,
-
-    LIBXL_ACTION_RESTART,
-    LIBXL_ACTION_RESTART_RENAME,
-
-    LIBXL_ACTION_PRESERVE,
-
-    LIBXL_ACTION_COREDUMP_DESTROY,
-    LIBXL_ACTION_COREDUMP_RESTART,
-};
-
-typedef struct {
-    libxl_domain_create_info c_info;
-    libxl_domain_build_info b_info;
-    libxl_device_model_info dm_info;
-
-    int num_disks, num_vifs, num_vif2s, num_pcidevs, num_vfbs, num_vkbs;
-
-    libxl_device_disk *disks;
-    libxl_device_nic *vifs;
-    libxl_device_net2 *vif2s;
-    libxl_device_pci *pcidevs;
-    libxl_device_vfb *vfbs;
-    libxl_device_vkb *vkbs;
-
-    enum libxl_action_on_shutdown on_poweroff;
-    enum libxl_action_on_shutdown on_reboot;
-    enum libxl_action_on_shutdown on_watchdog;
-    enum libxl_action_on_shutdown on_crash;
-} libxl_domain_config;
-
 /* context functions */
 int libxl_ctx_init(libxl_ctx *ctx, int version, xentoollog_logger*);
 int libxl_ctx_free(libxl_ctx *ctx);

@@ -286,7 +266,6 @@ void libxl_init_dm_info(libxl_device_mod
 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_destroy(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 3bbba45fac5c -r beded83ed669 tools/libxl/libxl.idl
@@ -327,3 +327,19 @@ libxl_net2info = Struct("net2info", [
     ("back_mac", libxl_mac),
     ("filter_mac", integer),
     ])
+
+libxl_domain_config = Struct("domain_config", [
+    ("c_info", libxl_domain_create_info),
+    ("b_info", libxl_domain_build_info),
+    ("dm_info", libxl_device_model_info),
+    ("disks", Array(libxl_device_disk)),
+    ("vifs", Array(libxl_device_nic)),
+    ("vif2s", Array(libxl_device_net2)),
+    ("pcidevs", Array(libxl_device_pci)),
+    ("vfbs", Array(libxl_device_vfb)),
+    ("vkbs", Array(libxl_device_vkb)),
+    ("on_poweroff", Enum("libxl_action_on_shutdown")),
+    ("on_reboot", Enum("libxl_action_on_shutdown")),
+    ("on_watchdog", Enum("libxl_action_on_shutdown")),
+    ("on_crash", Enum("libxl_action_on_shutdown")),
+    ])

diff -r 3bbba45fac5c -r beded83ed669 tools/libxl/libxl_create.c
@@ -27,39 +27,6 @@
 #include "libxl_internal.h"
 #include "flexarray.h"
 
-void libxl_domain_config_destroy(libxl_domain_config *d_config)
-{
-    int i;
-
-    for (i=0; i<d_config->num_disks; i++)
-        libxl_device_disk_destroy(&d_config->disks[i]);
-    free(d_config->disks);
-
-    for (i=0; i<d_config->num_vifs; i++)
-        libxl_device_nic_destroy(&d_config->vifs[i]);
-    free(d_config->vifs);
-
-    for (i=0; i<d_config->num_vif2s; i++)
-        libxl_device_net2_destroy(&d_config->vif2s[i]);
-    free(d_config->vif2s);
-
-    for (i=0; i<d_config->num_pcidevs; i++)
-        libxl_device_pci_destroy(&d_config->pcidevs[i]);
-    free(d_config->pcidevs);
-
-    for (i=0; i<d_config->num_vfbs; i++)
-        libxl_device_vfb_destroy(&d_config->vfbs[i]);
-    free(d_config->vfbs);
-
-    for (i=0; i<d_config->num_vkbs; i++)
-        libxl_device_vkb_destroy(&d_config->vkbs[i]);
-    free(d_config->vkbs);
-
-    libxl_domain_create_info_destroy(&d_config->c_info);
-    libxl_domain_build_info_destroy(&d_config->b_info);
-    libxl_device_model_info_destroy(&d_config->dm_info);
-}
-
 void libxl_init_create_info(libxl_domain_create_info *c_info)
 {
     memset(c_info, '\0', sizeof(*c_info));

Attached file is a python script which re-implements some xl commands in python, used for testing the above patch-series.

Gianni

#!/usr/bin/python

from xen.lowlevel import xl

class PyxlError(xl.Error):
pass
class PyxlArgError(PyxlError):
pass
class PyxlNoDomain(PyxlError):
pass

def superclass(self, orig, type, kwargs):

    if orig == None:
        return
    if orig.__class__ != type:
        raise TypeError
    map(lambda (k,v): kwargs.setdefault(k, v),
            filter(lambda (k,v): k[0] != '_' and not callable(v),
                    map(lambda k:(k, getattr(orig, k)), dir(orig))))
    return

class Ctx:

    class __impl(xl.ctx):
        pass

    __instance = None

    def __init__(self):
        if Ctx.__instance is None:
            Ctx.__instance = self.__impl()
        self.__dict__['Ctx_instance'] = Ctx.__instance

    def __getattr__(self, attr):
        return getattr(self.__instance, attr)
    def __setattr__(self, attr, value):
        return setattr(self.__instance, attr, value)

class DomInfo(xl.dominfo):

    def __init__(self, orig = None, **kwargs):
        superclass(self, orig, xl.dominfo, kwargs)
        xl.dominfo.__init__(self, **kwargs)
        self.name = None
    def set_name(self, name):
        self.name = str(name)
    def __repr__(self):
        if self.name is not None:
            return 'dominfo(%s:%d)'%(self.name, self.domid)
        else:
            return 'dominfo(%d)'%self.domid
    def __str__(self):
        return '%s %5d %5lu %5d     %s%s%s%s%s%s  %8.1f'%(self.name.ljust(40),
                self.domid,
                self.max_memkb / 1024,
                self.vcpu_online,
                self.running and 'r' or '-',
                self.blocked and 'b' or '-',
                self.paused and 'p' or '-',
                self.shutdown and 's' or '-',
                self.shutdown_reason == xl.SHUTDOWN_crash and 'c' or '-',
                self.dying and 'd' or '-',
                float(self.cpu_time) / 1e9)

def find_domain(arg, by_name = True, by_id = True):
ctx = Ctx()
domlist = map(DomInfo, ctx.list_domains()) map(lambda x:x.set_name(ctx.domid_to_name(x.domid)), domlist)

if by_id:
dl_by_id = dict(map(lambda v:(v.domid, v), domlist)) try:
return dl_by_id[int(arg)]
except:
pass

if by_name:
dl_by_name = dict(map(lambda v:(v.name, v), domlist)) try:
return dl_by_name[str(arg)]
except:
pass

raise PyxlNoDomain, "Domain '%s' not found"%arg

def uuid(s):

    a = ''.join(map(lambda x:'%.2x'%x, s))
    return '-'.join([a[0:8], a[8:12], a[12:16], a[16:20], a[20:]])

def do_rename(args):
"Rename a domain"

ctx = Ctx()
if len(args) is not 3:
raise PyxlArgError, 'Requires 2 arguments'
d = find_domain(args[1])

ctx.domain_rename(d.domid, args[2], d.name)

def do_list(args):
"List information about all/some domains"

verbose = '-v' in args
ctx = Ctx()
domlist = map(DomInfo, ctx.list_domains()) map(lambda x:x.set_name(ctx.domid_to_name(x.domid)), domlist)
print 'Name ID Mem VCPUs\tState\tTime(s)'
for x in domlist:
print '%s%s'%(x, verbose and ' %s'%uuid(x.uuid) or '')

def do_pause(args):
"Pause execution of a domain"

if len(args) is not 2:
raise PyxlArgError
ctx = Ctx()
d = find_domain(args[1])
ctx.domain_pause(d.domid)

def do_unpause(args):
"Unpause a paused domain"

if len(args) is not 2:
raise PyxlArgError
ctx = Ctx()
d = find_domain(args[1])
ctx.domain_unpause(d.domid)

def do_domid(args):
"Convert a domain name to domain id"
if len(args) is not 2:
raise PyxlArgError
ctx = Ctx()
d = find_domain(args[1], by_id = False)
print d.domid

def do_domname(args):
"Convert a domain id to domain name"
if len(args) is not 2:
raise PyxlArgError
ctx = Ctx()
d = find_domain(args[1], by_name = False)
print d.name

def do_pci_list_assignable(args):
"List all the assignable pci devices"
if len(args) is not 1:
raise PyxlArgError
ctx = Ctx()
for x in ctx.device_pci_list_assignable():
print "%.4x:%.2x:%.2x.%.1x"%(x.domain, x.bus, x.dev, x.func)

def do_pci_list(args):
"List pass-through pci devices for a domain"
if len(args) is not 2:
raise PyxlArgError
ctx = Ctx()
d = find_domain(args[1])
for x in ctx.device_pci_list(d.domid):
print "%.4x:%.2x:%.2x.%.1x"%(x.domain, x.bus, x.dev, x.func)

def do_pci_add(args):

    "Insert a new pass-through pci device"
    if len(args) not in [3,4]:
        raise PyxlArgError
    ctx = Ctx()
    d = find_domain(args[1])
    dev = ctx.device_pci_parse_bdf(args[2])
    # TODO: virtual slot
    ctx.device_pci_add(d.domid, dev)

def do_pci_del(args):

    "Remove a domain's pass-through pci device"
    if len(args) is not 3:
        raise PyxlArgError
    ctx = Ctx()
    d = find_domain(args[1])
    dev = ctx.device_pci_parse_bdf(args[2])
    ctx.device_pci_del(d.domid, dev)

def callback(xl, domid, priv):
print 'callback: %r %d %r'%(xl, domid, priv)

def do_destroy(args):

    if len(args) not in [2,3]:
        raise PyxlArgError
    force = (len(args) > 2 and '-f' in args)
    ctx = Ctx()
    d = find_domain(args[1])
    ctx.domain_destroy(d.domid, force)

def do_create(args):
hvmloader_path = '/home/scara/xen-unstable-tools.hg/tools/firmware/hvmloader/hvmloader' dm_path = '/home/scara/xen-unstable-tools.hg/tools/ioemu-remote/i386-dm/qemu-dm' name = 'pyxl-test'

# create info
ci = xl.domain_create_info()
ci.hap = True
ci.hvm = True
ci.oos = True
ci.name = name
ci.platformdata = None
ci.poolid = 0
ci.poolname = 'Pool-0'
ci.xsdata = None
ci.ssidref
ci.uuid = ''.join([chr(0) for x in xrange(16)])

# build info
bi = xl.domain_build_info()
#bi.cpuid = None
bi.cur_vcpus = 1
bi.max_vcpus = 1
bi.disable_migrate = False
bi.hvm = True
bi.u_hvm_pae = True
bi.u_hvm_apic = True
bi.u_hvm_acpi = True
bi.u_hvm_nx = True
bi.u_hvm_viridian = False
bi.u_hvm_timeoffset = None
bi.u_hvm_hpet = True
bi.u_hvm_vpt_align = True
bi.u_hvm_timer_mode = 1
bi.kernel = hvmloader_path
bi.shadow_memkb = 3072
bi.video_memkb = 8192
bi.target_memkb = 262144
bi.max_memkb = 262144
bi.tsc_mode = 0

# create dm
dm = xl.device_model_info()
dm.device_model = dm_path
dm.videoram = bi.video_memkb >> 10
dm.dom_name = name
dm.boot = 'cda'
dm.vnc = 1
dm.vncunused = 1
dm.vnclisten = '0.0.0.0'
dm.apic = bi.u_hvm_apic
dm.vcpus = bi.max_vcpus
dm.vcpu_avail = bi.cur_vcpus

# attach devices
disks = []
d = xl.device_disk()
d.physpath = '/vm/winxp.hd'
d.phystype = 5
d.unpluggable = True
d.virtpath = 'xvda'
d.readwrite = True
disks.append(d)

config = xl.domain_config()
config.c_info = ci
config.b_info = bi
config.dm_info = dm
config.disks = disks

ctx = Ctx()
domid = ctx.domain_create(config, callback, 'test') print 'domid %d'%domid

# unpause
ctx.domain_unpause(domid)

# daemonise maybe

def do_help(args):

    "Print this message"

    global cmdtbl
    if len(args) > 1:
        usage(args[1])
        return
    keys = cmdtbl.keys()
    keys.sort()
    print 'Usage pyxl [-v] <subcommand> [args]'
    print
    print 'pyxl full list of subcommands:'
    print
    for (k,fn,u) in map(lambda k:(k,) + cmdtbl[k], keys):
        print ' %s %s'%(k.ljust(20), fn.__doc__)

def usage(subcmd, exc = None):
global cmdtbl
from sys import stderr

if not cmdtbl.has_key(subcmd):
stderr.write('No such command %s\n'%subcmd) return

if exc is not None:
for msg in (exc.args):
stderr.write('\'xl %s\' %s\n'%(subcmd, msg))
if len(exc.args):
stderr.write('\n')

(fn, u) = cmdtbl[subcmd] stderr.write('Usage xl [-v] %s %s\n'%(subcmd, u)) stderr.write('\n%s\n\n'%fn.__doc__)

cmdtbl = {
    'create' : (do_create, '<ConfigFile> [options] [vars]'),
    'destroy' : (do_destroy, '<Domain> [-f]'),
    'rename' : (do_rename, '<Domain> <NewDomainName>'),
    'pause' : (do_pause, '<Domain>'),
    'unpause' : (do_unpause, '<Domain>'),
    'domid' : (do_domid, '<DomainName>'),
    'domname' : (do_domname, '<DomainID>'),
    'pci-attach' : (do_pci_add, '<Domain> <BDF> [Virtual Slot]'),
    'pci-detach' : (do_pci_del, '<Domain> <BDF>'),
    'pci-list' : (do_pci_list, '<Domain>'),
    'pci-list-assignable-devices' : (do_pci_list_assignable,''),
    'list' : (do_list, '[Domain]'),
    'help' : (do_help, '[subcommand]')
}

if __name__ == '__main__':
from sys import argv

if len(argv) < 2:
do_help(['help'])
raise SystemExit, True
if not cmdtbl.has_key(argv[1]):
from sys import stderr do_help(['help'])
stderr.write('No such command %s\n'%argv[1]) raise SystemExit, True
(fn, u) = cmdtbl[argv[1]]

try:
fn(argv[1:])
except PyxlArgError, e:
usage(argv[1], e)
raise SystemExit,True
except xl.Error, e:
from sys import stderr stderr.write('%s\n'%'\n'.join(map(lambda x:'Error: %s'%x, e.args))) raise SystemExit,True

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel