[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] PATCH: Misc fixes to events code



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;
 
 /**


-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]