I'm trying out the new events code with virt-manager and stumbled on some minor bugs - The python binding forgot to call LIBVIRT_BEGIN_ALLOW_THREADS, LIBVIRT_END_ALLOW_THREADS, LIBVIRT_ENSURE_THREAD_STATE or LIBVIRT_RELEASE_THREAD_STATE which meant virt-manager crashed and burned in a heap of python interpretor state corruption [1] - The API docs extracted assumes that enums without explicit value assignements started from 1 instead of 0. This caused an off-by-1 error for all the VIR_DOMAIN_EVENT_* constants in the generated python code. I briefly toyed with fixing docs/apibuild.py enum handling but then instead decided to just change libvirt.h to give explicit values. With these two fixes and some hacks to virt-manager, I've got the events triggering display state refreshes. A couple more things I've not fixed but are on the todo list - THe QEMU driver doesn't emit the ADDED or REMOVED events for VMs which virt-manager requires in order to fully avoid polling. - The VIR_EVENT_* constants aren't included in python, because the API docs can't cope with enum value asignments looking like (1 << 0) (ie fails to parse the left-shifts) Regards, Daniel [1] FYI, i'm using the libvirt-glib python binding in virt-manager http://libvirt.org/hg/libvirt-glib/ which is slightly bad too by not doing glib thread locking calls, but we're lucky because libvirt.so doesn't invoke any glib calls itself from the event callback context. Index: python/libvir.c =================================================================== RCS file: /data/cvs/libvirt/python/libvir.c,v retrieving revision 1.43 diff -u -p -r1.43 libvir.c --- python/libvir.c 31 Oct 2008 10:13:45 -0000 1.43 +++ python/libvir.c 4 Nov 2008 19:45:42 -0000 @@ -1537,29 +1537,33 @@ libvirt_virConnectDomainEventCallback(vi PyObject *pyobj_ret; PyObject *pyobj_conn_inst = (PyObject*)opaque; - PyObject *pyobj_dom = libvirt_virDomainPtrWrap(dom); + PyObject *pyobj_dom; PyObject *pyobj_dom_args; PyObject *pyobj_dom_inst; PyObject *dom_class; + int ret = -1; + + LIBVIRT_ENSURE_THREAD_STATE; /* Create a python instance of this virDomainPtr */ + pyobj_dom = libvirt_virDomainPtrWrap(dom); pyobj_dom_args = PyTuple_New(2); if(PyTuple_SetItem(pyobj_dom_args, 0, pyobj_conn_inst)!=0) { printf("%s error creating tuple",__FUNCTION__); - return -1; + goto cleanup; } if(PyTuple_SetItem(pyobj_dom_args, 1, pyobj_dom)!=0) { printf("%s error creating tuple",__FUNCTION__); - return -1; + goto cleanup; } Py_INCREF(pyobj_conn_inst); dom_class = getLibvirtDomainClassObject(); if(!PyClass_Check(dom_class)) { printf("%s dom_class is not a class!\n", __FUNCTION__); - return -1; + goto cleanup; } pyobj_dom_inst = PyInstance_New(dom_class, @@ -1571,7 +1575,7 @@ libvirt_virConnectDomainEventCallback(vi if(!pyobj_dom_inst) { printf("%s Error creating a python instance of virDomain\n", __FUNCTION__); PyErr_Print(); - return -1; + goto cleanup; } /* Call the Callback Dispatcher */ @@ -1586,12 +1590,15 @@ libvirt_virConnectDomainEventCallback(vi if(!pyobj_ret) { printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); PyErr_Print(); - return -1; } else { Py_DECREF(pyobj_ret); - return 0; + ret = 0; } + +cleanup: + LIBVIRT_RELEASE_THREAD_STATE; + return -1; } static PyObject * @@ -1620,10 +1627,14 @@ libvirt_virConnectDomainEventRegister(AT Py_INCREF(pyobj_conn_inst); + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virConnectDomainEventRegister(conn, libvirt_virConnectDomainEventCallback, (void *)pyobj_conn_inst); + LIBVIRT_END_ALLOW_THREADS; + py_retval = libvirt_intWrap(ret); return (py_retval); } @@ -1650,8 +1661,12 @@ libvirt_virConnectDomainEventDeregister( conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virConnectDomainEventDeregister(conn, libvirt_virConnectDomainEventCallback); + LIBVIRT_END_ALLOW_THREADS; + Py_DECREF(pyobj_conn_inst); py_retval = libvirt_intWrap(ret); return (py_retval); @@ -1679,13 +1694,15 @@ libvirt_virEventAddHandleFunc (int fd A PyObject *cb_args; PyObject *pyobj_args; + LIBVIRT_ENSURE_THREAD_STATE; + /* Lookup the python callback */ python_cb = PyDict_GetItemString(getLibvirtDictObject(), "eventInvokeHandleCallback"); if(!python_cb) { printf("%s Error finding eventInvokeHandleCallback\n", __FUNCTION__); PyErr_Print(); - return -1; + goto cleanup; } Py_INCREF(python_cb); @@ -1708,6 +1725,10 @@ libvirt_virEventAddHandleFunc (int fd A Py_XDECREF(result); Py_DECREF(pyobj_args); + +cleanup: + LIBVIRT_RELEASE_THREAD_STATE; + return 0; } @@ -1715,7 +1736,11 @@ static void libvirt_virEventUpdateHandleFunc(int fd, int event) { PyObject *result = NULL; - PyObject *pyobj_args = PyTuple_New(2); + PyObject *pyobj_args; + + LIBVIRT_ENSURE_THREAD_STATE; + + pyobj_args = PyTuple_New(2); PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(fd)); PyTuple_SetItem(pyobj_args, 1, libvirt_intWrap(event)); @@ -1724,13 +1749,20 @@ libvirt_virEventUpdateHandleFunc(int fd, Py_XDECREF(result); Py_DECREF(pyobj_args); + + LIBVIRT_RELEASE_THREAD_STATE; } + static int libvirt_virEventRemoveHandleFunc(int fd) { PyObject *result = NULL; - PyObject *pyobj_args = PyTuple_New(1); + PyObject *pyobj_args; + + LIBVIRT_ENSURE_THREAD_STATE; + + pyobj_args = PyTuple_New(1); PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(fd)); if(removeHandleObj && PyCallable_Check(removeHandleObj)) @@ -1738,6 +1770,9 @@ libvirt_virEventRemoveHandleFunc(int fd) Py_XDECREF(result); Py_DECREF(pyobj_args); + + LIBVIRT_RELEASE_THREAD_STATE; + return 0; } @@ -1754,13 +1789,15 @@ libvirt_virEventAddTimeoutFunc(int timeo PyObject *cb_args; PyObject *pyobj_args; + LIBVIRT_ENSURE_THREAD_STATE; + /* Lookup the python callback */ python_cb = PyDict_GetItemString(getLibvirtDictObject(), "eventInvokeTimeoutCallback"); if(!python_cb) { printf("%s Error finding eventInvokeTimeoutCallback\n", __FUNCTION__); PyErr_Print(); - return -1; + goto cleanup; } Py_INCREF(python_cb); @@ -1783,6 +1820,9 @@ libvirt_virEventAddTimeoutFunc(int timeo Py_XDECREF(result); Py_DECREF(pyobj_args); + +cleanup: + LIBVIRT_RELEASE_THREAD_STATE; return 0; } @@ -1790,7 +1830,11 @@ static void libvirt_virEventUpdateTimeoutFunc(int timer, int timeout) { PyObject *result = NULL; - PyObject *pyobj_args = PyTuple_New(2); + PyObject *pyobj_args; + + LIBVIRT_ENSURE_THREAD_STATE; + + pyobj_args = PyTuple_New(2); PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(timer)); PyTuple_SetItem(pyobj_args, 1, libvirt_intWrap(timeout)); @@ -1799,13 +1843,19 @@ libvirt_virEventUpdateTimeoutFunc(int ti Py_XDECREF(result); Py_DECREF(pyobj_args); + + LIBVIRT_RELEASE_THREAD_STATE; } static int libvirt_virEventRemoveTimeoutFunc(int timer) { PyObject *result = NULL; - PyObject *pyobj_args = PyTuple_New(1); + PyObject *pyobj_args; + + LIBVIRT_ENSURE_THREAD_STATE; + + pyobj_args = PyTuple_New(1); PyTuple_SetItem(pyobj_args, 0, libvirt_intWrap(timer)); if(updateTimeoutObj && PyCallable_Check(updateTimeoutObj)) @@ -1813,6 +1863,9 @@ libvirt_virEventRemoveTimeoutFunc(int ti Py_XDECREF(result); Py_DECREF(pyobj_args); + + LIBVIRT_RELEASE_THREAD_STATE; + return 0; } @@ -1845,6 +1898,8 @@ libvirt_virEventRegisterImpl(ATTRIBUTE_U Py_INCREF(updateTimeoutObj); Py_INCREF(removeTimeoutObj); + LIBVIRT_BEGIN_ALLOW_THREADS; + virEventRegisterImpl(libvirt_virEventAddHandleFunc, libvirt_virEventUpdateHandleFunc, libvirt_virEventRemoveHandleFunc, @@ -1852,6 +1907,8 @@ libvirt_virEventRegisterImpl(ATTRIBUTE_U libvirt_virEventUpdateTimeoutFunc, libvirt_virEventRemoveTimeoutFunc); + LIBVIRT_END_ALLOW_THREADS; + return VIR_PY_INT_SUCCESS; } Index: include/libvirt/libvirt.h.in =================================================================== RCS file: /data/cvs/libvirt/include/libvirt/libvirt.h.in,v retrieving revision 1.57 diff -u -p -r1.57 libvirt.h.in --- include/libvirt/libvirt.h.in 24 Oct 2008 12:05:39 -0000 1.57 +++ include/libvirt/libvirt.h.in 4 Nov 2008 19:45:42 -0000 @@ -1004,14 +1004,14 @@ virDomainPtr virDomainCreateL * a virDomainEventType is emitted during domain lifecycle events */ typedef enum { - VIR_DOMAIN_EVENT_ADDED, - VIR_DOMAIN_EVENT_REMOVED, - VIR_DOMAIN_EVENT_STARTED, - VIR_DOMAIN_EVENT_SUSPENDED, - VIR_DOMAIN_EVENT_RESUMED, - VIR_DOMAIN_EVENT_STOPPED, - VIR_DOMAIN_EVENT_SAVED, - VIR_DOMAIN_EVENT_RESTORED, + VIR_DOMAIN_EVENT_ADDED = 0, + VIR_DOMAIN_EVENT_REMOVED = 1, + VIR_DOMAIN_EVENT_STARTED = 2, + VIR_DOMAIN_EVENT_SUSPENDED = 3, + VIR_DOMAIN_EVENT_RESUMED = 4, + VIR_DOMAIN_EVENT_STOPPED = 5, + VIR_DOMAIN_EVENT_SAVED = 6, + VIR_DOMAIN_EVENT_RESTORED = 7, } virDomainEventType; /**