On Wed, Oct 29, 2008 at 12:03:26PM -0400, Ben Guthro wrote:

Attached are the python bindings for domain events

I have moved much of the complicated code from my prior submission out of C and into python.

Thanks :-)

This required a slight change to the generator. The new convention that we came up with is to append <classname>.py to the class as it is being generated, iff that file exists.

hum ... I didn't understood until I read the associated code. Fine !

examples/domain-events/events-python/event-test.py | 186 +++++++++ python/generator.py | 16 python/libvir.c | 410 +++++++++++++++++++++ python/libvir.py | 20 + python/libvirt_wrap.h | 27 + python/types.c | 47 ++ python/virConnect.py | 46 ++ 7 files changed, 747 insertions(+), 5 deletions(-)

Well, if you add new .py file i would expect the Makefile.am to be modified too

diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py new file mode 100755 index 0000000..ac53196 --- /dev/null +++ b/examples/domain-events/events-python/event-test.py [...] +def myDomainEventCallback1 (conn, dom, event, opaque): + print "myDomainEventCallback1 EVENT: Domain %s(%s) %s" % (dom.name(), dom.ID(), eventToString(event)) + +def myDomainEventCallback2 (conn, dom, event, opaque): + print "myDomainEventCallback2 EVENT: Domain %s(%s) %s" % (dom.name(), dom.ID(), eventToString(event))

Thinking out loud, maybe we should allow dom to be NULL/None in examples, if we extend the API later to add node related events dom would be NULL, isn't it ? [...]

+########################################## +# Main +########################################## + +def usage(): + print "usage: "+os.path.basename(sys.argv[0])+" [uri]" + print " uri will default to qemu:///system"

ideally for regression testing it would be nice to be able to provide a test driver definition, but augmented with an emulated scenario, things like: <scenario> <event type="start"> <domain type='test2'> <name>test2</name> <memory>512000</memory> <currentMemory>512000</currentMemory> <vcpu>2</vcpu> <os> <type arch='i686'>hvm</type> <boot dev='hd'/> </os> </domain> </event> <event type="pause"> <domain name="test"/> </event> <event type="resume"> <domain name="test"/> </event> <event type="destroy"> <domain name="test2"/> </event> </scenario>

diff --git a/python/generator.py b/python/generator.py index ca83eaf..6233c83 100755 --- a/python/generator.py +++ b/python/generator.py @@ -213,6 +213,8 @@ skipped_modules = {

skipped_types = { # 'int *': "usually a return type", + 'virConnectDomainEventCallback': "No function types in python", + 'virEventAddHandleFunc': "No function types in python", }

####################################################################### @@ -315,6 +317,7 @@ skip_impl = ( 'virStoragePoolListVolumes', 'virDomainBlockPeek', 'virDomainMemoryPeek', + 'virEventRegisterImpl', )

@@ -332,9 +335,8 @@ skip_function = ( 'virCopyLastError', # Python API is called virGetLastError instead 'virConnectOpenAuth', # Python C code is manually written 'virDefaultErrorFunc', # Python virErrorFuncHandler impl calls this from C - 'virConnectDomainEventRegister', # TODO: generate python bindings for these below XXX - 'virConnectDomainEventDeregister', - 'virEventRegisterImpl', + 'virConnectDomainEventRegister', # overridden in virConnect.py + 'virConnectDomainEventDeregister', # overridden in virConnect.py )

@@ -615,7 +617,6 @@ classes_destructors = { "virNetwork": "virNetworkFree", "virStoragePool": "virStoragePoolFree", "virStorageVol": "virStorageVolFree", - "virConnect": "virConnectClose", }

functions_noexcept = { @@ -1210,6 +1211,13 @@ def buildWrappers(): classes.write(" return ret\n");

classes.write("\n"); + # Append "<classname>.py" to class def, iff it exists + try: + extra = open(classname + ".py", "r") + classes.writelines(extra.readlines()) + extra.close() + except: + pass

Ah, now I understand the principle ... sure ! Well you managed to find your way though the generator, congrats :-)

+static int +libvirt_virConnectDomainEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + int event, + void *opaque) +{ + PyObject *pyobj_ret; + + PyObject *pyobj_conn_inst = (PyObject*)opaque; + PyObject *pyobj_dom = libvirt_virDomainPtrWrap(dom); + + PyObject *libvirt_module; + PyObject *libvirt_dict; + PyObject *pyobj_dom_class; + + PyObject *pyobj_dom_args; + PyObject *pyobj_dom_kw; + PyObject *pyobj_dom_inst; + + libvirt_module = PyImport_ImportModule("libvirt"); + if(!libvirt_module) { + printf("%s Error importing libvirt module\n", __FUNCTION__); + PyErr_Print(); + return -1; + }

That's where things starts to get a bit nasty

+ libvirt_dict = PyModule_GetDict(libvirt_module); + if(!libvirt_dict) { + printf("%s Error importing libvirt dictionary\n", __FUNCTION__); + PyErr_Print(); + return -1; + }

you're gonna open it on every event registration ? Sure the ImportModule does caching IIRC but that sounds a bit extreme. Maybe add an initialization funtion in libvir.py so that this get resolved only once don't you think so ?

+ pyobj_dom_class = PyDict_GetItemString(libvirt_dict, "virDomain"); + if(!pyobj_dom_class) { + printf("%s Error importing virDomain class\n", __FUNCTION__); + PyErr_Print(); + return -1; + }

+ /* Create a python instance of this virDomainPtr */ + pyobj_dom_args = PyTuple_New(1); + PyTuple_SetItem(pyobj_dom_args, 0, pyobj_conn_inst); + Py_INCREF(pyobj_conn_inst); + pyobj_dom_kw = PyDict_New(); + PyDict_SetItemString(pyobj_dom_kw, "_obj", pyobj_dom); + pyobj_dom_inst = PyInstance_New(pyobj_dom_class, pyobj_dom_args, + pyobj_dom_kw);

case for a NULL domain and pushing Py_None then would make sense IMHO

+ if(!pyobj_conn_inst) { + printf("%s Error creating a python instance of virDomain\n", __FUNCTION__); + PyErr_Print(); + return -1; + } + /* Call the Callback Dispatcher */ + pyobj_ret = PyObject_CallMethod(pyobj_conn_inst, + (char*)"dispatchDomainEventCallbacks", + (char*)"Oi", + pyobj_dom_inst, + event); + + Py_DECREF(pyobj_dom_inst); + Py_DECREF(pyobj_conn_inst);

We should make clear that the python callback can't keep any reference to the domain. Maybe the solution is to pass a name or UUID string and let the python user do the lookup, might also avoid the object duplication (but then the user will do the dictionary of domains and get into the same problems of object lifetime).... I hate asynch interfaces :-)

+ if(!pyobj_ret) { + printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); + PyErr_Print(); + return -1; + } else { + Py_DECREF(pyobj_ret); + return 0; + } +} + +static PyObject * +libvirt_virConnectDomainEventRegister(ATTRIBUTE_UNUSED PyObject * self, + PyObject * args) +{ + PyObject *py_retval; /* return value */ + PyObject *pyobj_conn; /* virConnectPtr */ + PyObject *pyobj_conn_inst; /* virConnect Python object */ + + virConnectPtr conn; + int ret = 0; + + if (!PyArg_ParseTuple + (args, (char *) "OO:virConnectDomainEventRegister", + &pyobj_conn, &pyobj_conn_inst)) { + printf("%s failed parsing tuple\n", __FUNCTION__); + return VIR_PY_INT_FAIL; + } + +#ifdef DEBUG_ERROR + printf("libvirt_virConnectDomainEventRegister(%p %p) called\n", + pyobj_conn, pyobj_conn_inst); +#endif + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + Py_INCREF(pyobj_conn_inst); + ret = virConnectDomainEventRegister(conn, + libvirt_virConnectDomainEventCallback, + (void *)pyobj_conn_inst); + + py_retval = libvirt_intWrap(ret); + return (py_retval); +} + +static PyObject * +libvirt_virConnectDomainEventDeregister(ATTRIBUTE_UNUSED PyObject * self, + PyObject * args) +{ + PyObject *py_retval; + PyObject *pyobj_conn; + PyObject *pyobj_conn_inst; + + virConnectPtr conn; + int ret = 0; + + if (!PyArg_ParseTuple + (args, (char *) "OO:virConnectDomainEventDeregister", + &pyobj_conn, &pyobj_conn_inst)) + return (NULL); + +#ifdef DEBUG_ERROR + printf("libvirt_virConnectDomainEventDeregister(%p) called\n", pyobj_conn); +#endif + + conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + + ret = virConnectDomainEventDeregister(conn, libvirt_virConnectDomainEventCallback); + Py_DECREF(pyobj_conn_inst); + py_retval = libvirt_intWrap(ret); + return (py_retval); +} + +/******************************************* + * Event Impl + *******************************************/ +static PyObject *addHandleObj = NULL; +static PyObject *updateHandleObj = NULL; +static PyObject *removeHandleObj = NULL; +static PyObject *addTimeoutObj = NULL; +static PyObject *updateTimeoutObj = NULL; +static PyObject *removeTimeoutObj = NULL; + + +static int +libvirt_virEventAddHandleFunc (int fd ATTRIBUTE_UNUSED, int event ATTRIBUTE_UNUSED, + virEventHandleCallback cb, void *opaque) +{ + PyObject *result = NULL; + PyObject *libvirt_dict; + PyObject *libvirt_module; + PyObject *python_cb; + PyObject *cb_obj; + PyObject *opaque_obj; + PyObject *cb_args; + PyObject *pyobj_args; + + /* Lookup the python callback */ + libvirt_module = PyImport_ImportModule("libvirt"); + if(!libvirt_module) { + printf("%s Error importing libvirt module\n", __FUNCTION__); + PyErr_Print(); + return -1; + } + libvirt_dict = PyModule_GetDict(libvirt_module); + if(!libvirt_dict) { + printf("%s Error importing libvirt dict\n", __FUNCTION__); + PyErr_Print(); + return -1; + }

Same remark, all this should probably be cached

+ python_cb = PyDict_GetItemString(libvirt_dict, "eventInvokeHandleCallback"); + if(!python_cb) { + printf("%s Error finding eventInvokeHandleCallback\n", __FUNCTION__); + PyErr_Print(); + return -1; + } + Py_INCREF(python_cb); + /* create tuple for cb */ + cb_obj = libvirt_virEventHandleCallbackWrap(cb); + opaque_obj = libvirt_virVoidPtrWrap(opaque); + + cb_args = PyTuple_New(2); + PyTuple_SetItem(cb_args, 0, cb_obj); + PyTuple_SetItem(cb_args, 1, opaque_obj); + + pyobj_args = PyTuple_New(4); + PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(fd)); + PyTuple_SetItem(pyobj_args, 1, libvirt_intWrap(event)); + PyTuple_SetItem(pyobj_args, 2, python_cb); + PyTuple_SetItem(pyobj_args, 3, cb_args); + + if(addHandleObj && PyCallable_Check(addHandleObj)) + result = PyEval_CallObject(addHandleObj, pyobj_args); + + Py_XDECREF(result); + Py_DECREF(pyobj_args); + return 0; +} + +static void +libvirt_virEventUpdateHandleFunc(int fd, int event) +{ + PyObject *result = NULL; + PyObject *pyobj_args = PyTuple_New(2); + PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(fd)); + PyTuple_SetItem(pyobj_args, 1, libvirt_intWrap(event)); + + if(updateHandleObj && PyCallable_Check(updateHandleObj)) + result = PyEval_CallObject(updateHandleObj, pyobj_args); + + Py_XDECREF(result); + Py_DECREF(pyobj_args); +} + +static int +libvirt_virEventRemoveHandleFunc(int fd) +{ + PyObject *result = NULL; + PyObject *pyobj_args = PyTuple_New(1); + PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(fd)); + + if(removeHandleObj && PyCallable_Check(removeHandleObj)) + result = PyEval_CallObject(removeHandleObj, pyobj_args); + + Py_XDECREF(result); + Py_DECREF(pyobj_args); + return 0; +} + +static int +libvirt_virEventAddTimeoutFunc(int timeout, virEventTimeoutCallback cb, + void *opaque) +{ + PyObject *result = NULL; + PyObject *libvirt_dict; + PyObject *libvirt_module; + PyObject *python_cb; + + PyObject *cb_obj; + PyObject *opaque_obj; + PyObject *cb_args; + PyObject *pyobj_args; + + /* Lookup the python callback */ + libvirt_module = PyImport_ImportModule("libvirt"); + if(!libvirt_module) { + printf("%s Error importing libvirt module\n", __FUNCTION__); + PyErr_Print(); + return -1; + } + libvirt_dict = PyModule_GetDict(libvirt_module); + if(!libvirt_dict) { + printf("%s Error importing libvirt dict\n", __FUNCTION__); + PyErr_Print(); + return -1; + }

idem

+ python_cb = PyDict_GetItemString(libvirt_dict, "eventInvokeTimeoutCallback"); + if(!python_cb) { + printf("%s Error finding eventInvokeTimeoutCallback\n", __FUNCTION__); + PyErr_Print(); + return -1; + } + Py_INCREF(python_cb); + + /* create tuple for cb */ + cb_obj = libvirt_virEventTimeoutCallbackWrap(cb); + opaque_obj = libvirt_virVoidPtrWrap(opaque); + + cb_args = PyTuple_New(2); + PyTuple_SetItem(cb_args, 0, cb_obj); + PyTuple_SetItem(cb_args, 1, opaque_obj); + + pyobj_args = PyTuple_New(3); + + PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(timeout)); + PyTuple_SetItem(pyobj_args, 1, python_cb); + PyTuple_SetItem(pyobj_args, 2, cb_args); + + if(addTimeoutObj && PyCallable_Check(addTimeoutObj)) + result = PyEval_CallObject(addTimeoutObj, pyobj_args); + + Py_XDECREF(result); + Py_DECREF(pyobj_args); + return 0; +} + +static void +libvirt_virEventUpdateTimeoutFunc(int timer, int timeout) +{ + PyObject *result = NULL; + PyObject *pyobj_args = PyTuple_New(2); + PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(timer)); + PyTuple_SetItem(pyobj_args, 1, libvirt_intWrap(timeout)); + + if(updateTimeoutObj && PyCallable_Check(updateTimeoutObj)) + result = PyEval_CallObject(updateTimeoutObj, pyobj_args); + + Py_XDECREF(result); + Py_DECREF(pyobj_args); +} + +static int +libvirt_virEventRemoveTimeoutFunc(int timer) +{ + PyObject *result = NULL; + PyObject *pyobj_args = PyTuple_New(1); + PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(timer)); + + if(updateTimeoutObj && PyCallable_Check(updateTimeoutObj)) + result = PyEval_CallObject(removeTimeoutObj, pyobj_args); + + Py_XDECREF(result); + Py_DECREF(pyobj_args); + return 0; +} + +static PyObject * +libvirt_virEventRegisterImpl(ATTRIBUTE_UNUSED PyObject * self, + PyObject * args) +{ + Py_XDECREF(addHandleObj); + Py_XDECREF(updateHandleObj); + Py_XDECREF(removeHandleObj); + Py_XDECREF(addTimeoutObj); + Py_XDECREF(updateTimeoutObj); + Py_XDECREF(removeTimeoutObj);

+ if (!PyArg_ParseTuple + (args, (char *) "OOOOOO:virEventRegisterImpl", + &addHandleObj, + &updateHandleObj, + &removeHandleObj, + &addTimeoutObj, + &updateTimeoutObj, + &removeTimeoutObj + )) + return VIR_PY_INT_FAIL; + + Py_INCREF(addHandleObj); + Py_INCREF(updateHandleObj); + Py_INCREF(removeHandleObj); + Py_INCREF(addTimeoutObj); + Py_INCREF(updateTimeoutObj); + Py_INCREF(removeTimeoutObj); + + virEventRegisterImpl(libvirt_virEventAddHandleFunc, + libvirt_virEventUpdateHandleFunc, + libvirt_virEventRemoveHandleFunc, + libvirt_virEventAddTimeoutFunc, + libvirt_virEventUpdateTimeoutFunc, + libvirt_virEventRemoveTimeoutFunc); + + return VIR_PY_INT_SUCCESS; +} [...]

hum, maybe the ParseTuple should be done before onto temporary variables and then only DECREF, right now if the parse fails, then the next event may lead to a crash due to garbage collected code :-) the include extension is a nice idea. Maybe comments should be automatically added at the beginning and end of included fragment in libvirt.py by generator.py, so that if people read it they know that part wasn't generated and comes from a given file.

diff --git a/python/virConnect.py b/python/virConnect.py new file mode 100644 index 0000000..23ae3e8 --- /dev/null +++ b/python/virConnect.py @@ -0,0 +1,46 @@ + # + # virConnect functions from virConnect.py (hand coded) + # + def __del__(self): + try: + for cb,opaque in self.domainEventCallbacks.items(): + del self.domainEventCallbacks[cb] + self.domainEventCallbacks = None + libvirtmod.virConnectDomainEventDeregister(self._o, self) + except AttributeError: + pass + + if self._o != None: + libvirtmod.virConnectClose(self._o) + self._o = None + + def domainEventDeregister(self, cb): + """Removes a Domain Event Callback. De-registering for a + domain callback will disable delivery of this event type """ + try: + del self.domainEventCallbacks[cb] + if len(self.domainEventCallbacks) == 0: + ret = libvirtmod.virConnectDomainEventDeregister(self._o, self) + if ret == -1: raise libvirtError ('virConnectDomainEventDeregister() failed', conn=self) + except AttributeError: + pass + + def domainEventRegister(self, cb, opaque): + """Adds a Domain Event Callback. Registering for a domain + callback will enable delivery of the events """ + try: + self.domainEventCallbacks[cb] = opaque + except AttributeError: + self.domainEventCallbacks = {cb:opaque} + ret = libvirtmod.virConnectDomainEventRegister(self._o, self) + if ret == -1: raise libvirtError ('virConnectDomainEventRegister() failed', conn=self) + + def dispatchDomainEventCallbacks(self, dom, event): + """Dispatches events to python user domain event callbacks + """ + try: + for cb,opaque in self.domainEventCallbacks.items(): + cb(self,dom,event,opaque) + return 0 + except AttributeError: + pass

seems there is a few trailing spaces or tabs at end of line in that module but i see them :-) A bit of feedback on those comments ? i think we are not far from commiting it too :-) thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/