diff --git a/ChangeLog b/ChangeLog index 98ecc9013b..8414809c71 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +Mon Nov 24 19:27:40 GMT 2008 Dnaiel P. Berrange + + * examples/domain-events/events-python/event-test.py, + python/libvir.c, python/libvir.py, python/libvirt_wrap.h, + python/types.c, python/virConnect.py: Fixes to python + bindings for changes in public event API (David Lively) + Mon Nov 24 19:22:40 GMT 2008 Dnaiel P. Berrange * tests/Makefile.am, tests/sexpr2xmltest.c, diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py index 45aaa93d87..7bea606e9c 100644 --- a/examples/domain-events/events-python/event-test.py +++ b/examples/domain-events/events-python/event-test.py @@ -72,22 +72,22 @@ def myAddHandle(fd, events, cb, opaque): h_events = events h_cb = cb h_opaque = opaque - mypoll.register(fd, myEventHandleTypeToPollEvent(events)) + return 0 def myUpdateHandle(watch, event): global h_fd, h_events - #print "Updating Handle %s %s" % (str(fd), str(events)) - h_fd = fd + #print "Updating Handle %s %s" % (str(h_fd), str(event)) h_events = event - mypoll.unregister(watch) - mypoll.register(watch, myEventHandleTypeToPollEvent(event)) + mypoll.unregister(h_fd) + mypoll.register(h_fd, myEventHandleTypeToPollEvent(event)) def myRemoveHandle(watch): global h_fd - #print "Removing Handle %s" % str(fd) + #print "Removing Handle %s" % str(h_fd) + mypoll.unregister(h_fd) h_fd = 0 - mypoll.unregister(watch) + return h_opaque def myAddTimeout(timeout, cb, opaque): global t_active, t_timeout, t_cb, t_opaque @@ -96,16 +96,18 @@ def myAddTimeout(timeout, cb, opaque): t_timeout = timeout; t_cb = cb; t_opaque = opaque; + return 0 def myUpdateTimeout(timer, timeout): global t_timeout - #print "Updating Timeout %s" % (str(timer), str(timeout)) + #print "Updating Timeout %s %s" % (str(timer), str(timeout)) t_timeout = timeout; def myRemoveTimeout(timer): global t_active #print "Removing Timeout %s" % str(timer) t_active = 0; + return t_opaque ########################################## # Main @@ -143,6 +145,14 @@ def main(): myRemoveTimeout ); vc = libvirt.open(uri) + # Close connection on exit (to test cleanup paths) + old_exitfunc = getattr(sys, 'exitfunc', None) + def exit(): + print "Closing " + str(vc) + vc.close() + if (old_exitfunc): old_exitfunc() + sys.exitfunc = exit + #Add 2 callbacks to prove this works with more than just one vc.domainEventRegister(myDomainEventCallback1,None) vc.domainEventRegister(myDomainEventCallback2,None) diff --git a/python/libvir.c b/python/libvir.c index 7d58442de5..ca1e890521 100644 --- a/python/libvir.c +++ b/python/libvir.c @@ -35,6 +35,18 @@ extern void initcygvirtmod(void); #define VIR_PY_INT_FAIL (libvirt_intWrap(-1)) #define VIR_PY_INT_SUCCESS (libvirt_intWrap(0)) +static char *py_str(PyObject *obj) +{ + PyObject *str = PyObject_Str(obj); + if (!str) { + PyErr_Print(); + PyErr_Clear(); + return NULL; + }; + return PyString_AsString(str); +} + + /************************************************************************ * * * Statistics * @@ -484,7 +496,8 @@ libvirt_virErrorFuncHandler(ATTRIBUTE_UNUSED void *ctx, virErrorPtr err) PyObject *result; #ifdef DEBUG_ERROR - printf("libvirt_virErrorFuncHandler(%p, %s, ...) called\n", ctx, msg); + printf("libvirt_virErrorFuncHandler(%p, %s, ...) called\n", ctx, + err->message); #endif if ((err == NULL) || (err->code == VIR_ERR_OK)) @@ -1780,12 +1793,19 @@ libvirt_virConnectDomainEventDeregister(ATTRIBUTE_UNUSED PyObject * self, * Event Impl *******************************************/ static PyObject *addHandleObj = NULL; +static char *addHandleName = NULL; static PyObject *updateHandleObj = NULL; +static char *updateHandleName = NULL; static PyObject *removeHandleObj = NULL; +static char *removeHandleName = NULL; static PyObject *addTimeoutObj = NULL; +static char *addTimeoutName = NULL; static PyObject *updateTimeoutObj = NULL; +static char *updateTimeoutName = NULL; static PyObject *removeTimeoutObj = NULL; +static char *removeTimeoutName = NULL; +#define NAME(fn) ( fn ## Name ? fn ## Name : # fn ) static int libvirt_virEventAddHandleFunc (int fd, @@ -1794,13 +1814,14 @@ libvirt_virEventAddHandleFunc (int fd, void *opaque, virFreeCallback ff) { - PyObject *result = NULL; + PyObject *result; PyObject *python_cb; PyObject *cb_obj; PyObject *ff_obj; PyObject *opaque_obj; PyObject *cb_args; PyObject *pyobj_args; + int retval = -1; LIBVIRT_ENSURE_THREAD_STATE; @@ -1809,9 +1830,19 @@ libvirt_virEventAddHandleFunc (int fd, "eventInvokeHandleCallback"); if(!python_cb) { #if DEBUG_ERROR - printf("%s Error finding eventInvokeHandleCallback\n", __FUNCTION__); + printf("%s: Error finding eventInvokeHandleCallback\n", __FUNCTION__); #endif PyErr_Print(); + PyErr_Clear(); + goto cleanup; + } + if (!PyCallable_Check(python_cb)) { +#if DEBUG_ERROR + char *name = py_str(python_cb); + printf("%s: %s is not callable\n", __FUNCTION__, + name ? name : "libvirt.eventInvokeHandleCallback"); + free(name); +#endif goto cleanup; } Py_INCREF(python_cb); @@ -1832,8 +1863,17 @@ libvirt_virEventAddHandleFunc (int fd, 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); + result = PyEval_CallObject(addHandleObj, pyobj_args); + if (!result) { + PyErr_Print(); + PyErr_Clear(); + } else if (!PyInt_Check(result)) { +#if DEBUG_ERROR + printf("%s: %s should return an int\n", __FUNCTION__, NAME(addHandle)); +#endif + } else { + retval = (int)PyInt_AsLong(result); + } Py_XDECREF(result); Py_DECREF(pyobj_args); @@ -1841,23 +1881,26 @@ libvirt_virEventAddHandleFunc (int fd, cleanup: LIBVIRT_RELEASE_THREAD_STATE; - return 0; + return retval; } static void -libvirt_virEventUpdateHandleFunc(int fd, int event) +libvirt_virEventUpdateHandleFunc(int watch, int event) { - PyObject *result = NULL; + PyObject *result; PyObject *pyobj_args; LIBVIRT_ENSURE_THREAD_STATE; pyobj_args = PyTuple_New(2); - PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(fd)); + PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(watch)); PyTuple_SetItem(pyobj_args, 1, libvirt_intWrap(event)); - if(updateHandleObj && PyCallable_Check(updateHandleObj)) - result = PyEval_CallObject(updateHandleObj, pyobj_args); + result = PyEval_CallObject(updateHandleObj, pyobj_args); + if (!result) { + PyErr_Print(); + PyErr_Clear(); + } Py_XDECREF(result); Py_DECREF(pyobj_args); @@ -1867,25 +1910,45 @@ libvirt_virEventUpdateHandleFunc(int fd, int event) static int -libvirt_virEventRemoveHandleFunc(int fd) +libvirt_virEventRemoveHandleFunc(int watch) { - PyObject *result = NULL; + PyObject *result; PyObject *pyobj_args; + PyObject *opaque; + PyObject *ff; + int retval = -1; + virFreeCallback cff; LIBVIRT_ENSURE_THREAD_STATE; pyobj_args = PyTuple_New(1); - PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(fd)); + PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(watch)); - if(removeHandleObj && PyCallable_Check(removeHandleObj)) - result = PyEval_CallObject(removeHandleObj, pyobj_args); + result = PyEval_CallObject(removeHandleObj, pyobj_args); + if (!result) { + PyErr_Print(); + PyErr_Clear(); + } else if (!PyTuple_Check(result) || PyTuple_Size(result) != 3) { +#if DEBUG_ERROR + printf("%s: %s must return opaque obj registered with %s" + "to avoid leaking libvirt memory\n", + __FUNCTION__, NAME(removeHandle), NAME(addHandle)); +#endif + } else { + opaque = PyTuple_GetItem(result, 1); + ff = PyTuple_GetItem(result, 2); + cff = PyvirFreeCallback_Get(ff); + if (cff) + (*cff)(PyvirVoidPtr_Get(opaque)); + retval = 0; + } Py_XDECREF(result); Py_DECREF(pyobj_args); LIBVIRT_RELEASE_THREAD_STATE; - return 0; + return retval; } static int @@ -1894,7 +1957,7 @@ libvirt_virEventAddTimeoutFunc(int timeout, void *opaque, virFreeCallback ff) { - PyObject *result = NULL; + PyObject *result; PyObject *python_cb; @@ -1903,6 +1966,7 @@ libvirt_virEventAddTimeoutFunc(int timeout, PyObject *opaque_obj; PyObject *cb_args; PyObject *pyobj_args; + int retval = -1; LIBVIRT_ENSURE_THREAD_STATE; @@ -1911,9 +1975,19 @@ libvirt_virEventAddTimeoutFunc(int timeout, "eventInvokeTimeoutCallback"); if(!python_cb) { #if DEBUG_ERROR - printf("%s Error finding eventInvokeTimeoutCallback\n", __FUNCTION__); + printf("%s: Error finding eventInvokeTimeoutCallback\n", __FUNCTION__); #endif PyErr_Print(); + PyErr_Clear(); + goto cleanup; + } + if (!PyCallable_Check(python_cb)) { +#if DEBUG_ERROR + char *name = py_str(python_cb); + printf("%s: %s is not callable\n", __FUNCTION__, + name ? name : "libvirt.eventInvokeTimeoutCallback"); + free(name); +#endif goto cleanup; } Py_INCREF(python_cb); @@ -1934,15 +2008,24 @@ libvirt_virEventAddTimeoutFunc(int 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); + result = PyEval_CallObject(addTimeoutObj, pyobj_args); + if (!result) { + PyErr_Print(); + PyErr_Clear(); + } else if (!PyInt_Check(result)) { +#if DEBUG_ERROR + printf("%s: %s should return an int\n", __FUNCTION__, NAME(addTimeout)); +#endif + } else { + retval = (int)PyInt_AsLong(result); + } Py_XDECREF(result); Py_DECREF(pyobj_args); cleanup: LIBVIRT_RELEASE_THREAD_STATE; - return 0; + return retval; } static void @@ -1957,8 +2040,11 @@ libvirt_virEventUpdateTimeoutFunc(int timer, int timeout) 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); + result = PyEval_CallObject(updateTimeoutObj, pyobj_args); + if (!result) { + PyErr_Print(); + PyErr_Clear(); + } Py_XDECREF(result); Py_DECREF(pyobj_args); @@ -1971,45 +2057,85 @@ libvirt_virEventRemoveTimeoutFunc(int timer) { PyObject *result = NULL; PyObject *pyobj_args; + PyObject *opaque; + PyObject *ff; + int retval = -1; + virFreeCallback cff; LIBVIRT_ENSURE_THREAD_STATE; pyobj_args = PyTuple_New(1); PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(timer)); - if(updateTimeoutObj && PyCallable_Check(updateTimeoutObj)) - result = PyEval_CallObject(removeTimeoutObj, pyobj_args); + result = PyEval_CallObject(removeTimeoutObj, pyobj_args); + if (!result) { + PyErr_Print(); + PyErr_Clear(); + } else if (!PyTuple_Check(result) || PyTuple_Size(result) != 3) { +#if DEBUG_ERROR + printf("%s: %s must return opaque obj registered with %s" + "to avoid leaking libvirt memory\n", + __FUNCTION__, NAME(removeTimeout), NAME(addTimeout)); +#endif + } else { + opaque = PyTuple_GetItem(result, 1); + ff = PyTuple_GetItem(result, 2); + cff = PyvirFreeCallback_Get(ff); + if (cff) + (*cff)(PyvirVoidPtr_Get(opaque)); + retval = 0; + } Py_XDECREF(result); Py_DECREF(pyobj_args); LIBVIRT_RELEASE_THREAD_STATE; - return 0; + return retval; } static PyObject * libvirt_virEventRegisterImpl(ATTRIBUTE_UNUSED PyObject * self, PyObject * args) { + /* Unref the previously-registered impl (if any) */ Py_XDECREF(addHandleObj); + free(addHandleName); Py_XDECREF(updateHandleObj); + free(updateHandleName); Py_XDECREF(removeHandleObj); + free(removeHandleName); Py_XDECREF(addTimeoutObj); + free(addTimeoutName); Py_XDECREF(updateTimeoutObj); + free(updateTimeoutName); Py_XDECREF(removeTimeoutObj); + free(removeTimeoutName); - if (!PyArg_ParseTuple - (args, (char *) "OOOOOO:virEventRegisterImpl", - &addHandleObj, - &updateHandleObj, - &removeHandleObj, - &addTimeoutObj, - &updateTimeoutObj, - &removeTimeoutObj - )) + /* Parse and check arguments */ + if (!PyArg_ParseTuple(args, (char *) "OOOOOO:virEventRegisterImpl", + &addHandleObj, &updateHandleObj, + &removeHandleObj, &addTimeoutObj, + &updateTimeoutObj, &removeTimeoutObj) || + !PyCallable_Check(addHandleObj) || + !PyCallable_Check(updateHandleObj) || + !PyCallable_Check(removeHandleObj) || + !PyCallable_Check(addTimeoutObj) || + !PyCallable_Check(updateTimeoutObj) || + !PyCallable_Check(removeTimeoutObj)) return VIR_PY_INT_FAIL; + /* Get argument string representations (for error reporting) */ + addHandleName = py_str(addTimeoutObj); + updateHandleName = py_str(updateHandleObj); + removeHandleName = py_str(removeHandleObj); + addTimeoutName = py_str(addTimeoutObj); + updateTimeoutName = py_str(updateTimeoutObj); + removeTimeoutName = py_str(removeTimeoutObj); + + /* Inc refs since we're holding onto these objects until + * the next call (if any) to this function. + */ Py_INCREF(addHandleObj); Py_INCREF(updateHandleObj); Py_INCREF(removeHandleObj); @@ -2019,6 +2145,9 @@ libvirt_virEventRegisterImpl(ATTRIBUTE_UNUSED PyObject * self, LIBVIRT_BEGIN_ALLOW_THREADS; + /* Now register our C EventImpl, which will dispatch + * to the Python callbacks passed in as args. + */ virEventRegisterImpl(libvirt_virEventAddHandleFunc, libvirt_virEventUpdateHandleFunc, libvirt_virEventRemoveHandleFunc, diff --git a/python/libvir.py b/python/libvir.py index 86bf422df6..b90f795b8d 100644 --- a/python/libvir.py +++ b/python/libvir.py @@ -126,11 +126,11 @@ def getVersion (name = None): # # Invoke an EventHandle callback # -def eventInvokeHandleCallback (fd, event, callback, opaque): +def eventInvokeHandleCallback (watch, fd, event, callback, opaque): """ Invoke the Event Impl Handle Callback in C """ - libvirtmod.virEventInvokeHandleCallback(fd, event, callback, opaque); + libvirtmod.virEventInvokeHandleCallback(watch, fd, event, callback, opaque); # # Invoke an EventTimeout callback diff --git a/python/libvirt_wrap.h b/python/libvirt_wrap.h index 9bcfc96a12..fa773d2d3c 100644 --- a/python/libvirt_wrap.h +++ b/python/libvirt_wrap.h @@ -91,6 +91,14 @@ typedef struct { virEventTimeoutCallback obj; } PyvirEventTimeoutCallback_Object; +#define PyvirFreeCallback_Get(v) (((v) == Py_None) ? NULL : \ + (((PyvirFreeCallback_Object *)(v))->obj)) + +typedef struct { + PyObject_HEAD + virFreeCallback obj; +} PyvirFreeCallback_Object; + #define PyvirVoidPtr_Get(v) (((v) == Py_None) ? NULL : \ (((PyvirVoidPtr_Object *)(v))->obj)) diff --git a/python/types.c b/python/types.c index 5c4e6ad7c0..c773f3094d 100644 --- a/python/types.c +++ b/python/types.c @@ -216,7 +216,6 @@ libvirt_virFreeCallbackWrap(virFreeCallback node) PyObject *ret; if (node == NULL) { - printf("%s: WARNING - Wrapping None\n", __FUNCTION__); Py_INCREF(Py_None); return (Py_None); } diff --git a/python/virConnect.py b/python/virConnect.py index ec29b33fca..1fdf548a57 100644 --- a/python/virConnect.py +++ b/python/virConnect.py @@ -32,12 +32,12 @@ ret = libvirtmod.virConnectDomainEventRegister(self._o, self) if ret == -1: raise libvirtError ('virConnectDomainEventRegister() failed', conn=self) - def dispatchDomainEventCallbacks(self, dom, event): + def dispatchDomainEventCallbacks(self, dom, event, detail): """Dispatches events to python user domain event callbacks """ try: for cb,opaque in self.domainEventCallbacks.items(): - cb(self,dom,event,opaque) + cb(self,dom,event,detail,opaque) return 0 except AttributeError: pass