Fix python bindings events code (David Lively)

ChangeLog

@@ -1,3 +1,10 @@
+Mon Nov 24 19:27:40 GMT 2008 Dnaiel P. Berrange <berrange@redhat.com>
+
+	* 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 <berrange@redhat.com>
 
 	* tests/Makefile.am, tests/sexpr2xmltest.c,

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))
+    #print "Updating Handle %s %s" % (str(h_fd), str(event))
-    h_fd = fd
     h_events = event
-    mypoll.unregister(watch)
+    mypoll.unregister(h_fd)
-    mypoll.register(watch, myEventHandleTypeToPollEvent(event))
+    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)

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);
+    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);
+    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);
+    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);
+    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);
+    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);
+    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
+    /* Parse and check arguments */
-        (args, (char *) "OOOOOO:virEventRegisterImpl",
+    if (!PyArg_ParseTuple(args, (char *) "OOOOOO:virEventRegisterImpl",
-                        &addHandleObj,
+                          &addHandleObj, &updateHandleObj,
-                        &updateHandleObj,
+                          &removeHandleObj, &addTimeoutObj,
-                        &removeHandleObj,
+                          &updateTimeoutObj, &removeTimeoutObj) ||
-                        &addTimeoutObj,
+        !PyCallable_Check(addHandleObj) ||
-                        &updateTimeoutObj,
+        !PyCallable_Check(updateHandleObj) ||
-                        &removeTimeoutObj
+        !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,

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

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

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

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