Xen devel - Jan 2011 - [PATCH 00 of 10] 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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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 <gianni.tedesco@citrix.com>
# 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

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 719ba64edad1 -r 9ae3901ab42f tools/python/genwrap.py
--- a/tools/python/genwrap.py	Tue Jan 11 16:01:05 2011 +0000
+++ b/tools/python/genwrap.py	Tue Jan 11 16:01:05 2011 +0000
@@ -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
--- a/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:05 2011 +0000
+++ b/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:05 2011 +0000
@@ -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;
 }
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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


# HG changeset patch
# User Gianni Tedesco <gianni.tedesco@citrix.com>
# Date 1294761666 0
# Node ID b06b8e4174e1660b775bfee70f132542afe0a62e
# Parent  9ae3901ab42fd0458cf6150014e1f376a25493b5
pyxl: Un-muddle libxl_domain_(pause|unpause) wrappers

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 9ae3901ab42f -r b06b8e4174e1 tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:05 2011 +0000
+++ b/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:06 2011 +0000
@@ -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"},

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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


# HG changeset patch
# User Gianni Tedesco <gianni.tedesco@citrix.com>
# Date 1294761666 0
# Node ID 26fc86ef80278a10cd9954d7ce7e4221f5015baf
# Parent  b06b8e4174e1660b775bfee70f132542afe0a62e
pyxl: Export relevant integer constants from python wrapper

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r b06b8e4174e1 -r 26fc86ef8027 tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:06 2011 +0000
+++ b/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:06 2011 +0000
@@ -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);
 }
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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 <gianni.tedesco@citrix.com>
# Date 1294761667 0
# Node ID 791046eea8ded39ad09f1cf4b8482580e2a42e6a
# Parent  26fc86ef80278a10cd9954d7ce7e4221f5015baf
pyxl: Allow subclassing of auto-generated python types

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 26fc86ef8027 -r 791046eea8de tools/python/genwrap.py
--- a/tools/python/genwrap.py	Tue Jan 11 16:01:06 2011 +0000
+++ b/tools/python/genwrap.py	Tue Jan 11 16:01:07 2011 +0000
@@ -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
--- a/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:06 2011 +0000
+++ b/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:07 2011 +0000
@@ -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          */

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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


# HG changeset patch
# User Gianni Tedesco <gianni.tedesco@citrix.com>
# Date 1294761667 0
# Node ID fb64e8d377d84f99b0a0bac3d7f04dd62fea25b9
# Parent  791046eea8ded39ad09f1cf4b8482580e2a42e6a
pyxl: Export PCI passthrough related libxl functions

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 791046eea8de -r fb64e8d377d8 tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:07 2011 +0000
+++ b/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:07 2011 +0000
@@ -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 }
 };
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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


# HG changeset patch
# User Gianni Tedesco <gianni.tedesco@citrix.com>
# 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.

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r fb64e8d377d8 -r f0cb6ba7a4cf tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:07 2011 +0000
+++ b/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:07 2011 +0000
@@ -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;
 }
 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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


# HG changeset patch
# User Gianni Tedesco <gianni.tedesco@citrix.com>
# 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.

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r f0cb6ba7a4cf -r 735df86eab9a tools/libxl/libxl.idl
--- a/tools/libxl/libxl.idl	Tue Jan 11 16:01:07 2011 +0000
+++ b/tools/libxl/libxl.idl	Tue Jan 11 16:01:08 2011 +0000
@@ -115,7 +115,7 @@ libxl_domain_build_info = Struct("domain
                                         ("bootloader_args", string),
                                         ("cmdline", string),
                                         ("ramdisk",
libxl_file_reference),
-                                        ("features", string, True),
+                                        ("features", string),
                                         ])),
                  ])),
     ],
diff -r f0cb6ba7a4cf -r 735df86eab9a tools/python/genwrap.py
--- a/tools/python/genwrap.py	Tue Jan 11 16:01:07 2011 +0000
+++ b/tools/python/genwrap.py	Tue Jan 11 16:01:08 2011 +0000
@@ -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.extend(u_check)
         l.append(''    return
genwrap__string_get(&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.extend(u_check)
         l.append(''    return genwrap__string_set(v,
&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'')

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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 <gianni.tedesco@citrix.com>
# 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.

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 735df86eab9a -r 9c57b1622bbc tools/libxl/gentypes.py
--- a/tools/libxl/gentypes.py	Tue Jan 11 16:01:08 2011 +0000
+++ b/tools/libxl/gentypes.py	Tue Jan 11 16:01:08 2011 +0000
@@ -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
--- a/tools/libxl/libxltypes.py	Tue Jan 11 16:01:08 2011 +0000
+++ b/tools/libxl/libxltypes.py	Tue Jan 11 16:01:08 2011 +0000
@@ -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/genwrap.py
--- a/tools/python/genwrap.py	Tue Jan 11 16:01:08 2011 +0000
+++ b/tools/python/genwrap.py	Tue Jan 11 16:01:08 2011 +0000
@@ -4,9 +4,11 @@ import sys,os
 
 import libxltypes
 
-(TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING) = xrange(4)
+(TYPE_BOOL, TYPE_INT, TYPE_UINT, TYPE_STRING, TYPE_LIST) = xrange(5)
 
 def py_type(ty):
+    if isinstance(ty, libxltypes.Array):
+        return TYPE_LIST
     if ty == libxltypes.bool or isinstance(ty, libxltypes.BitField) and
ty.width == 1:
         return TYPE_BOOL
     if isinstance(ty, libxltypes.Number):
@@ -27,7 +29,10 @@ def py_wrapstruct(ty):
     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))
+        if isinstance(f.type, libxltypes.Array):
+             l.append(''    PyObject *%s_ref;''%f.python_name)
+        else:
+            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"
@@ -42,7 +47,7 @@ def py_decls(ty):
     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))
+        l.append(''_hidden int 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
@@ -62,6 +67,15 @@ def union_check(f, ret, prefix = ''self->
         u_check.append(''    }'')
     return u_check
 
+def num_var(fname):
+    "Determine the name of the count variable for an array with given
name"
+    # Yes, it''s a bit of a hacky way to do it
+    try:
+        i = fname.rindex(''.'')
+    except ValueError:
+        return ''num_%s''%fname
+    return fname[:i] + ''num_'' + fname[i:]
+
 def py_attrib_get(ty, f):
     t = py_type(f.type)
     l = []
@@ -124,14 +138,22 @@ def py_attrib_set(ty, f):
         l.append(''    return genwrap__string_set(v,
&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)
+        if isinstance(f.type, libxltypes.Array):
+            at = f.type.array_type
+            l.append(''    if ( !genwrap__list_check(v, Py%s_Check) )
{''%at.rawname)
+            l.append(''        PyErr_SetString(PyExc_TypeError,
"Expected list of xl.%s");''%at.rawname)
+        else:
+            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))
+        if isinstance(f.type, libxltypes.Array):
+            l.append(''    self->%s_ref =
v;''%f.python_name)
+        else:
+            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:
@@ -141,23 +163,88 @@ def py_attrib_set(ty, f):
     l.append(''}'')
     return ''\n''.join(l) + "\n\n"
 
-def sync_func(ty):
+def list_unshadow_func(ty):
+    if len(shadow_fields(ty)):
+        uncall = ''        Py%s_Unshadow(val)\n''%ty.rawname
+    else:
+        uncall = ''''
+
+    if ty.destructor_fn:
+        fcall = """
+    for(i = 0; i < *len; i++) {
+        %s((*ret) + i);
+    }
+
+"""%ty.destructor_fn
+    else:
+        fcall = ''''
+
+    return """/* list of %s */
+static int %s_list_unshadow(PyObject *list, %s **ret, int *len)
+{
+    Py_ssize_t sz = 0, i;
+    %s *arr = NULL;
+
+    if ( list == Py_None || list == NULL )
+        goto out;
+
+    if ( !PyList_Check(list) ) {
+        PyErr_SetString(PyExc_TypeError, "Expected list of xl.%s");
+        return 0;
+    }
+
+    sz = PyList_Size(list);
+    if ( sz <= 0 )
+        goto out;
+
+    arr = calloc(sz, sizeof(*arr));
+    if ( NULL == arr ) {
+        PyErr_SetString(PyExc_MemoryError, "Allocating array of %s");
+        return 0;
+    }
+
+out:
+    for(i = 0; i < sz; i++) {
+        Py_%s *val;
+        val = (Py_%s *)PyList_GetItem(list, i);
+%s        memcpy(arr + i, &val->obj, sizeof(val->obj));
+    }
+%s
+    free(*ret);
+    *ret = arr;
+    *len = sz;
+    return 1;
+}
+
+"""%tuple([ty.typename for x in xrange(4)] + [ty.rawname for x
in xrange(4)] + [uncall, fcall])
+
+def unshadow_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(''int Py%s_Unshadow(Py_%s*self)\n''%(ty.rawname,
ty.rawname))
     l.append(''{'')
+    l.append(''    int ret = 1;'')
     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(''    }'')
+        if isinstance(f.type, libxltypes.Array):
+            at = f.type.array_type
+            l.append(''    if ( !%s_list_unshadow(self->%s_ref,
''%(at.typename, f.python_name))
+            l.append(''                           &self->obj.%s,
''%f.name)
+            l.append(''                           &self->obj.%s)
)''%num_var(f.name))
+            l.append(''        ret = 0;'')
+        else:
+            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(''        if ( !Py%s_Unshadow(x)
)''%f.type.rawname)
+                l.append(''            ret = 0;'')
+            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(''    return ret;'')
     l.append(''}'')
     l.append('''')
     return ''\n''.join(l)
@@ -181,18 +268,24 @@ static PyObject *Py%s_new(PyTypeObject *
 
 """%tuple(ty.rawname for x in range(5))
 
-    funcs += sync_func(ty)
+    funcs += unshadow_func(ty)
 
-
+    l.append('''')
     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);'')
+        if isinstance(f.type, libxltypes.Array):
+            l.append(''    self->obj.%s = NULL;''%f.name)
+            l.append(''    self->obj.%s =
0;''%num_var(f.name))
+        else:
+            l.append(''    memset(&self->obj.%s, 0,
sizeof(self->obj.%s));''%(
+                     f.name, f.name))
+    if ty.destructor_fn is not None:
+        l.append(''   
%s(&self->obj);''%ty.destructor_fn)
+    l.append(''    self->ob_type->tp_free((PyObject
*)self);'')
     l.append(''}'')
     l.append('''')
 
@@ -279,7 +372,7 @@ def dbg_tree(str, indent=0):
 # 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 = []):
+def flatten_type(ty, sdict, path = None, depth = 0, condvar = []):
     if not isinstance(ty, libxltypes.Aggregate):
         return ty.fields
 
@@ -295,6 +388,9 @@ def flatten_type(ty, path = None, depth 
         if isinstance(f.type, libxltypes.Aggregate) and \
                 f.type.typename is not None:
             f.shadow_type = True
+        elif isinstance(f.type, libxltypes.Array):
+            sdict[f.type.array_type] = None
+            f.shadow_type = True
         else:
             f.shadow_type = False
 
@@ -308,7 +404,7 @@ def flatten_type(ty, path = None, depth 
             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))
+            ret.extend(flatten_type(f.type, sdict, f.name, depth + 1, condvar))
             if isinstance(ty, libxltypes.KeyedUnion):
                 condvar.pop()
         else:
@@ -318,13 +414,14 @@ def flatten_type(ty, path = None, depth 
 
     return ret
 
-def frob_type(type):
+def frob_type(type, sdict):
     dbg_tree(''[%s]''%type.typename)
-    type.fields = flatten_type(type)
+    type.fields = flatten_type(type, sdict)
     return type
 
 def frob_types(types):
-    return map(frob_type, types)
+    sdict = {}
+    return (map(lambda x:frob_type(x, sdict), types), sdict)
 
 if __name__ == ''__main__'':
     if len(sys.argv) < 4:
@@ -334,7 +431,7 @@ if __name__ == ''__main__'':
     idl = sys.argv[1]
     (_,types) = libxltypes.parse(idl)
 
-    types = frob_types(types)
+    (types, sdict) = frob_types(types)
 
     decls = sys.argv[2]
     f = open(decls, ''w'')
@@ -364,6 +461,9 @@ _hidden void genwrap__init(PyObject *m);
 /* Generic type initialiser */
 _hidden int genwrap__obj_init(PyObject *self, PyObject *args, PyObject *kwds);
 
+/* Generic checker for list wrapper */
+_hidden int genwrap__list_check(PyObject *list, int(*cbfn)(PyObject *obj));
+
 /* Auto-generated get/set functions for simple data-types */
 _hidden int genwrap__string_set(PyObject *v, char **str);
 _hidden PyObject *genwrap__string_get(char **str);
@@ -399,6 +499,8 @@ _hidden int genwrap__ll_set(PyObject *v,
 #include "%s"
 
 """ % tuple(('' ''.join(sys.argv),) +
(os.path.split(decls)[-1:]),))
+    for ty in sdict.keys():
+        f.write(list_unshadow_func(ty))
     for ty in types:
         f.write(''/* Attribute get/set functions for %s
*/\n''%ty.typename)
         for a in ty.fields:
diff -r 735df86eab9a -r 9c57b1622bbc tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:08 2011 +0000
+++ b/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:08 2011 +0000
@@ -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;

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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


# HG changeset patch
# User Gianni Tedesco <gianni.tedesco@citrix.com>
# Date 1294761669 0
# Node ID 3bbba45fac5c7f614d75076aaf3e567799b2b4a7
# Parent  9c57b1622bbcc6252e59d18dca5e33673d6fc236
xl: Implement enum types in IDL

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 9c57b1622bbc -r 3bbba45fac5c tools/libxl/libxltypes.py
--- a/tools/libxl/libxltypes.py	Tue Jan 11 16:01:08 2011 +0000
+++ b/tools/libxl/libxltypes.py	Tue Jan 11 16:01:09 2011 +0000
@@ -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)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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 <gianni.tedesco@citrix.com>
# 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.

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 3bbba45fac5c -r beded83ed669 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Tue Jan 11 16:01:09 2011 +0000
+++ b/tools/libxl/libxl.h	Tue Jan 11 16:01:09 2011 +0000
@@ -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
--- a/tools/libxl/libxl.idl	Tue Jan 11 16:01:09 2011 +0000
+++ b/tools/libxl/libxl.idl	Tue Jan 11 16:01:09 2011 +0000
@@ -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
--- a/tools/libxl/libxl_create.c	Tue Jan 11 16:01:09 2011 +0000
+++ b/tools/libxl/libxl_create.c	Tue Jan 11 16:01:09 2011 +0000
@@ -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));
diff -r 3bbba45fac5c -r beded83ed669 tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:09 2011 +0000
+++ b/tools/python/xen/lowlevel/xl/xl.c	Tue Jan 11 16:01:09 2011 +0000
@@ -443,6 +443,68 @@ static PyObject *pyxl_domid_to_name(XlOb
     return ret;
 }
 
+struct cb_priv {
+    PyObject *xl_ctx, *cb, *cb_priv;
+};
+
+static int console_ready(libxl_ctx *ctx, uint32_t domid, void *priv)
+{
+    struct cb_priv *p = priv;
+    PyObject *d;
+
+    /* no callback provided, succeed */
+    if ( p->cb == Py_None )
+        return 0;
+
+    d = PyInt_FromLong(domid);
+    if ( d == NULL )
+        return -1;
+
+    if ( PyObject_CallFunctionObjArgs(p->cb, p->xl_ctx, d, p->cb_priv,
NULL) ) {
+        Py_DECREF(d);
+        return 0;
+    }
+
+    Py_DECREF(d);
+    return -1;
+}
+
+static PyObject *pyxl_domain_create(XlObject *self, PyObject *args)
+{
+    PyObject *console_cb, *cb_priv, *rfile = NULL;
+    Py_domain_config *d_config;
+    struct cb_priv priv;
+    uint32_t domid;
+
+    if ( !PyArg_ParseTuple(args, "OOO|O", &d_config,
&console_cb, &cb_priv, &rfile) )
+        return NULL;
+
+    if ( !Pydomain_config_Check((PyObject *)d_config) ) {
+        PyErr_SetString(PyExc_TypeError, "Expected
xl.domain_config");
+        return NULL;
+    }
+
+    if ( console_cb != Py_None && !PyCallable_Check(console_cb) ) {
+        PyErr_SetString(PyExc_TypeError, "Expected callable console
callback");
+        return NULL;
+    }
+
+    if ( !Pydomain_config_Unshadow(d_config) )
+        return NULL;
+
+    priv.xl_ctx = (PyObject *)self;
+    priv.cb = console_cb;
+    priv.cb_priv = cb_priv;
+
+    if ( libxl_domain_create_new(&self->ctx, &d_config->obj,
+                                    console_ready, &priv, &domid) ) {
+        PyErr_SetString(xl_error_obj, "Error creating domain");
+        return NULL;
+    }
+
+    return PyInt_FromLong(domid);
+}
+
 static PyObject *pyxl_domain_shutdown(XlObject *self, PyObject *args)
 {
     int domid, req = 0;
@@ -517,7 +579,7 @@ static PyObject *pyxl_pci_add(XlObject *
     if ( !PyArg_ParseTuple(args, "iO", &domid, &obj) )
         return NULL;
     if ( !Pydevice_pci_Check(obj) ) {
-        PyErr_SetString(PyExc_TypeError, "Xxpected xl.device_pci");
+        PyErr_SetString(PyExc_TypeError, "Expected xl.device_pci");
         return NULL;
     }
     pci = (Py_device_pci *)obj;
@@ -538,7 +600,7 @@ static PyObject *pyxl_pci_del(XlObject *
     if ( !PyArg_ParseTuple(args, "iO|i", &domid, &obj,
&force) )
         return NULL;
     if ( !Pydevice_pci_Check(obj) ) {
-        PyErr_SetString(PyExc_TypeError, "Xxpected xl.device_pci");
+        PyErr_SetString(PyExc_TypeError, "Expected xl.device_pci");
         return NULL;
     }
     pci = (Py_device_pci *)obj;
@@ -649,6 +711,8 @@ static PyMethodDef pyxl_methods[] = {
          "Retrieve name from domain-id"},
     {"domain_shutdown", (PyCFunction)pyxl_domain_shutdown,
METH_VARARGS,
          "Shutdown a domain"},
+    {"domain_create", (PyCFunction)pyxl_domain_create, METH_VARARGS,
+         "Create or restore a domain"},
     {"domain_destroy", (PyCFunction)pyxl_domain_destroy,
METH_VARARGS,
          "Destroy a domain"},
     {"domain_pause", (PyCFunction)pyxl_domain_pause, METH_VARARGS,

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, 2011-01-11 at 16:03 +0000, Gianni Tedesco wrote:
> 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.
Attached file is a python script which re-implements some xl commands in
python, used for testing the above patch-series.

Gianni


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Gianni Tedesco writes ("[Xen-devel] [PATCH 07 of 10] pyxl: Recursively scan
type-tree to produce complete binding
boilerplate"):
> 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:
> --- a/tools/libxl/libxl.idl	Tue Jan 11 16:01:07 2011 +0000
> +++ b/tools/libxl/libxl.idl	Tue Jan 11 16:01:08 2011 +0000
...
> -                                        ("features", string,
True),
> +                                        ("features", string),
This makes a substantive change to both the interface and the
generated code, and wasn''t mentioned in the comment.  I''m not
convinced it''s a change that''s appropriate during feature
freeze.

So I''m pushing 1-6 and the rest will have to wait for after the 4.1
release, I''m afraid.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, 2011-01-11 at 19:26 +0000, Ian Jackson wrote:
> Gianni Tedesco writes ("[Xen-devel] [PATCH 07 of 10] pyxl: Recursively
scan type-tree to produce complete binding boilerplate"):
> > 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:
> 
> > --- a/tools/libxl/libxl.idl	Tue Jan 11 16:01:07 2011 +0000
> > +++ b/tools/libxl/libxl.idl	Tue Jan 11 16:01:08 2011 +0000
> ...
> > -                                        ("features",
string, True),
> > +                                        ("features",
string),
> 
> This makes a substantive change to both the interface and the
> generated code, and wasn''t mentioned in the comment.  I''m
not
> convinced it''s a change that''s appropriate during feature
freeze.
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.
> So I''m pushing 1-6 and the rest will have to wait for after the
4.1
> release, I''m afraid.
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.

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 760d458df83d tools/python/genwrap.py
--- a/tools/python/genwrap.py	Wed Jan 12 12:44:18 2011 +0000
+++ b/tools/python/genwrap.py	Wed Jan 12 12:46:57 2011 +0000
@@ -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'')



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Gianni Tedesco writes ("Re: [Xen-devel] [PATCH 07 of 10] pyxl: Recursively
scan type-tree to produce complete binding
boilerplate"):
> 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.
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.
> 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.
All of these are possibilities but I think this series should probably
wait for 4.2 now.
> > So I''m pushing 1-6 and the rest will have to wait for after
the 4.1
> > release, I''m afraid.
> 
> Your chainsaw.... my baby... :(
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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

I wrote:
> If you need to do something which changes the autogenerated code then
> I''ll be wanting a second opinion at least.
Specifically, I''d like some a discussion of each change to the
autogenerated output, and why it is necessary/justifiable/safe.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, 12 Jan 2011, Gianni Tedesco wrote:
> On Tue, 2011-01-11 at 19:26 +0000, Ian Jackson wrote:
> > Gianni Tedesco writes ("[Xen-devel] [PATCH 07 of 10] pyxl:
Recursively scan type-tree to produce complete binding boilerplate"):
> > > 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:
> >
> > > --- a/tools/libxl/libxl.idl Tue Jan 11 16:01:07 2011 +0000
> > > +++ b/tools/libxl/libxl.idl Tue Jan 11 16:01:08 2011 +0000
> > ...
> > > -                                        ("features",
string, True),
> > > +                                        ("features",
string),
> >
> > This makes a substantive change to both the interface and the
> > generated code, and wasn''t mentioned in the comment. 
I''m not
> > convinced it''s a change that''s appropriate during
feature freeze.
> 
> 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.
> 
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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, 2011-01-12 at 13:00 +0000, Ian Jackson wrote:
> I wrote:
> > If you need to do something which changes the autogenerated code then
> > I''ll be wanting a second opinion at least.
> 
> Specifically, I''d like some a discussion of each change to the
> autogenerated output, and why it is necessary/justifiable/safe.
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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, 2011-01-12 at 13:17 +0000, Stefano Stabellini
wrote:
> On Wed, 12 Jan 2011, Gianni Tedesco wrote:
> > On Tue, 2011-01-11 at 19:26 +0000, Ian Jackson wrote:
> > > Gianni Tedesco writes ("[Xen-devel] [PATCH 07 of 10] pyxl:
Recursively scan type-tree to produce complete binding boilerplate"):
> > > > 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:
> > >
> > > > --- a/tools/libxl/libxl.idl Tue Jan 11 16:01:07 2011 +0000
> > > > +++ b/tools/libxl/libxl.idl Tue Jan 11 16:01:08 2011 +0000
> > > ...
> > > > -                                       
("features", string, True),
> > > > +                                       
("features", string),
> > >
> > > This makes a substantive change to both the interface and the
> > > generated code, and wasn''t mentioned in the comment. 
I''m not
> > > convinced it''s a change that''s appropriate
during feature freeze.
> > 
> > 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.
> > 
> 
> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel