[libvirt] PATCH: Misc fixes to events code
Ben Guthro
bguthro at virtualiron.com
Tue Nov 4 20:12:16 UTC 2008
These look good to me
+1
Daniel P. Berrange wrote on 11/04/2008 02:56 PM:
> 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;
>
> /**
>
>
More information about the libvir-list
mailing list