[libvirt] [PATCH 00/10] python: Implement missing stream/event ops

Currently it isn't possible to implement the equivalent of 'virsh console' with the python bindings. This series implements the missing functionality required to do so. Cole Robinson (10): python: libvirt-override: use simpler debug python: generator: Don't print warning if nothing to warn about python: Implement bindings for virStreamEventAddCallback python: Implement virStreamSend/Recv python: Implement virStreamSend/RecvAll helpers Promote virEvent*Handle/Timeout to public API events: Correct virEventAddTimeout docs python: Add bindings for virEvent*Handle/Timeout python: events: Fix C->Python handle callback prototype python: Mark event callback wrappers as private daemon/libvirtd.c | 1 - daemon/mdns.c | 1 - examples/domain-events/events-python/event-test.py | 6 +- python/generator.py | 27 +- python/libvirt-override-virConnect.py | 102 ++--- python/libvirt-override-virStream.py | 123 +++++- python/libvirt-override.c | 492 ++++++++++++++----- python/libvirt-override.py | 80 +++- python/typewrappers.c | 14 + python/typewrappers.h | 2 +- src/conf/domain_event.c | 1 - src/conf/domain_event.h | 1 - src/fdstream.c | 1 - src/libvirt_private.syms | 9 - src/libvirt_public.syms | 6 + src/libxl/libxl_driver.c | 1 - src/lxc/lxc_driver.c | 1 - src/network/bridge_driver.c | 1 - src/node_device/node_device_hal.c | 1 - src/node_device/node_device_udev.c | 1 - src/openvz/openvz_driver.c | 1 - src/qemu/qemu_domain.c | 1 - src/qemu/qemu_driver.c | 1 - src/qemu/qemu_monitor.c | 1 - src/remote/remote_driver.c | 1 - src/test/test_driver.c | 1 - src/uml/uml_driver.c | 1 - src/util/event.c | 56 +++ src/util/event.h | 73 --- src/util/util.c | 1 - src/vbox/vbox_tmpl.c | 1 - src/xen/xen_inotify.c | 1 - src/xen/xs_internal.c | 1 - tools/console.c | 1 - 34 files changed, 692 insertions(+), 320 deletions(-) -- 1.7.4.4

In a couple instances we have to mark a debug variable as ATTRIBUTE_UNUSED to avoid warnings. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/libvirt-override.c | 159 ++++++++++++++++----------------------------- 1 files changed, 55 insertions(+), 104 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 61d5b7d..d6582e2 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -28,6 +28,16 @@ extern void initlibvirtmod(void); extern void initcygvirtmod(void); #endif +//#define DEBUG_ERROR 1 + +#if DEBUG_ERROR +# define DEBUG(fmt, ...) \ + printf(fmt, __VA_ARGS__) +#else +# define DEBUG(fmt, ...) \ + do {} while (0) +#endif + /* The two-statement sequence "Py_INCREF(Py_None); return Py_None;" is so common that we encapsulate it here. Now, each use is simply return VIR_PY_NONE; */ @@ -621,10 +631,8 @@ libvirt_virErrorFuncHandler(ATTRIBUTE_UNUSED void *ctx, virErrorPtr err) PyObject *list, *info; PyObject *result; -#ifdef DEBUG_ERROR - printf("libvirt_virErrorFuncHandler(%p, %s, ...) called\n", ctx, - err->message); -#endif + DEBUG("libvirt_virErrorFuncHandler(%p, %s, ...) called\n", ctx, + err->message); if ((err == NULL) || (err->code == VIR_ERR_OK)) return; @@ -671,10 +679,8 @@ libvirt_virRegisterErrorHandler(ATTRIBUTE_UNUSED PyObject * self, &pyobj_ctx)) return (NULL); -#ifdef DEBUG_ERROR - printf("libvirt_virRegisterErrorHandler(%p, %p) called\n", pyobj_ctx, - pyobj_f); -#endif + DEBUG("libvirt_virRegisterErrorHandler(%p, %p) called\n", pyobj_ctx, + pyobj_f); virSetErrorFunc(NULL, libvirt_virErrorFuncHandler); if (libvirt_virPythonErrorFuncHandler != NULL) { @@ -2451,9 +2457,7 @@ getLibvirtModuleObject (void) { /* Bogus (char *) cast for RHEL-5 python API brokenness */ libvirt_module = PyImport_ImportModule((char *)"libvirt"); if(!libvirt_module) { -#if DEBUG_ERROR - printf("%s Error importing libvirt module\n", __FUNCTION__); -#endif + DEBUG("%s Error importing libvirt module\n", __FUNCTION__); PyErr_Print(); return NULL; } @@ -2469,9 +2473,7 @@ getLibvirtDictObject (void) { // PyModule_GetDict returns a borrowed reference libvirt_dict = PyModule_GetDict(getLibvirtModuleObject()); if(!libvirt_dict) { -#if DEBUG_ERROR - printf("%s Error importing libvirt dictionary\n", __FUNCTION__); -#endif + DEBUG("%s Error importing libvirt dictionary\n", __FUNCTION__); PyErr_Print(); return NULL; } @@ -2489,9 +2491,7 @@ getLibvirtDomainClassObject (void) { libvirt_dom_class = PyDict_GetItemString(getLibvirtDictObject(), "virDomain"); if(!libvirt_dom_class) { -#if DEBUG_ERROR - printf("%s Error importing virDomain class\n", __FUNCTION__); -#endif + DEBUG("%s Error importing virDomain class\n", __FUNCTION__); PyErr_Print(); return NULL; } @@ -2528,24 +2528,18 @@ libvirt_virConnectDomainEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, pyobj_dom = libvirt_virDomainPtrWrap(dom); pyobj_dom_args = PyTuple_New(2); if(PyTuple_SetItem(pyobj_dom_args, 0, pyobj_conn_inst)!=0) { -#if DEBUG_ERROR - printf("%s error creating tuple",__FUNCTION__); -#endif + DEBUG("%s error creating tuple",__FUNCTION__); goto cleanup; } if(PyTuple_SetItem(pyobj_dom_args, 1, pyobj_dom)!=0) { -#if DEBUG_ERROR - printf("%s error creating tuple",__FUNCTION__); -#endif + DEBUG("%s error creating tuple",__FUNCTION__); goto cleanup; } Py_INCREF(pyobj_conn_inst); dom_class = getLibvirtDomainClassObject(); if(!PyClass_Check(dom_class)) { -#if DEBUG_ERROR - printf("%s dom_class is not a class!\n", __FUNCTION__); -#endif + DEBUG("%s dom_class is not a class!\n", __FUNCTION__); goto cleanup; } @@ -2556,9 +2550,8 @@ libvirt_virConnectDomainEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, Py_DECREF(pyobj_dom_args); if(!pyobj_dom_inst) { -#if DEBUG_ERROR - printf("%s Error creating a python instance of virDomain\n", __FUNCTION__); -#endif + DEBUG("%s Error creating a python instance of virDomain\n", + __FUNCTION__); PyErr_Print(); goto cleanup; } @@ -2574,9 +2567,7 @@ libvirt_virConnectDomainEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, Py_DECREF(pyobj_dom_inst); if(!pyobj_ret) { -#if DEBUG_ERROR - printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); -#endif + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); PyErr_Print(); } else { Py_DECREF(pyobj_ret); @@ -2603,16 +2594,12 @@ libvirt_virConnectDomainEventRegister(ATTRIBUTE_UNUSED PyObject * self, if (!PyArg_ParseTuple (args, (char *) "OO:virConnectDomainEventRegister", &pyobj_conn, &pyobj_conn_inst)) { -#if DEBUG_ERROR - printf("%s failed parsing tuple\n", __FUNCTION__); -#endif + DEBUG("%s failed parsing tuple\n", __FUNCTION__); return VIR_PY_INT_FAIL; } -#ifdef DEBUG_ERROR - printf("libvirt_virConnectDomainEventRegister(%p %p) called\n", - pyobj_conn, pyobj_conn_inst); -#endif + DEBUG("libvirt_virConnectDomainEventRegister(%p %p) called\n", + pyobj_conn, pyobj_conn_inst); conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); Py_INCREF(pyobj_conn_inst); @@ -2645,9 +2632,7 @@ libvirt_virConnectDomainEventDeregister(ATTRIBUTE_UNUSED PyObject * self, &pyobj_conn, &pyobj_conn_inst)) return (NULL); -#ifdef DEBUG_ERROR - printf("libvirt_virConnectDomainEventDeregister(%p) called\n", pyobj_conn); -#endif + DEBUG("libvirt_virConnectDomainEventDeregister(%p) called\n", pyobj_conn); conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); @@ -2702,19 +2687,16 @@ libvirt_virEventAddHandleFunc (int fd, python_cb = PyDict_GetItemString(getLibvirtDictObject(), "eventInvokeHandleCallback"); if(!python_cb) { -#if DEBUG_ERROR - printf("%s: Error finding eventInvokeHandleCallback\n", __FUNCTION__); -#endif + DEBUG("%s: Error finding eventInvokeHandleCallback\n", __FUNCTION__); 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"); -#endif + char *name ATTRIBUTE_UNUSED; + name = py_str(python_cb); + DEBUG("%s: %s is not callable\n", __FUNCTION__, + name ? name : "libvirt.eventInvokeHandleCallback"); goto cleanup; } Py_INCREF(python_cb); @@ -2740,9 +2722,7 @@ libvirt_virEventAddHandleFunc (int fd, PyErr_Print(); PyErr_Clear(); } else if (!PyInt_Check(result)) { -#if DEBUG_ERROR - printf("%s: %s should return an int\n", __FUNCTION__, NAME(addHandle)); -#endif + DEBUG("%s: %s should return an int\n", __FUNCTION__, NAME(addHandle)); } else { retval = (int)PyInt_AsLong(result); } @@ -2801,11 +2781,9 @@ libvirt_virEventRemoveHandleFunc(int watch) 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 + DEBUG("%s: %s must return opaque obj registered with %s" + "to avoid leaking libvirt memory\n", + __FUNCTION__, NAME(removeHandle), NAME(addHandle)); } else { opaque = PyTuple_GetItem(result, 1); ff = PyTuple_GetItem(result, 2); @@ -2846,19 +2824,16 @@ libvirt_virEventAddTimeoutFunc(int timeout, python_cb = PyDict_GetItemString(getLibvirtDictObject(), "eventInvokeTimeoutCallback"); if(!python_cb) { -#if DEBUG_ERROR - printf("%s: Error finding eventInvokeTimeoutCallback\n", __FUNCTION__); -#endif + DEBUG("%s: Error finding eventInvokeTimeoutCallback\n", __FUNCTION__); 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"); -#endif + char *name ATTRIBUTE_UNUSED; + name = py_str(python_cb); + DEBUG("%s: %s is not callable\n", __FUNCTION__, + name ? name : "libvirt.eventInvokeTimeoutCallback"); goto cleanup; } Py_INCREF(python_cb); @@ -2884,9 +2859,7 @@ libvirt_virEventAddTimeoutFunc(int timeout, PyErr_Print(); PyErr_Clear(); } else if (!PyInt_Check(result)) { -#if DEBUG_ERROR - printf("%s: %s should return an int\n", __FUNCTION__, NAME(addTimeout)); -#endif + DEBUG("%s: %s should return an int\n", __FUNCTION__, NAME(addTimeout)); } else { retval = (int)PyInt_AsLong(result); } @@ -2943,11 +2916,9 @@ libvirt_virEventRemoveTimeoutFunc(int timer) 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 + DEBUG("%s: %s must return opaque obj registered with %s" + "to avoid leaking libvirt memory\n", + __FUNCTION__, NAME(removeTimeout), NAME(addTimeout)); } else { opaque = PyTuple_GetItem(result, 1); ff = PyTuple_GetItem(result, 2); @@ -3127,9 +3098,7 @@ libvirt_virConnectDomainEventLifecycleCallback(virConnectPtr conn ATTRIBUTE_UNUS Py_DECREF(pyobj_dom); if(!pyobj_ret) { -#if DEBUG_ERROR - printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); -#endif + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); PyErr_Print(); } else { Py_DECREF(pyobj_ret); @@ -3173,9 +3142,7 @@ libvirt_virConnectDomainEventGenericCallback(virConnectPtr conn ATTRIBUTE_UNUSED Py_DECREF(pyobj_dom); if(!pyobj_ret) { -#if DEBUG_ERROR - printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); -#endif + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); PyErr_Print(); } else { Py_DECREF(pyobj_ret); @@ -3222,9 +3189,7 @@ libvirt_virConnectDomainEventRTCChangeCallback(virConnectPtr conn ATTRIBUTE_UNUS Py_DECREF(pyobj_dom); if(!pyobj_ret) { -#if DEBUG_ERROR - printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); -#endif + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); PyErr_Print(); } else { Py_DECREF(pyobj_ret); @@ -3271,9 +3236,7 @@ libvirt_virConnectDomainEventWatchdogCallback(virConnectPtr conn ATTRIBUTE_UNUSE Py_DECREF(pyobj_dom); if(!pyobj_ret) { -#if DEBUG_ERROR - printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); -#endif + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); PyErr_Print(); } else { Py_DECREF(pyobj_ret); @@ -3322,9 +3285,7 @@ libvirt_virConnectDomainEventIOErrorCallback(virConnectPtr conn ATTRIBUTE_UNUSED Py_DECREF(pyobj_dom); if(!pyobj_ret) { -#if DEBUG_ERROR - printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); -#endif + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); PyErr_Print(); } else { Py_DECREF(pyobj_ret); @@ -3374,9 +3335,7 @@ libvirt_virConnectDomainEventIOErrorReasonCallback(virConnectPtr conn ATTRIBUTE_ Py_DECREF(pyobj_dom); if(!pyobj_ret) { -#if DEBUG_ERROR - printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); -#endif + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); PyErr_Print(); } else { Py_DECREF(pyobj_ret); @@ -3463,9 +3422,7 @@ libvirt_virConnectDomainEventGraphicsCallback(virConnectPtr conn ATTRIBUTE_UNUSE Py_DECREF(pyobj_dom); if(!pyobj_ret) { -#if DEBUG_ERROR - printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); -#endif + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); PyErr_Print(); } else { Py_DECREF(pyobj_ret); @@ -3541,16 +3498,12 @@ libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject * self, if (!PyArg_ParseTuple (args, (char *) "OOiO:virConnectDomainEventRegisterAny", &pyobj_conn, &pyobj_dom, &eventID, &pyobj_cbData)) { -#if DEBUG_ERROR - printf("%s failed parsing tuple\n", __FUNCTION__); -#endif + DEBUG("%s failed parsing tuple\n", __FUNCTION__); return VIR_PY_INT_FAIL; } -#ifdef DEBUG_ERROR - printf("libvirt_virConnectDomainEventRegister(%p %p %d %p) called\n", + DEBUG("libvirt_virConnectDomainEventRegister(%p %p %d %p) called\n", pyobj_conn, pyobj_dom, eventID, pyobj_cbData); -#endif conn = PyvirConnect_Get(pyobj_conn); if (pyobj_dom == Py_None) dom = NULL; @@ -3622,9 +3575,7 @@ libvirt_virConnectDomainEventDeregisterAny(ATTRIBUTE_UNUSED PyObject * self, &pyobj_conn, &callbackID)) return (NULL); -#ifdef DEBUG_ERROR - printf("libvirt_virConnectDomainEventDeregister(%p) called\n", pyobj_conn); -#endif + DEBUG("libvirt_virConnectDomainEventDeregister(%p) called\n", pyobj_conn); conn = (virConnectPtr) PyvirConnect_Get(pyobj_conn); -- 1.7.4.4

On Wed, Jun 15, 2011 at 09:23:10PM -0400, Cole Robinson wrote:
In a couple instances we have to mark a debug variable as ATTRIBUTE_UNUSED to avoid warnings.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/libvirt-override.c | 159 ++++++++++++++++----------------------------- 1 files changed, 55 insertions(+), 104 deletions(-)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 61d5b7d..d6582e2 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -28,6 +28,16 @@ extern void initlibvirtmod(void); extern void initcygvirtmod(void); #endif
+//#define DEBUG_ERROR 1
/* ... */ instead or #if 0/#endif
+#if DEBUG_ERROR +# define DEBUG(fmt, ...) \ + printf(fmt, __VA_ARGS__) +#else +# define DEBUG(fmt, ...) \ + do {} while (0) +#endif
But except for that ACK, nice cleanup independant from anything else, please push :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 06/16/2011 09:43 AM, Daniel Veillard wrote:
On Wed, Jun 15, 2011 at 09:23:10PM -0400, Cole Robinson wrote:
In a couple instances we have to mark a debug variable as ATTRIBUTE_UNUSED to avoid warnings.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/libvirt-override.c | 159 ++++++++++++++++----------------------------- 1 files changed, 55 insertions(+), 104 deletions(-)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 61d5b7d..d6582e2 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -28,6 +28,16 @@ extern void initlibvirtmod(void); extern void initcygvirtmod(void); #endif
+//#define DEBUG_ERROR 1
/* ... */ instead or #if 0/#endif
+#if DEBUG_ERROR +# define DEBUG(fmt, ...) \ + printf(fmt, __VA_ARGS__) +#else +# define DEBUG(fmt, ...) \ + do {} while (0) +#endif
But except for that ACK, nice cleanup independant from anything else, please push :-)
Daniel
Pushed with the #if 0 change, thanks for the review. - Cole

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/python/generator.py b/python/generator.py index 39c3ca7..6ee0ada 100755 --- a/python/generator.py +++ b/python/generator.py @@ -654,9 +654,11 @@ def buildStubs(): print "Generated %d wrapper functions" % nb_wrap - print "Missing type converters: " - for type in unknown_types.keys(): - print "%s:%d " % (type, len(unknown_types[type])), + if unknown_types: + print "Missing type converters: " + for type in unknown_types.keys(): + print "%s:%d " % (type, len(unknown_types[type])), + print for f in functions_failed: -- 1.7.4.4

On Wed, Jun 15, 2011 at 09:23:11PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/python/generator.py b/python/generator.py index 39c3ca7..6ee0ada 100755 --- a/python/generator.py +++ b/python/generator.py @@ -654,9 +654,11 @@ def buildStubs():
print "Generated %d wrapper functions" % nb_wrap
- print "Missing type converters: " - for type in unknown_types.keys(): - print "%s:%d " % (type, len(unknown_types[type])), + if unknown_types: + print "Missing type converters: " + for type in unknown_types.keys(): + print "%s:%d " % (type, len(unknown_types[type])), + print
ACK, man, that's old code :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 06/16/2011 09:44 AM, Daniel Veillard wrote:
On Wed, Jun 15, 2011 at 09:23:11PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/python/generator.py b/python/generator.py index 39c3ca7..6ee0ada 100755 --- a/python/generator.py +++ b/python/generator.py @@ -654,9 +654,11 @@ def buildStubs():
print "Generated %d wrapper functions" % nb_wrap
- print "Missing type converters: " - for type in unknown_types.keys(): - print "%s:%d " % (type, len(unknown_types[type])), + if unknown_types: + print "Missing type converters: " + for type in unknown_types.keys(): + print "%s:%d " % (type, len(unknown_types[type])), + print
ACK, man, that's old code :-)
:) Thanks, pushed now. - Cole

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 7 +-- python/libvirt-override-virStream.py | 24 ++++++---- python/libvirt-override.c | 81 ++++++++++++++++++++++++++++++++++ python/typewrappers.h | 1 - 4 files changed, 99 insertions(+), 14 deletions(-) diff --git a/python/generator.py b/python/generator.py index 6ee0ada..6dcd203 100755 --- a/python/generator.py +++ b/python/generator.py @@ -197,6 +197,7 @@ skipped_types = { 'virConnectDomainEventWatchdogCallback': "No function types in python", 'virConnectDomainEventIOErrorCallback': "No function types in python", 'virConnectDomainEventGraphicsCallback': "No function types in python", + 'virStreamEventCallback': "No function types in python", 'virEventAddHandleFunc': "No function types in python", } @@ -391,13 +392,10 @@ skip_function = ( 'virConnectDomainEventDeregisterAny', # overridden in virConnect.py 'virSaveLastError', # We have our own python error wrapper 'virFreeError', # Only needed if we use virSaveLastError - 'virStreamEventAddCallback', 'virStreamRecvAll', 'virStreamSendAll', - 'virStreamRef', - 'virStreamFree', - # These have no use for bindings users. + # 'Ref' functions have no use for bindings users. "virConnectRef", "virDomainRef", "virInterfaceRef", @@ -407,6 +405,7 @@ skip_function = ( "virNWFilterRef", "virStoragePoolRef", "virStorageVolRef", + 'virStreamRef', # This functions shouldn't be called via the bindings (and even the docs # contain an explicit warning to that effect). The equivalent should be diff --git a/python/libvirt-override-virStream.py b/python/libvirt-override-virStream.py index f50a7ef..56f1df5 100644 --- a/python/libvirt-override-virStream.py +++ b/python/libvirt-override-virStream.py @@ -9,12 +9,18 @@ libvirtmod.virStreamFree(self._o) self._o = None - def eventAddCallback(self, cb, opaque): - """ """ - try: - self.cb = cb - self.opaque = opaque - ret = libvirtmod.virStreamEventAddCallback(self._o, self) - if ret == -1: raise libvirtError ('virStreamEventAddCallback() failed', conn=self._conn) - except AttributeError: - pass + def dispatchStreamEventCallback(self, events, cbData): + """ + Dispatches events to python user's stream event callbacks + """ + cb = cbData["cb"] + opaque = cbData["opaque"] + + cb(self, events, opaque) + return 0 + + def eventAddCallback(self, events, cb, opaque): + self.cb = cb + cbData = {"stream": self, "cb" : cb, "opaque" : opaque} + ret = libvirtmod.virStreamEventAddCallback(self._o, events, cbData) + if ret == -1: raise libvirtError ('virStreamEventAddCallback() failed') diff --git a/python/libvirt-override.c b/python/libvirt-override.c index d6582e2..7d071fe 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3588,6 +3588,86 @@ libvirt_virConnectDomainEventDeregisterAny(ATTRIBUTE_UNUSED PyObject * self, return (py_retval); } +static void +libvirt_virStreamEventFreeFunc(void *opaque) +{ + PyObject *pyobj_stream = (PyObject*)opaque; + LIBVIRT_ENSURE_THREAD_STATE; + Py_DECREF(pyobj_stream); + LIBVIRT_RELEASE_THREAD_STATE; +} + +static void +libvirt_virStreamEventCallback(virStreamPtr st ATTRIBUTE_UNUSED, + int events, + void *opaque) +{ + PyObject *pyobj_cbData = (PyObject *)opaque; + PyObject *pyobj_stream; + PyObject *pyobj_ret; + PyObject *dictKey; + + LIBVIRT_ENSURE_THREAD_STATE; + + Py_INCREF(pyobj_cbData); + dictKey = libvirt_constcharPtrWrap("stream"); + pyobj_stream = PyDict_GetItem(pyobj_cbData, dictKey); + Py_DECREF(dictKey); + + /* Call the pure python dispatcher */ + pyobj_ret = PyObject_CallMethod(pyobj_stream, + (char *)"dispatchStreamEventCallback", + (char *)"iO", + events, pyobj_cbData); + + Py_DECREF(pyobj_cbData); + + if (!pyobj_ret) { + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); + PyErr_Print(); + } else { + Py_DECREF(pyobj_ret); + } + + LIBVIRT_RELEASE_THREAD_STATE; +} + +static PyObject * +libvirt_virStreamEventAddCallback(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval; + PyObject *pyobj_stream; + PyObject *pyobj_cbData; + virStreamPtr stream; + virStreamEventCallback cb = libvirt_virStreamEventCallback; + int ret; + int events; + + if (!PyArg_ParseTuple(args, (char *) "OiO:virStreamEventAddCallback", + &pyobj_stream, &events, &pyobj_cbData)) { + DEBUG("%s failed to parse tuple\n", __FUNCTION__); + return VIR_PY_INT_FAIL; + } + + DEBUG("libvirt_virStreamEventAddCallback(%p, %d, %p) called\n", + pyobj_stream, events, pyobj_cbData); + stream = PyvirStream_Get(pyobj_stream); + + Py_INCREF(pyobj_cbData); + + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virStreamEventAddCallback(stream, events, cb, pyobj_cbData, + libvirt_virStreamEventFreeFunc); + LIBVIRT_END_ALLOW_THREADS; + + if (ret < 0) { + Py_DECREF(pyobj_cbData); + } + + py_retval = libvirt_intWrap(ret); + return py_retval; +} /************************************************************************ * * @@ -3606,6 +3686,7 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virConnectDomainEventDeregister", libvirt_virConnectDomainEventDeregister, METH_VARARGS, NULL}, {(char *) "virConnectDomainEventRegisterAny", libvirt_virConnectDomainEventRegisterAny, METH_VARARGS, NULL}, {(char *) "virConnectDomainEventDeregisterAny", libvirt_virConnectDomainEventDeregisterAny, METH_VARARGS, NULL}, + {(char *) "virStreamEventAddCallback", libvirt_virStreamEventAddCallback, METH_VARARGS, NULL}, {(char *) "virDomainGetInfo", libvirt_virDomainGetInfo, METH_VARARGS, NULL}, {(char *) "virDomainGetState", libvirt_virDomainGetState, METH_VARARGS, NULL}, {(char *) "virDomainGetBlockInfo", libvirt_virDomainGetBlockInfo, METH_VARARGS, NULL}, diff --git a/python/typewrappers.h b/python/typewrappers.h index cb5e5db..cc98110 100644 --- a/python/typewrappers.h +++ b/python/typewrappers.h @@ -150,7 +150,6 @@ typedef struct { void* obj; } PyvirVoidPtr_Object; - PyObject * libvirt_intWrap(int val); PyObject * libvirt_longWrap(long val); PyObject * libvirt_ulongWrap(unsigned long val); -- 1.7.4.4

On 06/15/2011 09:23 PM, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 7 +-- python/libvirt-override-virStream.py | 24 ++++++---- python/libvirt-override.c | 81 ++++++++++++++++++++++++++++++++++ python/typewrappers.h | 1 - 4 files changed, 99 insertions(+), 14 deletions(-)
diff --git a/python/generator.py b/python/generator.py index 6ee0ada..6dcd203 100755 --- a/python/generator.py +++ b/python/generator.py @@ -197,6 +197,7 @@ skipped_types = { 'virConnectDomainEventWatchdogCallback': "No function types in python", 'virConnectDomainEventIOErrorCallback': "No function types in python", 'virConnectDomainEventGraphicsCallback': "No function types in python", + 'virStreamEventCallback': "No function types in python", 'virEventAddHandleFunc': "No function types in python", }
@@ -391,13 +392,10 @@ skip_function = ( 'virConnectDomainEventDeregisterAny', # overridden in virConnect.py 'virSaveLastError', # We have our own python error wrapper 'virFreeError', # Only needed if we use virSaveLastError - 'virStreamEventAddCallback', 'virStreamRecvAll', 'virStreamSendAll', - 'virStreamRef', - 'virStreamFree',
Just noticed this 'virStreamFree' shouldn't have been removed. I'll squash in that change. - Cole

On Wed, Jun 15, 2011 at 09:23:12PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 7 +-- python/libvirt-override-virStream.py | 24 ++++++---- python/libvirt-override.c | 81 ++++++++++++++++++++++++++++++++++ python/typewrappers.h | 1 - 4 files changed, 99 insertions(+), 14 deletions(-)
That looks fine, though as you pointed out there was one extra remove, ACK once fixed Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 06/16/2011 10:33 AM, Daniel Veillard wrote:
On Wed, Jun 15, 2011 at 09:23:12PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 7 +-- python/libvirt-override-virStream.py | 24 ++++++---- python/libvirt-override.c | 81 ++++++++++++++++++++++++++++++++++ python/typewrappers.h | 1 - 4 files changed, 99 insertions(+), 14 deletions(-)
That looks fine, though as you pointed out there was one extra remove,
ACK once fixed
Pushed with my suggested change. Thanks, Cole

The return values for the python version are different that the C version of virStreamSend: on success we return a string, an error raises an exception, and if the stream would block we return int(-2). We need to do this since strings aren't passed by reference in python. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 9 +++-- python/libvirt-override-virStream.py | 35 +++++++++++++++++++ python/libvirt-override.c | 62 ++++++++++++++++++++++++++++++++++ python/typewrappers.c | 14 ++++++++ python/typewrappers.h | 1 + 5 files changed, 117 insertions(+), 4 deletions(-) diff --git a/python/generator.py b/python/generator.py index 6dcd203..fdc2068 100755 --- a/python/generator.py +++ b/python/generator.py @@ -347,8 +347,6 @@ skip_impl = ( 'virNWFilterGetUUID', 'virNWFilterGetUUIDString', 'virNWFilterLookupByUUID', - 'virStreamRecv', - 'virStreamSend', 'virStoragePoolGetUUID', 'virStoragePoolGetUUIDString', 'virStoragePoolLookupByUUID', @@ -392,8 +390,11 @@ skip_function = ( 'virConnectDomainEventDeregisterAny', # overridden in virConnect.py 'virSaveLastError', # We have our own python error wrapper 'virFreeError', # Only needed if we use virSaveLastError - 'virStreamRecvAll', - 'virStreamSendAll', + + 'virStreamRecvAll', # XXX: Can be written in pure python? + 'virStreamSendAll', # XXX: Can be written in pure python? + 'virStreamRecv', # overridden in libvirt-override-virStream.py + 'virStreamSend', # overridden in libvirt-override-virStream.py # 'Ref' functions have no use for bindings users. "virConnectRef", diff --git a/python/libvirt-override-virStream.py b/python/libvirt-override-virStream.py index 56f1df5..f8a1d0b 100644 --- a/python/libvirt-override-virStream.py +++ b/python/libvirt-override-virStream.py @@ -24,3 +24,38 @@ cbData = {"stream": self, "cb" : cb, "opaque" : opaque} ret = libvirtmod.virStreamEventAddCallback(self._o, events, cbData) if ret == -1: raise libvirtError ('virStreamEventAddCallback() failed') + + def recv(self, nbytes): + """Write a series of bytes to the stream. This method may + block the calling application for an arbitrary amount + of time. + + Errors are not guaranteed to be reported synchronously + with the call, but may instead be delayed until a + subsequent call. + + On success, the received data is returned. On failure, an + exception is raised. If the stream is a NONBLOCK stream and + the request would block, integer -2 is returned. + """ + ret = libvirtmod.virStreamRecv(self._o, nbytes) + if ret == None: raise libvirtError ('virStreamRecv() failed') + return ret + + def send(self, data): + """Write a series of bytes to the stream. This method may + block the calling application for an arbitrary amount + of time. Once an application has finished sending data + it should call virStreamFinish to wait for successful + confirmation from the driver, or detect any error + + This method may not be used if a stream source has been + registered + + Errors are not guaranteed to be reported synchronously + with the call, but may instead be delayed until a + subsequent call. + """ + ret = libvirtmod.virStreamSend(self._o, data, len(data)) + if ret == -1: raise libvirtError ('virStreamSend() failed') + return ret diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 7d071fe..388c937 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3669,6 +3669,66 @@ libvirt_virStreamEventAddCallback(PyObject *self ATTRIBUTE_UNUSED, return py_retval; } +static PyObject * +libvirt_virStreamRecv(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *pyobj_stream; + virStreamPtr stream; + char *buf = NULL; + int ret; + int nbytes; + + if (!PyArg_ParseTuple(args, (char *) "Oi:virStreamRecv", + &pyobj_stream, &nbytes)) { + DEBUG("%s failed to parse tuple\n", __FUNCTION__); + return VIR_PY_NONE; + } + stream = PyvirStream_Get(pyobj_stream); + + if ((buf = malloc(nbytes+1 > 0 ? nbytes+1 : 1)) == NULL) + return VIR_PY_NONE; + + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virStreamRecv(stream, buf, nbytes); + LIBVIRT_END_ALLOW_THREADS; + + buf[ret > -1 ? ret : 0] = '\0'; + DEBUG("StreamRecv ret=%d strlen=%d\n", ret, (int) strlen(buf)); + + if (ret < 0) + return libvirt_intWrap(ret); + return libvirt_charPtrSizeWrap((char *) buf, (Py_ssize_t) ret); +} + +static PyObject * +libvirt_virStreamSend(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval; + PyObject *pyobj_stream; + virStreamPtr stream; + char *data; + int ret; + int nbytes; + + if (!PyArg_ParseTuple(args, (char *) "Ozi:virStreamRecv", + &pyobj_stream, &data, &nbytes)) { + DEBUG("%s failed to parse tuple\n", __FUNCTION__); + return VIR_PY_INT_FAIL; + } + stream = PyvirStream_Get(pyobj_stream); + + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virStreamSend(stream, data, nbytes); + LIBVIRT_END_ALLOW_THREADS; + + DEBUG("StreamSend ret=%d\n", ret); + + py_retval = libvirt_intWrap(ret); + return py_retval; +} + /************************************************************************ * * * The registration stuff * @@ -3687,6 +3747,8 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virConnectDomainEventRegisterAny", libvirt_virConnectDomainEventRegisterAny, METH_VARARGS, NULL}, {(char *) "virConnectDomainEventDeregisterAny", libvirt_virConnectDomainEventDeregisterAny, METH_VARARGS, NULL}, {(char *) "virStreamEventAddCallback", libvirt_virStreamEventAddCallback, METH_VARARGS, NULL}, + {(char *) "virStreamRecv", libvirt_virStreamRecv, METH_VARARGS, NULL}, + {(char *) "virStreamSend", libvirt_virStreamSend, METH_VARARGS, NULL}, {(char *) "virDomainGetInfo", libvirt_virDomainGetInfo, METH_VARARGS, NULL}, {(char *) "virDomainGetState", libvirt_virDomainGetState, METH_VARARGS, NULL}, {(char *) "virDomainGetBlockInfo", libvirt_virDomainGetBlockInfo, METH_VARARGS, NULL}, diff --git a/python/typewrappers.c b/python/typewrappers.c index e39d3cd..b5758b4 100644 --- a/python/typewrappers.c +++ b/python/typewrappers.c @@ -77,6 +77,20 @@ libvirt_ulonglongWrap(unsigned long long val) } PyObject * +libvirt_charPtrSizeWrap(char *str, Py_ssize_t size) +{ + PyObject *ret; + + if (str == NULL) { + Py_INCREF(Py_None); + return (Py_None); + } + ret = PyString_FromStringAndSize(str, size); + free(str); + return (ret); +} + +PyObject * libvirt_charPtrWrap(char *str) { PyObject *ret; diff --git a/python/typewrappers.h b/python/typewrappers.h index cc98110..305d594 100644 --- a/python/typewrappers.h +++ b/python/typewrappers.h @@ -156,6 +156,7 @@ PyObject * libvirt_ulongWrap(unsigned long val); PyObject * libvirt_longlongWrap(long long val); PyObject * libvirt_ulonglongWrap(unsigned long long val); PyObject * libvirt_charPtrWrap(char *str); +PyObject * libvirt_charPtrSizeWrap(char *str, Py_ssize_t size); PyObject * libvirt_constcharPtrWrap(const char *str); PyObject * libvirt_charPtrConstWrap(const char *str); PyObject * libvirt_virConnectPtrWrap(virConnectPtr node); -- 1.7.4.4

On Wed, Jun 15, 2011 at 09:23:13PM -0400, Cole Robinson wrote:
The return values for the python version are different that the C version of virStreamSend: on success we return a string, an error raises an exception, and if the stream would block we return int(-2). We need to do this since strings aren't passed by reference in python.
I find this a bit bizarre, either we return a string or we return an integer, but if we don't have any other way to provide the information Since we provide the wrapper what about returning (code#, "string value") allowing to have only one type on return instead (i.e. just change recv() in the override below)
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 9 +++-- python/libvirt-override-virStream.py | 35 +++++++++++++++++++ python/libvirt-override.c | 62 ++++++++++++++++++++++++++++++++++ python/typewrappers.c | 14 ++++++++ python/typewrappers.h | 1 + 5 files changed, 117 insertions(+), 4 deletions(-)
+ + def recv(self, nbytes): + """Write a series of bytes to the stream. This method may + block the calling application for an arbitrary amount + of time. + + Errors are not guaranteed to be reported synchronously + with the call, but may instead be delayed until a + subsequent call. + + On success, the received data is returned. On failure, an + exception is raised. If the stream is a NONBLOCK stream and + the request would block, integer -2 is returned. + """ + ret = libvirtmod.virStreamRecv(self._o, nbytes) + if ret == None: raise libvirtError ('virStreamRecv() failed') + return ret
otherwise looks fine to me, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 06/16/2011 10:44 AM, Daniel Veillard wrote:
On Wed, Jun 15, 2011 at 09:23:13PM -0400, Cole Robinson wrote:
The return values for the python version are different that the C version of virStreamSend: on success we return a string, an error raises an exception, and if the stream would block we return int(-2). We need to do this since strings aren't passed by reference in python.
I find this a bit bizarre, either we return a string or we return an integer, but if we don't have any other way to provide the information Since we provide the wrapper what about returning (code#, "string value") allowing to have only one type on return instead (i.e. just change recv() in the override below)
Certainly from a C perspective it's bizarre, but with dynamic typing I think this makes the interface much simpler without much confusion. -2 is just a special value here, much like "" means EOF. The code essentially looks like: ret = stream.recv(1024) if ret == "": return # EOL if ret == -2: return # E_WOULDBLOCK And only users of NONBLOCK flag need to handle this case. Even if they forget to do so, their code would likely error quickly if they try to perform any string operations on the returned value. The benefit of returning a tuple is that it makes the multiple return values clear to an API user who doesn't read the docs, however I think it increases the chance of client bugs and makes client code uglier. Users who aren't using the NONBLOCK flag will still have to handle to at least acknowledge the return code which has no meaning in their case. For NONBLOCK users, what do we set the string value to if the error code is -2? - string "": However this is a value with special meaning, and if the user mishandles the error code or outright doesn't check it, handling this value may cause hard do diagnose behavior - None: If the error code is mishandled, this would likely fail in the same way as the original proposal, which is okay. But if the user was simply doing a check like 'if not stringdata:' for an EOL check, they would incorrectly match this case as well, which has the same problem as "" (unlike a value of -2) Thanks, Cole
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 9 +++-- python/libvirt-override-virStream.py | 35 +++++++++++++++++++ python/libvirt-override.c | 62 ++++++++++++++++++++++++++++++++++ python/typewrappers.c | 14 ++++++++ python/typewrappers.h | 1 + 5 files changed, 117 insertions(+), 4 deletions(-)
+ + def recv(self, nbytes): + """Write a series of bytes to the stream. This method may + block the calling application for an arbitrary amount + of time. + + Errors are not guaranteed to be reported synchronously + with the call, but may instead be delayed until a + subsequent call. + + On success, the received data is returned. On failure, an + exception is raised. If the stream is a NONBLOCK stream and + the request would block, integer -2 is returned. + """ + ret = libvirtmod.virStreamRecv(self._o, nbytes) + if ret == None: raise libvirtError ('virStreamRecv() failed') + return ret
otherwise looks fine to me,
Daniel

On Mon, Jun 20, 2011 at 03:28:07PM -0400, Cole Robinson wrote:
On 06/16/2011 10:44 AM, Daniel Veillard wrote:
On Wed, Jun 15, 2011 at 09:23:13PM -0400, Cole Robinson wrote:
The return values for the python version are different that the C version of virStreamSend: on success we return a string, an error raises an exception, and if the stream would block we return int(-2). We need to do this since strings aren't passed by reference in python.
I find this a bit bizarre, either we return a string or we return an integer, but if we don't have any other way to provide the information Since we provide the wrapper what about returning (code#, "string value") allowing to have only one type on return instead (i.e. just change recv() in the override below)
Certainly from a C perspective it's bizarre, but with dynamic typing I think this makes the interface much simpler without much confusion. -2 is just a special value here, much like "" means EOF. The code essentially looks like:
ret = stream.recv(1024) if ret == "": return # EOL if ret == -2: return # E_WOULDBLOCK
And only users of NONBLOCK flag need to handle this case. Even if they forget to do so, their code would likely error quickly if they try to perform any string operations on the returned value.
The benefit of returning a tuple is that it makes the multiple return values clear to an API user who doesn't read the docs, however I think it increases the chance of client bugs and makes client code uglier. Users who aren't using the NONBLOCK flag will still have to handle to at least acknowledge the return code which has no meaning in their case. For NONBLOCK users, what do we set the string value to if the error code is -2?
- string "": However this is a value with special meaning, and if the user mishandles the error code or outright doesn't check it, handling this value may cause hard do diagnose behavior - None: If the error code is mishandled, this would likely fail in the same way as the original proposal, which is okay. But if the user was simply doing a check like 'if not stringdata:' for an EOL check, they would incorrectly match this case as well, which has the same problem as "" (unlike a value of -2)
Okay :-) ACK ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

Pure python implementation. The handler callbacks have been altered a bit compared to the C API: RecvAll doesn't pass length of the data read since that can be trivially obtained from python string objects, and SendAll requires the handler to return the string data to send rather than store the data in a string pointer. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 4 +- python/libvirt-override-virStream.py | 64 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/python/generator.py b/python/generator.py index fdc2068..b73dc57 100755 --- a/python/generator.py +++ b/python/generator.py @@ -391,8 +391,8 @@ skip_function = ( 'virSaveLastError', # We have our own python error wrapper 'virFreeError', # Only needed if we use virSaveLastError - 'virStreamRecvAll', # XXX: Can be written in pure python? - 'virStreamSendAll', # XXX: Can be written in pure python? + 'virStreamRecvAll', # Pure python libvirt-override-virStream.py + 'virStreamSendAll', # Pure python libvirt-override-virStream.py 'virStreamRecv', # overridden in libvirt-override-virStream.py 'virStreamSend', # overridden in libvirt-override-virStream.py diff --git a/python/libvirt-override-virStream.py b/python/libvirt-override-virStream.py index f8a1d0b..82e1648 100644 --- a/python/libvirt-override-virStream.py +++ b/python/libvirt-override-virStream.py @@ -25,6 +25,70 @@ ret = libvirtmod.virStreamEventAddCallback(self._o, events, cbData) if ret == -1: raise libvirtError ('virStreamEventAddCallback() failed') + def recvAll(self, handler, opaque): + """Receive the entire data stream, sending the data to the + requested data sink. This is simply a convenient alternative + to virStreamRecv, for apps that do blocking-I/o. + + A hypothetical handler function looks like: + + def handler(stream, # virStream instance + buf, # string containing received data + opaque): # extra data passed to recvAll as opaque + fd = opaque + return os.write(fd, buf) + """ + while True: + got = self.recv(1024*64) + if got == -2: + raise libvirtError("cannot use recvAll with " + "nonblocking stream") + if len(got) == 0: + break + + try: + ret = handler(self, got, opaque) + if type(ret) is int and ret < 0: + raise RuntimeError("recvAll handler returned %d" % ret) + except Exception, e: + try: + self.abort() + except: + pass + raise e + + def sendAll(self, handler, opaque): + """ + Send the entire data stream, reading the data from the + requested data source. This is simply a convenient alternative + to virStreamSend, for apps that do blocking-I/o. + + A hypothetical handler function looks like: + + def handler(stream, # virStream instance + nbytes, # int amt of data to read + opaque): # extra data passed to recvAll as opaque + fd = opaque + return os.read(fd, nbytes) + """ + while True: + try: + got = handler(self, 1024*64, opaque) + except: + try: + self.abort() + except: + pass + raise e + + if got == "": + break + + ret = self.send(got) + if ret == -2: + raise libvirtError("cannot use recvAll with " + "nonblocking stream") + def recv(self, nbytes): """Write a series of bytes to the stream. This method may block the calling application for an arbitrary amount -- 1.7.4.4

On Wed, Jun 15, 2011 at 09:23:14PM -0400, Cole Robinson wrote:
Pure python implementation. The handler callbacks have been altered a bit compared to the C API: RecvAll doesn't pass length of the data read since that can be trivially obtained from python string objects, and SendAll requires the handler to return the string data to send rather than store the data in a string pointer.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 4 +- python/libvirt-override-virStream.py | 64 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/python/generator.py b/python/generator.py index fdc2068..b73dc57 100755 --- a/python/generator.py +++ b/python/generator.py @@ -391,8 +391,8 @@ skip_function = ( 'virSaveLastError', # We have our own python error wrapper 'virFreeError', # Only needed if we use virSaveLastError
- 'virStreamRecvAll', # XXX: Can be written in pure python? - 'virStreamSendAll', # XXX: Can be written in pure python? + 'virStreamRecvAll', # Pure python libvirt-override-virStream.py + 'virStreamSendAll', # Pure python libvirt-override-virStream.py 'virStreamRecv', # overridden in libvirt-override-virStream.py 'virStreamSend', # overridden in libvirt-override-virStream.py
diff --git a/python/libvirt-override-virStream.py b/python/libvirt-override-virStream.py index f8a1d0b..82e1648 100644 --- a/python/libvirt-override-virStream.py +++ b/python/libvirt-override-virStream.py @@ -25,6 +25,70 @@ ret = libvirtmod.virStreamEventAddCallback(self._o, events, cbData) if ret == -1: raise libvirtError ('virStreamEventAddCallback() failed')
+ def recvAll(self, handler, opaque): + """Receive the entire data stream, sending the data to the + requested data sink. This is simply a convenient alternative + to virStreamRecv, for apps that do blocking-I/o. + + A hypothetical handler function looks like: + + def handler(stream, # virStream instance + buf, # string containing received data + opaque): # extra data passed to recvAll as opaque + fd = opaque + return os.write(fd, buf) + """ + while True: + got = self.recv(1024*64) + if got == -2: + raise libvirtError("cannot use recvAll with " + "nonblocking stream") + if len(got) == 0: + break + + try: + ret = handler(self, got, opaque) + if type(ret) is int and ret < 0: + raise RuntimeError("recvAll handler returned %d" % ret) + except Exception, e: + try: + self.abort() + except: + pass + raise e + + def sendAll(self, handler, opaque): + """ + Send the entire data stream, reading the data from the + requested data source. This is simply a convenient alternative + to virStreamSend, for apps that do blocking-I/o. + + A hypothetical handler function looks like: + + def handler(stream, # virStream instance + nbytes, # int amt of data to read + opaque): # extra data passed to recvAll as opaque + fd = opaque + return os.read(fd, nbytes) + """ + while True: + try: + got = handler(self, 1024*64, opaque) + except: + try: + self.abort() + except: + pass + raise e + + if got == "": + break + + ret = self.send(got) + if ret == -2: + raise libvirtError("cannot use recvAll with " + "nonblocking stream") + def recv(self, nbytes): """Write a series of bytes to the stream. This method may block the calling application for an arbitrary amount
ACK -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

Since we virEventRegisterDefaultImpl is now a public API, callers need a way to invoke the default registered Handle and Timeout functions. We already have general functions for these internally, so promote them to the public API. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- daemon/libvirtd.c | 1 - daemon/mdns.c | 1 - python/generator.py | 8 ++++ src/conf/domain_event.c | 1 - src/conf/domain_event.h | 1 - src/fdstream.c | 1 - src/libvirt_private.syms | 9 ---- src/libvirt_public.syms | 6 +++ src/libxl/libxl_driver.c | 1 - src/lxc/lxc_driver.c | 1 - src/network/bridge_driver.c | 1 - src/node_device/node_device_hal.c | 1 - src/node_device/node_device_udev.c | 1 - src/openvz/openvz_driver.c | 1 - src/qemu/qemu_domain.c | 1 - src/qemu/qemu_driver.c | 1 - src/qemu/qemu_monitor.c | 1 - src/remote/remote_driver.c | 1 - src/test/test_driver.c | 1 - src/uml/uml_driver.c | 1 - src/util/event.c | 56 +++++++++++++++++++++++++++ src/util/event.h | 73 ------------------------------------ src/util/util.c | 1 - src/vbox/vbox_tmpl.c | 1 - src/xen/xen_inotify.c | 1 - src/xen/xs_internal.c | 1 - tools/console.c | 1 - 27 files changed, 70 insertions(+), 104 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index bcaa37b..5f291ec 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -62,7 +62,6 @@ #include "uuid.h" #include "remote_driver.h" #include "conf.h" -#include "event.h" #include "event_poll.h" #include "memory.h" #include "stream.h" diff --git a/daemon/mdns.c b/daemon/mdns.c index 03695fd..ca4a433 100644 --- a/daemon/mdns.c +++ b/daemon/mdns.c @@ -39,7 +39,6 @@ #include "libvirtd.h" #include "mdns.h" -#include "event.h" #include "event_poll.h" #include "memory.h" diff --git a/python/generator.py b/python/generator.py index b73dc57..745828b 100755 --- a/python/generator.py +++ b/python/generator.py @@ -396,6 +396,14 @@ skip_function = ( 'virStreamRecv', # overridden in libvirt-override-virStream.py 'virStreamSend', # overridden in libvirt-override-virStream.py + # XXX: Skip for now, some work needed to handle Timeout/Handle callbacks + 'virEventAddHandle', + 'virEventRemoveHandle', + 'virEventUpdateHandle', + 'virEventAddTimeout', + 'virEventRemoveTimeout', + 'virEventUpdateTimeout', + # 'Ref' functions have no use for bindings users. "virConnectRef", "virDomainRef", diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index a1f1b0f..785e9e4 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -24,7 +24,6 @@ #include <config.h> #include "domain_event.h" -#include "event.h" #include "logging.h" #include "datatypes.h" #include "memory.h" diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index a80868b..ea481b3 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -25,7 +25,6 @@ #ifndef __DOMAIN_EVENT_H__ # define __DOMAIN_EVENT_H__ -# include "event.h" # include "domain_conf.h" typedef struct _virDomainEventCallback virDomainEventCallback; diff --git a/src/fdstream.c b/src/fdstream.c index e19694f..c1ad787 100644 --- a/src/fdstream.c +++ b/src/fdstream.c @@ -38,7 +38,6 @@ #include "datatypes.h" #include "logging.h" #include "memory.h" -#include "event.h" #include "util.h" #include "files.h" #include "configmake.h" diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5202da3..e12fc04 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -452,15 +452,6 @@ ebtablesContextNew; ebtablesRemoveForwardAllowIn; -# event.h -virEventAddHandle; -virEventAddTimeout; -virEventRemoveHandle; -virEventRemoveTimeout; -virEventUpdateHandle; -virEventUpdateTimeout; - - # event_poll.h virEventPollToNativeEvents; virEventPollFromNativeEvents; diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index f7a6df6..67c7a82 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -458,6 +458,12 @@ LIBVIRT_0.9.3 { virDomainGetBlockPullInfo; virDomainPinVcpuFlags; virDomainSendKey; + virEventAddHandle; + virEventAddTimeout; + virEventRemoveHandle; + virEventRemoveTimeout; + virEventUpdateHandle; + virEventUpdateTimeout; virNodeGetCPUStats; virNodeGetMemoryStats; } LIBVIRT_0.9.2; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 86ed850..5a5951f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -38,7 +38,6 @@ #include "datatypes.h" #include "files.h" #include "memory.h" -#include "event.h" #include "uuid.h" #include "command.h" #include "libxl_driver.h" diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3b0d2a6..d0f7158 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -45,7 +45,6 @@ #include "util.h" #include "bridge.h" #include "veth.h" -#include "event.h" #include "nodeinfo.h" #include "uuid.h" #include "stats_linux.h" diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5e4b4d9..4b94959 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -48,7 +48,6 @@ #include "bridge_driver.h" #include "network_conf.h" #include "driver.h" -#include "event.h" #include "buf.h" #include "util.h" #include "command.h" diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index a90e777..27fedc9 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -32,7 +32,6 @@ #include "virterror_internal.h" #include "driver.h" #include "datatypes.h" -#include "event.h" #include "memory.h" #include "uuid.h" #include "logging.h" diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 8b9694e..2d4e078 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -37,7 +37,6 @@ #include "uuid.h" #include "util.h" #include "buf.h" -#include "event.h" #define VIR_FROM_THIS VIR_FROM_NODEDEV diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 645e426..c13f346 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -49,7 +49,6 @@ #include "virterror_internal.h" #include "datatypes.h" #include "openvz_driver.h" -#include "event.h" #include "buf.h" #include "util.h" #include "openvz_conf.h" diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5f18ad3..f123c92 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -30,7 +30,6 @@ #include "logging.h" #include "virterror_internal.h" #include "c-ctype.h" -#include "event.h" #include "cpu/cpu.h" #include "ignore-value.h" #include "uuid.h" diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 853c84c..6c739e4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -62,7 +62,6 @@ #include "virterror_internal.h" #include "logging.h" #include "datatypes.h" -#include "event.h" #include "buf.h" #include "util.h" #include "nodeinfo.h" diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1428921..89a3f64 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -32,7 +32,6 @@ #include "qemu_monitor_text.h" #include "qemu_monitor_json.h" #include "qemu_conf.h" -#include "event.h" #include "virterror_internal.h" #include "memory.h" #include "logging.h" diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f4b43e0..3a88aea 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -80,7 +80,6 @@ #include "qemu_protocol.h" #include "memory.h" #include "util.h" -#include "event.h" #include "ignore-value.h" #include "files.h" #include "command.h" diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f522143..6c8b9cf 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -44,7 +44,6 @@ #include "interface_conf.h" #include "domain_conf.h" #include "domain_event.h" -#include "event.h" #include "storage_conf.h" #include "node_device_conf.h" #include "xml.h" diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 1d26422..e557fe8 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -47,7 +47,6 @@ #include "uml_driver.h" #include "uml_conf.h" -#include "event.h" #include "buf.h" #include "util.h" #include "nodeinfo.h" diff --git a/src/util/event.c b/src/util/event.c index 11f025b..4108221 100644 --- a/src/util/event.c +++ b/src/util/event.c @@ -37,6 +37,16 @@ static virEventAddTimeoutFunc addTimeoutImpl = NULL; static virEventUpdateTimeoutFunc updateTimeoutImpl = NULL; static virEventRemoveTimeoutFunc removeTimeoutImpl = NULL; +/** + * virEventAddHandle: register a callback for monitoring file handle events + * + * @fd: file handle to monitor for events + * @events: bitset of events to watch from virEventHandleType constants + * @cb: callback to invoke when an event occurs + * @opaque: user data to pass to callback + * + * returns -1 if the file handle cannot be registered, 0 upon success + */ int virEventAddHandle(int fd, int events, virEventHandleCallback cb, @@ -48,10 +58,25 @@ int virEventAddHandle(int fd, return addHandleImpl(fd, events, cb, opaque, ff); } +/** + * virEventUpdateHandle: change event set for a monitored file handle + * + * @watch: watch whose file handle to update + * @events: bitset of events to watch from virEventHandleType constants + * + * Will not fail if fd exists + */ void virEventUpdateHandle(int watch, int events) { updateHandleImpl(watch, events); } +/** + * virEventRemoveHandle: unregister a callback from a file handle + * + * @watch: watch whose file handle to remove + * + * returns -1 if the file handle was not registered, 0 upon success + */ int virEventRemoveHandle(int watch) { if (!removeHandleImpl) return -1; @@ -59,6 +84,19 @@ int virEventRemoveHandle(int watch) { return removeHandleImpl(watch); } +/** + * virEventAddTimeout: register a callback for a timer event + * + * @frequency: time between events in milliseconds + * @cb: callback to invoke when an event occurs + * @opaque: user data to pass to callback + * + * Setting frequency to -1 will disable the timer. Setting the frequency + * to zero will cause it to fire on every event loop iteration. + * + * returns -1 if the timer cannot be registered, a positive + * integer timer id upon success + */ int virEventAddTimeout(int timeout, virEventTimeoutCallback cb, void *opaque, @@ -69,10 +107,28 @@ int virEventAddTimeout(int timeout, return addTimeoutImpl(timeout, cb, opaque, ff); } +/** + * virEventUpdateTimeoutImpl: change frequency for a timer + * + * @timer: timer id to change + * @frequency: time between events in milliseconds + * + * Setting frequency to -1 will disable the timer. Setting the frequency + * to zero will cause it to fire on every event loop iteration. + * + * Will not fail if timer exists + */ void virEventUpdateTimeout(int timer, int timeout) { updateTimeoutImpl(timer, timeout); } +/** + * virEventRemoveTimeout: unregister a callback for a timer + * + * @timer: the timer id to remove + * + * returns -1 if the timer was not registered, 0 upon success + */ int virEventRemoveTimeout(int timer) { if (!removeTimeoutImpl) return -1; diff --git a/src/util/event.h b/src/util/event.h index 68b06c6..2fef314 100644 --- a/src/util/event.h +++ b/src/util/event.h @@ -24,78 +24,5 @@ #ifndef __VIR_EVENT_H__ # define __VIR_EVENT_H__ # include "internal.h" -/** - * virEventAddHandle: register a callback for monitoring file handle events - * - * @fd: file handle to monitor for events - * @events: bitset of events to watch from virEventHandleType constants - * @cb: callback to invoke when an event occurs - * @opaque: user data to pass to callback - * - * returns -1 if the file handle cannot be registered, 0 upon success - */ -int virEventAddHandle(int fd, int events, - virEventHandleCallback cb, - void *opaque, - virFreeCallback ff); - -/** - * virEventUpdateHandle: change event set for a monitored file handle - * - * @watch: watch whose file handle to update - * @events: bitset of events to watch from virEventHandleType constants - * - * Will not fail if fd exists - */ -void virEventUpdateHandle(int watch, int events); - -/** - * virEventRemoveHandle: unregister a callback from a file handle - * - * @watch: watch whose file handle to remove - * - * returns -1 if the file handle was not registered, 0 upon success - */ -int virEventRemoveHandle(int watch); - -/** - * virEventAddTimeout: register a callback for a timer event - * - * @frequency: time between events in milliseconds - * @cb: callback to invoke when an event occurs - * @opaque: user data to pass to callback - * - * Setting frequency to -1 will disable the timer. Setting the frequency - * to zero will cause it to fire on every event loop iteration. - * - * returns -1 if the timer cannot be registered, a positive - * integer timer id upon success - */ -int virEventAddTimeout(int frequency, - virEventTimeoutCallback cb, - void *opaque, - virFreeCallback ff); - -/** - * virEventUpdateTimeoutImpl: change frequency for a timer - * - * @timer: timer id to change - * @frequency: time between events in milliseconds - * - * Setting frequency to -1 will disable the timer. Setting the frequency - * to zero will cause it to fire on every event loop iteration. - * - * Will not fail if timer exists - */ -void virEventUpdateTimeout(int timer, int frequency); - -/** - * virEventRemoveTimeout: unregister a callback for a timer - * - * @timer: the timer id to remove - * - * returns -1 if the timer was not registered, 0 upon success - */ -int virEventRemoveTimeout(int timer); #endif /* __VIR_EVENT_H__ */ diff --git a/src/util/util.c b/src/util/util.c index df4dfac..9d29fec 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -68,7 +68,6 @@ #include "dirname.h" #include "virterror_internal.h" #include "logging.h" -#include "event.h" #include "buf.h" #include "util.h" #include "memory.h" diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index f2233a5..7fd1200 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -49,7 +49,6 @@ #include "storage_conf.h" #include "storage_file.h" #include "uuid.h" -#include "event.h" #include "memory.h" #include "nodeinfo.h" #include "logging.h" diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index 71bb6be..24b4376 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -31,7 +31,6 @@ #include "datatypes.h" #include "driver.h" #include "memory.h" -#include "event.h" #include "xen_driver.h" #include "conf.h" #include "domain_conf.h" diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index b684d3d..04bf93a 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -29,7 +29,6 @@ #include "datatypes.h" #include "driver.h" #include "memory.h" -#include "event.h" #include "logging.h" #include "uuid.h" #include "xen_driver.h" diff --git a/tools/console.c b/tools/console.c index 0428239..7ca95a3 100644 --- a/tools/console.c +++ b/tools/console.c @@ -43,7 +43,6 @@ # include "memory.h" # include "virterror_internal.h" -# include "event.h" /* ie Ctrl-] as per telnet */ # define CTRL_CLOSE_BRACKET '\35' -- 1.7.4.4

On Wed, Jun 15, 2011 at 09:23:15PM -0400, Cole Robinson wrote:
Since we virEventRegisterDefaultImpl is now a public API, callers need a way to invoke the default registered Handle and Timeout functions. We already have general functions for these internally, so promote them to the public API.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- daemon/libvirtd.c | 1 - daemon/mdns.c | 1 - python/generator.py | 8 ++++ src/conf/domain_event.c | 1 - src/conf/domain_event.h | 1 - src/fdstream.c | 1 - src/libvirt_private.syms | 9 ---- src/libvirt_public.syms | 6 +++ src/libxl/libxl_driver.c | 1 - src/lxc/lxc_driver.c | 1 - src/network/bridge_driver.c | 1 - src/node_device/node_device_hal.c | 1 - src/node_device/node_device_udev.c | 1 - src/openvz/openvz_driver.c | 1 - src/qemu/qemu_domain.c | 1 - src/qemu/qemu_driver.c | 1 - src/qemu/qemu_monitor.c | 1 - src/remote/remote_driver.c | 1 - src/test/test_driver.c | 1 - src/uml/uml_driver.c | 1 - src/util/event.c | 56 +++++++++++++++++++++++++++ src/util/event.h | 73 ------------------------------------ src/util/util.c | 1 - src/vbox/vbox_tmpl.c | 1 - src/xen/xen_inotify.c | 1 - src/xen/xs_internal.c | 1 - tools/console.c | 1 - 27 files changed, 70 insertions(+), 104 deletions(-)
Removed from event.h, but not added to include/libvirt/libvirt.h.in ? Something missing surely Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/event.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/event.c b/src/util/event.c index 4108221..bd781ec 100644 --- a/src/util/event.c +++ b/src/util/event.c @@ -87,11 +87,11 @@ int virEventRemoveHandle(int watch) { /** * virEventAddTimeout: register a callback for a timer event * - * @frequency: time between events in milliseconds + * @timeout: time between events in milliseconds * @cb: callback to invoke when an event occurs * @opaque: user data to pass to callback * - * Setting frequency to -1 will disable the timer. Setting the frequency + * Setting timeout to -1 will disable the timer. Setting the timeout * to zero will cause it to fire on every event loop iteration. * * returns -1 if the timer cannot be registered, a positive -- 1.7.4.4

On Wed, Jun 15, 2011 at 09:23:16PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/util/event.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/event.c b/src/util/event.c index 4108221..bd781ec 100644 --- a/src/util/event.c +++ b/src/util/event.c @@ -87,11 +87,11 @@ int virEventRemoveHandle(int watch) { /** * virEventAddTimeout: register a callback for a timer event * - * @frequency: time between events in milliseconds + * @timeout: time between events in milliseconds * @cb: callback to invoke when an event occurs * @opaque: user data to pass to callback * - * Setting frequency to -1 will disable the timer. Setting the frequency + * Setting timeout to -1 will disable the timer. Setting the timeout * to zero will cause it to fire on every event loop iteration. * * returns -1 if the timer cannot be registered, a positive
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 11 +-- python/libvirt-override.c | 192 +++++++++++++++++++++++++++++++++++++------ python/libvirt-override.py | 54 ++++++++++++ 3 files changed, 221 insertions(+), 36 deletions(-) diff --git a/python/generator.py b/python/generator.py index 745828b..2777c61 100755 --- a/python/generator.py +++ b/python/generator.py @@ -198,7 +198,8 @@ skipped_types = { 'virConnectDomainEventIOErrorCallback': "No function types in python", 'virConnectDomainEventGraphicsCallback': "No function types in python", 'virStreamEventCallback': "No function types in python", - 'virEventAddHandleFunc': "No function types in python", + 'virEventHandleCallback': "No function types in python", + 'virEventTimeoutCallback': "No function types in python", } ####################################################################### @@ -396,14 +397,6 @@ skip_function = ( 'virStreamRecv', # overridden in libvirt-override-virStream.py 'virStreamSend', # overridden in libvirt-override-virStream.py - # XXX: Skip for now, some work needed to handle Timeout/Handle callbacks - 'virEventAddHandle', - 'virEventRemoveHandle', - 'virEventUpdateHandle', - 'virEventAddTimeout', - 'virEventRemoveTimeout', - 'virEventUpdateTimeout', - # 'Ref' functions have no use for bindings users. "virConnectRef", "virDomainRef", diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 388c937..b000718 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -59,7 +59,6 @@ static char *py_str(PyObject *obj) return PyString_AsString(str); } - /************************************************************************ * * * Statistics * @@ -2499,6 +2498,30 @@ getLibvirtDomainClassObject (void) { Py_INCREF(libvirt_dom_class); return libvirt_dom_class; } + +static PyObject * +libvirt_lookupPythonFunc(const char *funcname) +{ + PyObject *python_cb; + + /* Lookup the python callback */ + python_cb = PyDict_GetItemString(getLibvirtDictObject(), funcname); + + if (!python_cb) { + DEBUG("%s: Error finding %s\n", __FUNCTION__, funcname); + PyErr_Print(); + PyErr_Clear(); + return NULL; + } + + if (!PyCallable_Check(python_cb)) { + DEBUG("%s: %s is not callable\n", __FUNCTION__, funcname); + return NULL; + } + + return python_cb; +} + /******************************************* * Domain Events *******************************************/ @@ -2684,19 +2707,8 @@ libvirt_virEventAddHandleFunc (int fd, LIBVIRT_ENSURE_THREAD_STATE; /* Lookup the python callback */ - python_cb = PyDict_GetItemString(getLibvirtDictObject(), - "eventInvokeHandleCallback"); - if(!python_cb) { - DEBUG("%s: Error finding eventInvokeHandleCallback\n", __FUNCTION__); - PyErr_Print(); - PyErr_Clear(); - goto cleanup; - } - if (!PyCallable_Check(python_cb)) { - char *name ATTRIBUTE_UNUSED; - name = py_str(python_cb); - DEBUG("%s: %s is not callable\n", __FUNCTION__, - name ? name : "libvirt.eventInvokeHandleCallback"); + python_cb = libvirt_lookupPythonFunc("eventInvokeHandleCallback"); + if (!python_cb) { goto cleanup; } Py_INCREF(python_cb); @@ -2801,6 +2813,7 @@ libvirt_virEventRemoveHandleFunc(int watch) return retval; } + static int libvirt_virEventAddTimeoutFunc(int timeout, virEventTimeoutCallback cb, @@ -2821,19 +2834,8 @@ libvirt_virEventAddTimeoutFunc(int timeout, LIBVIRT_ENSURE_THREAD_STATE; /* Lookup the python callback */ - python_cb = PyDict_GetItemString(getLibvirtDictObject(), - "eventInvokeTimeoutCallback"); - if(!python_cb) { - DEBUG("%s: Error finding eventInvokeTimeoutCallback\n", __FUNCTION__); - PyErr_Print(); - PyErr_Clear(); - goto cleanup; - } - if (!PyCallable_Check(python_cb)) { - char *name ATTRIBUTE_UNUSED; - name = py_str(python_cb); - DEBUG("%s: %s is not callable\n", __FUNCTION__, - name ? name : "libvirt.eventInvokeTimeoutCallback"); + python_cb = libvirt_lookupPythonFunc("eventInvokeTimeoutCallback"); + if (!python_cb) { goto cleanup; } Py_INCREF(python_cb); @@ -3051,6 +3053,140 @@ libvirt_virEventInvokeTimeoutCallback(PyObject *self ATTRIBUTE_UNUSED, return VIR_PY_INT_SUCCESS; } +static void +libvirt_virEventHandleCallback(int watch, + int fd, + int events, + void *opaque) +{ + PyObject *pyobj_cbData = (PyObject *)opaque; + PyObject *pyobj_ret; + PyObject *python_cb; + + LIBVIRT_ENSURE_THREAD_STATE; + + /* Lookup the python callback */ + python_cb = libvirt_lookupPythonFunc("_dispatchEventHandleCallback"); + if (!python_cb) { + goto cleanup; + } + + Py_INCREF(pyobj_cbData); + + /* Call the pure python dispatcher */ + pyobj_ret = PyObject_CallFunction(python_cb, + (char *)"iiiO", + watch, fd, events, pyobj_cbData); + + Py_DECREF(pyobj_cbData); + + if (!pyobj_ret) { + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); + PyErr_Print(); + } else { + Py_DECREF(pyobj_ret); + } + +cleanup: + LIBVIRT_RELEASE_THREAD_STATE; +} + +static PyObject * +libvirt_virEventAddHandle(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval; + PyObject *pyobj_cbData; + virEventHandleCallback cb = libvirt_virEventHandleCallback; + int events; + int fd; + int ret; + + if (!PyArg_ParseTuple(args, (char *) "iiO:virEventAddHandle", + &fd, &events, &pyobj_cbData)) { + DEBUG("%s failed to parse tuple\n", __FUNCTION__); + return VIR_PY_INT_FAIL; + } + + Py_INCREF(pyobj_cbData); + + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virEventAddHandle(fd, events, cb, pyobj_cbData, NULL); + LIBVIRT_END_ALLOW_THREADS; + + if (ret < 0) { + Py_DECREF(pyobj_cbData); + } + + py_retval = libvirt_intWrap(ret); + return py_retval; +} + +static void +libvirt_virEventTimeoutCallback(int timer, + void *opaque) +{ + PyObject *pyobj_cbData = (PyObject *)opaque; + PyObject *pyobj_ret; + PyObject *python_cb; + + LIBVIRT_ENSURE_THREAD_STATE; + + /* Lookup the python callback */ + python_cb = libvirt_lookupPythonFunc("_dispatchEventTimeoutCallback"); + if (!python_cb) { + goto cleanup; + } + + Py_INCREF(pyobj_cbData); + + /* Call the pure python dispatcher */ + pyobj_ret = PyObject_CallFunction(python_cb, + (char *)"iO", + timer, pyobj_cbData); + + Py_DECREF(pyobj_cbData); + + if (!pyobj_ret) { + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); + PyErr_Print(); + } else { + Py_DECREF(pyobj_ret); + } + +cleanup: + LIBVIRT_RELEASE_THREAD_STATE; +} + +static PyObject * +libvirt_virEventAddTimeout(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval; + PyObject *pyobj_cbData; + virEventTimeoutCallback cb = libvirt_virEventTimeoutCallback; + int timeout; + int ret; + + if (!PyArg_ParseTuple(args, (char *) "iO:virEventAddTimeout", + &timeout, &pyobj_cbData)) { + DEBUG("%s failed to parse tuple\n", __FUNCTION__); + return VIR_PY_INT_FAIL; + } + + Py_INCREF(pyobj_cbData); + + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virEventAddTimeout(timeout, cb, pyobj_cbData, NULL); + LIBVIRT_END_ALLOW_THREADS; + + if (ret < 0) { + Py_DECREF(pyobj_cbData); + } + + py_retval = libvirt_intWrap(ret); + return py_retval; +} static void libvirt_virConnectDomainEventFreeFunc(void *opaque) @@ -3789,6 +3925,8 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virStoragePoolGetUUIDString", libvirt_virStoragePoolGetUUIDString, METH_VARARGS, NULL}, {(char *) "virStoragePoolLookupByUUID", libvirt_virStoragePoolLookupByUUID, METH_VARARGS, NULL}, {(char *) "virEventRegisterImpl", libvirt_virEventRegisterImpl, METH_VARARGS, NULL}, + {(char *) "virEventAddHandle", libvirt_virEventAddHandle, METH_VARARGS, NULL}, + {(char *) "virEventAddTimeout", libvirt_virEventAddTimeout, METH_VARARGS, NULL}, {(char *) "virEventInvokeHandleCallback", libvirt_virEventInvokeHandleCallback, METH_VARARGS, NULL}, {(char *) "virEventInvokeTimeoutCallback", libvirt_virEventInvokeTimeoutCallback, METH_VARARGS, NULL}, {(char *) "virNodeListDevices", libvirt_virNodeListDevices, METH_VARARGS, NULL}, diff --git a/python/libvirt-override.py b/python/libvirt-override.py index d544a0e..b611ca4 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -131,3 +131,57 @@ def eventInvokeTimeoutCallback (timer, callback, opaque): Invoke the Event Impl Timeout Callback in C """ libvirtmod.virEventInvokeTimeoutCallback(timer, callback, opaque); + +def _dispatchEventHandleCallback(watch, fd, events, cbData): + cb = cbData["cb"] + opaque = cbData["opaque"] + + cb(watch, fd, events, opaque) + return 0 + +def _dispatchEventTimeoutCallback(timer, cbData): + cb = cbData["cb"] + opaque = cbData["opaque"] + + cb(timer, opaque) + return 0 + +def virEventAddHandle(fd, events, cb, opaque): + """ + register a callback for monitoring file handle events + + @fd: file handle to monitor for events + @events: bitset of events to watch from virEventHandleType constants + @cb: callback to invoke when an event occurs + @opaque: user data to pass to callback + + Example callback prototype is: + def cb(watch, # int id of the handle + fd, # int file descriptor the event occured on + events, # int bitmap of events that have occured + opaque): # opaque data passed to eventAddHandle + """ + cbData = {"cb" : cb, "opaque" : opaque} + ret = libvirtmod.virEventAddHandle(fd, events, cbData) + if ret == -1: raise libvirtError ('virEventAddHandle() failed') + return ret + +def virEventAddTimeout(timeout, cb, opaque): + """ + register a callback for a timer event + + @timeout: time between events in milliseconds + @cb: callback to invoke when an event occurs + @opaque: user data to pass to callback + + Setting timeout to -1 will disable the timer. Setting the timeout + to zero will cause it to fire on every event loop iteration. + + Example callback prototype is: + def cb(timer, # int id of the timer + opaque): # opaque data passed to eventAddTimeout + """ + cbData = {"cb" : cb, "opaque" : opaque} + ret = libvirtmod.virEventAddTimeout(timeout, cbData) + if ret == -1: raise libvirtError ('virEventAddTimeout() failed') + return ret -- 1.7.4.4

On Wed, Jun 15, 2011 at 09:23:17PM -0400, Cole Robinson wrote:
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/generator.py | 11 +-- python/libvirt-override.c | 192 +++++++++++++++++++++++++++++++++++++------ python/libvirt-override.py | 54 ++++++++++++ 3 files changed, 221 insertions(+), 36 deletions(-)
diff --git a/python/generator.py b/python/generator.py index 745828b..2777c61 100755 --- a/python/generator.py +++ b/python/generator.py @@ -198,7 +198,8 @@ skipped_types = { 'virConnectDomainEventIOErrorCallback': "No function types in python", 'virConnectDomainEventGraphicsCallback': "No function types in python", 'virStreamEventCallback': "No function types in python", - 'virEventAddHandleFunc': "No function types in python", + 'virEventHandleCallback': "No function types in python", + 'virEventTimeoutCallback': "No function types in python", }
####################################################################### @@ -396,14 +397,6 @@ skip_function = ( 'virStreamRecv', # overridden in libvirt-override-virStream.py 'virStreamSend', # overridden in libvirt-override-virStream.py
- # XXX: Skip for now, some work needed to handle Timeout/Handle callbacks - 'virEventAddHandle', - 'virEventRemoveHandle', - 'virEventUpdateHandle', - 'virEventAddTimeout', - 'virEventRemoveTimeout', - 'virEventUpdateTimeout', - # 'Ref' functions have no use for bindings users. "virConnectRef", "virDomainRef", diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 388c937..b000718 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -59,7 +59,6 @@ static char *py_str(PyObject *obj) return PyString_AsString(str); }
- /************************************************************************ * * * Statistics * @@ -2499,6 +2498,30 @@ getLibvirtDomainClassObject (void) { Py_INCREF(libvirt_dom_class); return libvirt_dom_class; } + +static PyObject * +libvirt_lookupPythonFunc(const char *funcname) +{ + PyObject *python_cb; + + /* Lookup the python callback */ + python_cb = PyDict_GetItemString(getLibvirtDictObject(), funcname); + + if (!python_cb) { + DEBUG("%s: Error finding %s\n", __FUNCTION__, funcname); + PyErr_Print(); + PyErr_Clear(); + return NULL; + } + + if (!PyCallable_Check(python_cb)) { + DEBUG("%s: %s is not callable\n", __FUNCTION__, funcname); + return NULL; + } + + return python_cb; +} + /******************************************* * Domain Events *******************************************/ @@ -2684,19 +2707,8 @@ libvirt_virEventAddHandleFunc (int fd, LIBVIRT_ENSURE_THREAD_STATE;
/* Lookup the python callback */ - python_cb = PyDict_GetItemString(getLibvirtDictObject(), - "eventInvokeHandleCallback"); - if(!python_cb) { - DEBUG("%s: Error finding eventInvokeHandleCallback\n", __FUNCTION__); - PyErr_Print(); - PyErr_Clear(); - goto cleanup; - } - if (!PyCallable_Check(python_cb)) { - char *name ATTRIBUTE_UNUSED; - name = py_str(python_cb); - DEBUG("%s: %s is not callable\n", __FUNCTION__, - name ? name : "libvirt.eventInvokeHandleCallback"); + python_cb = libvirt_lookupPythonFunc("eventInvokeHandleCallback"); + if (!python_cb) { goto cleanup; } Py_INCREF(python_cb); @@ -2801,6 +2813,7 @@ libvirt_virEventRemoveHandleFunc(int watch) return retval; }
+ static int libvirt_virEventAddTimeoutFunc(int timeout, virEventTimeoutCallback cb, @@ -2821,19 +2834,8 @@ libvirt_virEventAddTimeoutFunc(int timeout, LIBVIRT_ENSURE_THREAD_STATE;
/* Lookup the python callback */ - python_cb = PyDict_GetItemString(getLibvirtDictObject(), - "eventInvokeTimeoutCallback"); - if(!python_cb) { - DEBUG("%s: Error finding eventInvokeTimeoutCallback\n", __FUNCTION__); - PyErr_Print(); - PyErr_Clear(); - goto cleanup; - } - if (!PyCallable_Check(python_cb)) { - char *name ATTRIBUTE_UNUSED; - name = py_str(python_cb); - DEBUG("%s: %s is not callable\n", __FUNCTION__, - name ? name : "libvirt.eventInvokeTimeoutCallback"); + python_cb = libvirt_lookupPythonFunc("eventInvokeTimeoutCallback"); + if (!python_cb) { goto cleanup; } Py_INCREF(python_cb); @@ -3051,6 +3053,140 @@ libvirt_virEventInvokeTimeoutCallback(PyObject *self ATTRIBUTE_UNUSED, return VIR_PY_INT_SUCCESS; }
+static void +libvirt_virEventHandleCallback(int watch, + int fd, + int events, + void *opaque) +{ + PyObject *pyobj_cbData = (PyObject *)opaque; + PyObject *pyobj_ret; + PyObject *python_cb; + + LIBVIRT_ENSURE_THREAD_STATE; + + /* Lookup the python callback */ + python_cb = libvirt_lookupPythonFunc("_dispatchEventHandleCallback"); + if (!python_cb) { + goto cleanup; + } + + Py_INCREF(pyobj_cbData); + + /* Call the pure python dispatcher */ + pyobj_ret = PyObject_CallFunction(python_cb, + (char *)"iiiO", + watch, fd, events, pyobj_cbData); + + Py_DECREF(pyobj_cbData); + + if (!pyobj_ret) { + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); + PyErr_Print(); + } else { + Py_DECREF(pyobj_ret); + } + +cleanup: + LIBVIRT_RELEASE_THREAD_STATE; +} + +static PyObject * +libvirt_virEventAddHandle(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval; + PyObject *pyobj_cbData; + virEventHandleCallback cb = libvirt_virEventHandleCallback; + int events; + int fd; + int ret; + + if (!PyArg_ParseTuple(args, (char *) "iiO:virEventAddHandle", + &fd, &events, &pyobj_cbData)) { + DEBUG("%s failed to parse tuple\n", __FUNCTION__); + return VIR_PY_INT_FAIL; + } + + Py_INCREF(pyobj_cbData); + + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virEventAddHandle(fd, events, cb, pyobj_cbData, NULL); + LIBVIRT_END_ALLOW_THREADS; + + if (ret < 0) { + Py_DECREF(pyobj_cbData); + } + + py_retval = libvirt_intWrap(ret); + return py_retval; +} + +static void +libvirt_virEventTimeoutCallback(int timer, + void *opaque) +{ + PyObject *pyobj_cbData = (PyObject *)opaque; + PyObject *pyobj_ret; + PyObject *python_cb; + + LIBVIRT_ENSURE_THREAD_STATE; + + /* Lookup the python callback */ + python_cb = libvirt_lookupPythonFunc("_dispatchEventTimeoutCallback"); + if (!python_cb) { + goto cleanup; + } + + Py_INCREF(pyobj_cbData); + + /* Call the pure python dispatcher */ + pyobj_ret = PyObject_CallFunction(python_cb, + (char *)"iO", + timer, pyobj_cbData); + + Py_DECREF(pyobj_cbData); + + if (!pyobj_ret) { + DEBUG("%s - ret:%p\n", __FUNCTION__, pyobj_ret); + PyErr_Print(); + } else { + Py_DECREF(pyobj_ret); + } + +cleanup: + LIBVIRT_RELEASE_THREAD_STATE; +} + +static PyObject * +libvirt_virEventAddTimeout(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + PyObject *py_retval; + PyObject *pyobj_cbData; + virEventTimeoutCallback cb = libvirt_virEventTimeoutCallback; + int timeout; + int ret; + + if (!PyArg_ParseTuple(args, (char *) "iO:virEventAddTimeout", + &timeout, &pyobj_cbData)) { + DEBUG("%s failed to parse tuple\n", __FUNCTION__); + return VIR_PY_INT_FAIL; + } + + Py_INCREF(pyobj_cbData); + + LIBVIRT_BEGIN_ALLOW_THREADS; + ret = virEventAddTimeout(timeout, cb, pyobj_cbData, NULL); + LIBVIRT_END_ALLOW_THREADS; + + if (ret < 0) { + Py_DECREF(pyobj_cbData); + } + + py_retval = libvirt_intWrap(ret); + return py_retval; +}
static void libvirt_virConnectDomainEventFreeFunc(void *opaque) @@ -3789,6 +3925,8 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virStoragePoolGetUUIDString", libvirt_virStoragePoolGetUUIDString, METH_VARARGS, NULL}, {(char *) "virStoragePoolLookupByUUID", libvirt_virStoragePoolLookupByUUID, METH_VARARGS, NULL}, {(char *) "virEventRegisterImpl", libvirt_virEventRegisterImpl, METH_VARARGS, NULL}, + {(char *) "virEventAddHandle", libvirt_virEventAddHandle, METH_VARARGS, NULL}, + {(char *) "virEventAddTimeout", libvirt_virEventAddTimeout, METH_VARARGS, NULL}, {(char *) "virEventInvokeHandleCallback", libvirt_virEventInvokeHandleCallback, METH_VARARGS, NULL}, {(char *) "virEventInvokeTimeoutCallback", libvirt_virEventInvokeTimeoutCallback, METH_VARARGS, NULL}, {(char *) "virNodeListDevices", libvirt_virNodeListDevices, METH_VARARGS, NULL}, diff --git a/python/libvirt-override.py b/python/libvirt-override.py index d544a0e..b611ca4 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -131,3 +131,57 @@ def eventInvokeTimeoutCallback (timer, callback, opaque): Invoke the Event Impl Timeout Callback in C """ libvirtmod.virEventInvokeTimeoutCallback(timer, callback, opaque); + +def _dispatchEventHandleCallback(watch, fd, events, cbData): + cb = cbData["cb"] + opaque = cbData["opaque"] + + cb(watch, fd, events, opaque) + return 0 + +def _dispatchEventTimeoutCallback(timer, cbData): + cb = cbData["cb"] + opaque = cbData["opaque"] + + cb(timer, opaque) + return 0 + +def virEventAddHandle(fd, events, cb, opaque): + """ + register a callback for monitoring file handle events + + @fd: file handle to monitor for events + @events: bitset of events to watch from virEventHandleType constants + @cb: callback to invoke when an event occurs + @opaque: user data to pass to callback + + Example callback prototype is: + def cb(watch, # int id of the handle + fd, # int file descriptor the event occured on + events, # int bitmap of events that have occured + opaque): # opaque data passed to eventAddHandle + """ + cbData = {"cb" : cb, "opaque" : opaque} + ret = libvirtmod.virEventAddHandle(fd, events, cbData) + if ret == -1: raise libvirtError ('virEventAddHandle() failed') + return ret + +def virEventAddTimeout(timeout, cb, opaque): + """ + register a callback for a timer event + + @timeout: time between events in milliseconds + @cb: callback to invoke when an event occurs + @opaque: user data to pass to callback + + Setting timeout to -1 will disable the timer. Setting the timeout + to zero will cause it to fire on every event loop iteration. + + Example callback prototype is: + def cb(timer, # int id of the timer + opaque): # opaque data passed to eventAddTimeout + """ + cbData = {"cb" : cb, "opaque" : opaque} + ret = libvirtmod.virEventAddTimeout(timeout, cbData) + if ret == -1: raise libvirtError ('virEventAddTimeout() failed') + return ret -- 1.7.4.4
Looks fine, libvirt_lookupPythonFunc() is a nice cleanup. It's a bit hard to track the Py_DECREF/Py_INCREF, hopefully they are fine in all cases (errro paths are hard for this), ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

If registering our own event loop implementation written in python, any handles or timeouts callbacks registered by libvirt C code must are wrapped in a python function. There is some argument trickery that makes this all work, by wrapping the user passed opaque value in a tuple, along with the callback function. Problem is, the current setup requires the user's event loop to know about this trickery, rather than just treating the opaque value as truly opaque. Fix this in a backwards compatible manner, and adjust the example python event loop to do things the proper way. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- examples/domain-events/events-python/event-test.py | 6 +--- python/libvirt-override.py | 26 ++++++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py index df75dce..76fda2b 100644 --- a/examples/domain-events/events-python/event-test.py +++ b/examples/domain-events/events-python/event-test.py @@ -66,8 +66,7 @@ class virEventLoopPure: self.cb(self.handle, self.fd, events, - self.opaque[0], - self.opaque[1]) + self.opaque) # This class contains the data we need to track for a # single periodic timer @@ -96,8 +95,7 @@ class virEventLoopPure: def dispatch(self): self.cb(self.timer, - self.opaque[0], - self.opaque[1]) + self.opaque) def __init__(self): diff --git a/python/libvirt-override.py b/python/libvirt-override.py index b611ca4..8df9dc1 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -117,19 +117,41 @@ def getVersion (name = None): # # Invoke an EventHandle callback # -def eventInvokeHandleCallback (watch, fd, event, callback, opaque): +def eventInvokeHandleCallback(watch, fd, event, opaque, opaquecompat=None): """ Invoke the Event Impl Handle Callback in C """ + # libvirt 0.9.2 and earlier required custom event loops to know + # that opaque=(cb, original_opaque) and pass the values individually + # to this wrapper. This should handle the back compat case, and make + # future invocations match the virEventHandleCallback prototype + if opaquecompat: + callback = opaque + opaque = opaquecompat + else: + callback = opaque[0] + opaque = opaque[1] + libvirtmod.virEventInvokeHandleCallback(watch, fd, event, callback, opaque); # # Invoke an EventTimeout callback # -def eventInvokeTimeoutCallback (timer, callback, opaque): +def eventInvokeTimeoutCallback(timer, opaque, opaquecompat=None): """ Invoke the Event Impl Timeout Callback in C """ + # libvirt 0.9.2 and earlier required custom event loops to know + # that opaque=(cb, original_opaque) and pass the values individually + # to this wrapper. This should handle the back compat case, and make + # future invocations match the virEventTimeoutCallback prototype + if opaquecompat: + callback = opaque + opaque = opaquecompat + else: + callback = opaque[0] + opaque = opaque[1] + libvirtmod.virEventInvokeTimeoutCallback(timer, callback, opaque); def _dispatchEventHandleCallback(watch, fd, events, cbData): -- 1.7.4.4

On Wed, Jun 15, 2011 at 09:23:18PM -0400, Cole Robinson wrote:
If registering our own event loop implementation written in python, any handles or timeouts callbacks registered by libvirt C code must are wrapped in a python function. There is some argument trickery that
"must be" :-)
makes this all work, by wrapping the user passed opaque value in a tuple, along with the callback function.
Problem is, the current setup requires the user's event loop to know about this trickery, rather than just treating the opaque value as truly opaque.
Fix this in a backwards compatible manner, and adjust the example python event loop to do things the proper way.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- examples/domain-events/events-python/event-test.py | 6 +--- python/libvirt-override.py | 26 ++++++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py index df75dce..76fda2b 100644 --- a/examples/domain-events/events-python/event-test.py +++ b/examples/domain-events/events-python/event-test.py @@ -66,8 +66,7 @@ class virEventLoopPure: self.cb(self.handle, self.fd, events, - self.opaque[0], - self.opaque[1]) + self.opaque)
# This class contains the data we need to track for a # single periodic timer @@ -96,8 +95,7 @@ class virEventLoopPure:
def dispatch(self): self.cb(self.timer, - self.opaque[0], - self.opaque[1]) + self.opaque)
def __init__(self): diff --git a/python/libvirt-override.py b/python/libvirt-override.py index b611ca4..8df9dc1 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -117,19 +117,41 @@ def getVersion (name = None): # # Invoke an EventHandle callback # -def eventInvokeHandleCallback (watch, fd, event, callback, opaque): +def eventInvokeHandleCallback(watch, fd, event, opaque, opaquecompat=None): """ Invoke the Event Impl Handle Callback in C """ + # libvirt 0.9.2 and earlier required custom event loops to know + # that opaque=(cb, original_opaque) and pass the values individually + # to this wrapper. This should handle the back compat case, and make + # future invocations match the virEventHandleCallback prototype + if opaquecompat: + callback = opaque + opaque = opaquecompat + else: + callback = opaque[0] + opaque = opaque[1] + libvirtmod.virEventInvokeHandleCallback(watch, fd, event, callback, opaque);
I would rather use dynamic type test like if type(callback) == type(()) and type(callback[0]) == type(lambda x:x): than rely on an extra argument detection Note: I didn't tried, I don't know if python type will allow the test for functions and lambda to work, otherwise use a predefined function or another one from the module.
# # Invoke an EventTimeout callback # -def eventInvokeTimeoutCallback (timer, callback, opaque): +def eventInvokeTimeoutCallback(timer, opaque, opaquecompat=None): """ Invoke the Event Impl Timeout Callback in C """ + # libvirt 0.9.2 and earlier required custom event loops to know + # that opaque=(cb, original_opaque) and pass the values individually + # to this wrapper. This should handle the back compat case, and make + # future invocations match the virEventTimeoutCallback prototype + if opaquecompat: + callback = opaque + opaque = opaquecompat + else: + callback = opaque[0] + opaque = opaque[1] + libvirtmod.virEventInvokeTimeoutCallback(timer, callback, opaque);
Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 06/19/2011 09:44 PM, Daniel Veillard wrote:
On Wed, Jun 15, 2011 at 09:23:18PM -0400, Cole Robinson wrote:
If registering our own event loop implementation written in python, any handles or timeouts callbacks registered by libvirt C code must are wrapped in a python function. There is some argument trickery that
"must be" :-)
makes this all work, by wrapping the user passed opaque value in a tuple, along with the callback function.
Problem is, the current setup requires the user's event loop to know about this trickery, rather than just treating the opaque value as truly opaque.
Fix this in a backwards compatible manner, and adjust the example python event loop to do things the proper way.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- examples/domain-events/events-python/event-test.py | 6 +--- python/libvirt-override.py | 26 ++++++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py index df75dce..76fda2b 100644 --- a/examples/domain-events/events-python/event-test.py +++ b/examples/domain-events/events-python/event-test.py @@ -66,8 +66,7 @@ class virEventLoopPure: self.cb(self.handle, self.fd, events, - self.opaque[0], - self.opaque[1]) + self.opaque)
# This class contains the data we need to track for a # single periodic timer @@ -96,8 +95,7 @@ class virEventLoopPure:
def dispatch(self): self.cb(self.timer, - self.opaque[0], - self.opaque[1]) + self.opaque)
def __init__(self): diff --git a/python/libvirt-override.py b/python/libvirt-override.py index b611ca4..8df9dc1 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -117,19 +117,41 @@ def getVersion (name = None): # # Invoke an EventHandle callback # -def eventInvokeHandleCallback (watch, fd, event, callback, opaque): +def eventInvokeHandleCallback(watch, fd, event, opaque, opaquecompat=None): """ Invoke the Event Impl Handle Callback in C """ + # libvirt 0.9.2 and earlier required custom event loops to know + # that opaque=(cb, original_opaque) and pass the values individually + # to this wrapper. This should handle the back compat case, and make + # future invocations match the virEventHandleCallback prototype + if opaquecompat: + callback = opaque + opaque = opaquecompat + else: + callback = opaque[0] + opaque = opaque[1] + libvirtmod.virEventInvokeHandleCallback(watch, fd, event, callback, opaque);
I would rather use dynamic type test like if type(callback) == type(()) and type(callback[0]) == type(lambda x:x):
than rely on an extra argument detection Note: I didn't tried, I don't know if python type will allow the test for functions and lambda to work, otherwise use a predefined function or another one from the module.
Part of the problem is we need to preserve the old number of function arguments, so that an old event loop works with newer libvirt, otherwise user will pass 5 args where only 4 allowed. Also we can't only do a dynamic test of the opaque value, since the original opaque value could easily be opaque=(somefunc, somedata) and we would end up incorrectly matching that. So checking opaquecompat has to factor into it in some way, and if we are doing that I don't think there is much benefit to poking into the values, since if they don't match our expectations there isn't much we can do to work around it, so better to just let it error out naturally IMO. Thanks, Cole

On Mon, Jun 20, 2011 at 03:26:18PM -0400, Cole Robinson wrote:
On 06/19/2011 09:44 PM, Daniel Veillard wrote:
On Wed, Jun 15, 2011 at 09:23:18PM -0400, Cole Robinson wrote:
If registering our own event loop implementation written in python, any handles or timeouts callbacks registered by libvirt C code must are wrapped in a python function. There is some argument trickery that
"must be" :-)
makes this all work, by wrapping the user passed opaque value in a tuple, along with the callback function.
Problem is, the current setup requires the user's event loop to know about this trickery, rather than just treating the opaque value as truly opaque.
Fix this in a backwards compatible manner, and adjust the example python event loop to do things the proper way.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- examples/domain-events/events-python/event-test.py | 6 +--- python/libvirt-override.py | 26 ++++++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/examples/domain-events/events-python/event-test.py b/examples/domain-events/events-python/event-test.py index df75dce..76fda2b 100644 --- a/examples/domain-events/events-python/event-test.py +++ b/examples/domain-events/events-python/event-test.py @@ -66,8 +66,7 @@ class virEventLoopPure: self.cb(self.handle, self.fd, events, - self.opaque[0], - self.opaque[1]) + self.opaque)
# This class contains the data we need to track for a # single periodic timer @@ -96,8 +95,7 @@ class virEventLoopPure:
def dispatch(self): self.cb(self.timer, - self.opaque[0], - self.opaque[1]) + self.opaque)
def __init__(self): diff --git a/python/libvirt-override.py b/python/libvirt-override.py index b611ca4..8df9dc1 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -117,19 +117,41 @@ def getVersion (name = None): # # Invoke an EventHandle callback # -def eventInvokeHandleCallback (watch, fd, event, callback, opaque): +def eventInvokeHandleCallback(watch, fd, event, opaque, opaquecompat=None): """ Invoke the Event Impl Handle Callback in C """ + # libvirt 0.9.2 and earlier required custom event loops to know + # that opaque=(cb, original_opaque) and pass the values individually + # to this wrapper. This should handle the back compat case, and make + # future invocations match the virEventHandleCallback prototype + if opaquecompat: + callback = opaque + opaque = opaquecompat + else: + callback = opaque[0] + opaque = opaque[1] + libvirtmod.virEventInvokeHandleCallback(watch, fd, event, callback, opaque);
I would rather use dynamic type test like if type(callback) == type(()) and type(callback[0]) == type(lambda x:x):
than rely on an extra argument detection Note: I didn't tried, I don't know if python type will allow the test for functions and lambda to work, otherwise use a predefined function or another one from the module.
Part of the problem is we need to preserve the old number of function arguments, so that an old event loop works with newer libvirt, otherwise user will pass 5 args where only 4 allowed.
Also we can't only do a dynamic test of the opaque value, since the original opaque value could easily be opaque=(somefunc, somedata) and we would end up incorrectly matching that. So checking opaquecompat has to factor into it in some way, and if we are doing that I don't think there is much benefit to poking into the values, since if they don't match our expectations there isn't much we can do to work around it, so better to just let it error out naturally IMO.
Okay, ACK :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

These functions aren't intended to be called directly by users, so mark them as private. While we're at it, remove unneeded exception handling, and break some long lines. Signed-off-by: Cole Robinson <crobinso@redhat.com> --- python/libvirt-override-virConnect.py | 102 ++++++++++++++------------------- python/libvirt-override-virStream.py | 2 +- python/libvirt-override.c | 24 ++++---- python/libvirt-override.py | 4 +- 4 files changed, 59 insertions(+), 73 deletions(-) diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index 362be75..5be9659 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -33,7 +33,7 @@ ret = libvirtmod.virConnectDomainEventRegister(self._o, self) if ret == -1: raise libvirtError ('virConnectDomainEventRegister() failed', conn=self) - def dispatchDomainEventCallbacks(self, dom, event, detail): + def _dispatchDomainEventCallbacks(self, dom, event, detail): """Dispatches events to python user domain event callbacks """ try: @@ -43,7 +43,7 @@ except AttributeError: pass - def dispatchDomainEventLifecycleCallback(self, dom, event, detail, cbData): + def _dispatchDomainEventLifecycleCallback(self, dom, event, detail, cbData): """Dispatches events to python user domain lifecycle event callbacks """ cb = cbData["cb"] @@ -52,89 +52,75 @@ cb(self, virDomain(self, _obj=dom), event, detail, opaque) return 0 - def dispatchDomainEventGenericCallback(self, dom, cbData): + def _dispatchDomainEventGenericCallback(self, dom, cbData): """Dispatches events to python user domain generic event callbacks """ - try: - cb = cbData["cb"] - opaque = cbData["opaque"] + cb = cbData["cb"] + opaque = cbData["opaque"] - cb(self, virDomain(self, _obj=dom), opaque) - return 0 - except AttributeError: - pass + cb(self, virDomain(self, _obj=dom), opaque) + return 0 - def dispatchDomainEventRTCChangeCallback(self, dom, offset, cbData): + def _dispatchDomainEventRTCChangeCallback(self, dom, offset, cbData): """Dispatches events to python user domain RTC change event callbacks """ - try: - cb = cbData["cb"] - opaque = cbData["opaque"] + cb = cbData["cb"] + opaque = cbData["opaque"] - cb(self, virDomain(self, _obj=dom), offset ,opaque) - return 0 - except AttributeError: - pass + cb(self, virDomain(self, _obj=dom), offset ,opaque) + return 0 - def dispatchDomainEventWatchdogCallback(self, dom, action, cbData): + def _dispatchDomainEventWatchdogCallback(self, dom, action, cbData): """Dispatches events to python user domain watchdog event callbacks """ - try: - cb = cbData["cb"] - opaque = cbData["opaque"] + cb = cbData["cb"] + opaque = cbData["opaque"] - cb(self, virDomain(self, _obj=dom), action, opaque) - return 0 - except AttributeError: - pass + cb(self, virDomain(self, _obj=dom), action, opaque) + return 0 - def dispatchDomainEventIOErrorCallback(self, dom, srcPath, devAlias, action, cbData): + def _dispatchDomainEventIOErrorCallback(self, dom, srcPath, devAlias, + action, cbData): """Dispatches events to python user domain IO error event callbacks """ - try: - cb = cbData["cb"] - opaque = cbData["opaque"] + cb = cbData["cb"] + opaque = cbData["opaque"] - cb(self, virDomain(self, _obj=dom), srcPath, devAlias, action, opaque) - return 0 - except AttributeError: - pass + cb(self, virDomain(self, _obj=dom), srcPath, devAlias, action, opaque) + return 0 - def dispatchDomainEventIOErrorReasonCallback(self, dom, srcPath, devAlias, action, reason, cbData): + def _dispatchDomainEventIOErrorReasonCallback(self, dom, srcPath, + devAlias, action, reason, + cbData): """Dispatches events to python user domain IO error event callbacks """ - try: - cb = cbData["cb"] - opaque = cbData["opaque"] + cb = cbData["cb"] + opaque = cbData["opaque"] - cb(self, virDomain(self, _obj=dom), srcPath, devAlias, action, reason, opaque) - return 0 - except AttributeError: - pass + cb(self, virDomain(self, _obj=dom), srcPath, devAlias, action, + reason, opaque) + return 0 - def dispatchDomainEventGraphicsCallback(self, dom, phase, localAddr, remoteAddr, authScheme, subject, cbData): + def _dispatchDomainEventGraphicsCallback(self, dom, phase, localAddr, + remoteAddr, authScheme, subject, + cbData): """Dispatches events to python user domain graphics event callbacks """ - try: - cb = cbData["cb"] - opaque = cbData["opaque"] + cb = cbData["cb"] + opaque = cbData["opaque"] - cb(self, virDomain(self, _obj=dom), phase, localAddr, remoteAddr, authScheme, subject, opaque) - return 0 - except AttributeError: - pass + cb(self, virDomain(self, _obj=dom), phase, localAddr, remoteAddr, + authScheme, subject, opaque) + return 0 - def dispatchDomainEventBlockPullCallback(self, dom, path, status, cbData): + def _dispatchDomainEventBlockPullCallback(self, dom, path, status, cbData): """Dispatches events to python user domain blockPull event callbacks """ - try: - cb = cbData["cb"] - opaque = cbData["opaque"] + cb = cbData["cb"] + opaque = cbData["opaque"] - cb(self, virDomain(self, _obj=dom), path, status, opaque) - return 0 - except AttributeError: - pass + cb(self, virDomain(self, _obj=dom), path, status, opaque) + return 0 def domainEventDeregisterAny(self, callbackID): """Removes a Domain Event Callback. De-registering for a diff --git a/python/libvirt-override-virStream.py b/python/libvirt-override-virStream.py index 82e1648..92e6fb1 100644 --- a/python/libvirt-override-virStream.py +++ b/python/libvirt-override-virStream.py @@ -9,7 +9,7 @@ libvirtmod.virStreamFree(self._o) self._o = None - def dispatchStreamEventCallback(self, events, cbData): + def _dispatchStreamEventCallback(self, events, cbData): """ Dispatches events to python user's stream event callbacks """ diff --git a/python/libvirt-override.c b/python/libvirt-override.c index b000718..5a36be0 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2581,7 +2581,7 @@ libvirt_virConnectDomainEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, /* Call the Callback Dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_conn_inst, - (char*)"dispatchDomainEventCallbacks", + (char*)"_dispatchDomainEventCallbacks", (char*)"Oii", pyobj_dom_inst, event, @@ -2707,7 +2707,7 @@ libvirt_virEventAddHandleFunc (int fd, LIBVIRT_ENSURE_THREAD_STATE; /* Lookup the python callback */ - python_cb = libvirt_lookupPythonFunc("eventInvokeHandleCallback"); + python_cb = libvirt_lookupPythonFunc("_eventInvokeHandleCallback"); if (!python_cb) { goto cleanup; } @@ -2834,7 +2834,7 @@ libvirt_virEventAddTimeoutFunc(int timeout, LIBVIRT_ENSURE_THREAD_STATE; /* Lookup the python callback */ - python_cb = libvirt_lookupPythonFunc("eventInvokeTimeoutCallback"); + python_cb = libvirt_lookupPythonFunc("_eventInvokeTimeoutCallback"); if (!python_cb) { goto cleanup; } @@ -3224,7 +3224,7 @@ libvirt_virConnectDomainEventLifecycleCallback(virConnectPtr conn ATTRIBUTE_UNUS /* Call the Callback Dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_conn, - (char*)"dispatchDomainEventLifecycleCallback", + (char*)"_dispatchDomainEventLifecycleCallback", (char*)"OiiO", pyobj_dom, event, detail, @@ -3270,7 +3270,7 @@ libvirt_virConnectDomainEventGenericCallback(virConnectPtr conn ATTRIBUTE_UNUSED /* Call the Callback Dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_conn, - (char*)"dispatchDomainEventGenericCallback", + (char*)"_dispatchDomainEventGenericCallback", (char*)"OO", pyobj_dom, pyobj_cbData); @@ -3315,7 +3315,7 @@ libvirt_virConnectDomainEventRTCChangeCallback(virConnectPtr conn ATTRIBUTE_UNUS /* Call the Callback Dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_conn, - (char*)"dispatchDomainEventRTCChangeCallback", + (char*)"_dispatchDomainEventRTCChangeCallback", (char*)"OLO", pyobj_dom, (PY_LONG_LONG)utcoffset, @@ -3362,7 +3362,7 @@ libvirt_virConnectDomainEventWatchdogCallback(virConnectPtr conn ATTRIBUTE_UNUSE /* Call the Callback Dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_conn, - (char*)"dispatchDomainEventWatchdogCallback", + (char*)"_dispatchDomainEventWatchdogCallback", (char*)"OiO", pyobj_dom, action, @@ -3411,7 +3411,7 @@ libvirt_virConnectDomainEventIOErrorCallback(virConnectPtr conn ATTRIBUTE_UNUSED /* Call the Callback Dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_conn, - (char*)"dispatchDomainEventIOErrorCallback", + (char*)"_dispatchDomainEventIOErrorCallback", (char*)"OssiO", pyobj_dom, srcPath, devAlias, action, @@ -3461,7 +3461,7 @@ libvirt_virConnectDomainEventIOErrorReasonCallback(virConnectPtr conn ATTRIBUTE_ /* Call the Callback Dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_conn, - (char*)"dispatchDomainEventIOErrorReasonCallback", + (char*)"_dispatchDomainEventIOErrorReasonCallback", (char*)"OssisO", pyobj_dom, srcPath, devAlias, action, reason, @@ -3547,7 +3547,7 @@ libvirt_virConnectDomainEventGraphicsCallback(virConnectPtr conn ATTRIBUTE_UNUSE /* Call the Callback Dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_conn, - (char*)"dispatchDomainEventGraphicsCallback", + (char*)"_dispatchDomainEventGraphicsCallback", (char*)"OiOOsOO", pyobj_dom, phase, pyobj_local, pyobj_remote, @@ -3596,7 +3596,7 @@ libvirt_virConnectDomainEventBlockPullCallback(virConnectPtr conn ATTRIBUTE_UNUS /* Call the Callback Dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_conn, - (char*)"dispatchDomainEventBlockPullCallback", + (char*)"_dispatchDomainEventBlockPullCallback", (char*)"OsiO", pyobj_dom, path, status, pyobj_cbData); @@ -3752,7 +3752,7 @@ libvirt_virStreamEventCallback(virStreamPtr st ATTRIBUTE_UNUSED, /* Call the pure python dispatcher */ pyobj_ret = PyObject_CallMethod(pyobj_stream, - (char *)"dispatchStreamEventCallback", + (char *)"_dispatchStreamEventCallback", (char *)"iO", events, pyobj_cbData); diff --git a/python/libvirt-override.py b/python/libvirt-override.py index 8df9dc1..98241d6 100644 --- a/python/libvirt-override.py +++ b/python/libvirt-override.py @@ -117,7 +117,7 @@ def getVersion (name = None): # # Invoke an EventHandle callback # -def eventInvokeHandleCallback(watch, fd, event, opaque, opaquecompat=None): +def _eventInvokeHandleCallback(watch, fd, event, opaque, opaquecompat=None): """ Invoke the Event Impl Handle Callback in C """ @@ -137,7 +137,7 @@ def eventInvokeHandleCallback(watch, fd, event, opaque, opaquecompat=None): # # Invoke an EventTimeout callback # -def eventInvokeTimeoutCallback(timer, opaque, opaquecompat=None): +def _eventInvokeTimeoutCallback(timer, opaque, opaquecompat=None): """ Invoke the Event Impl Timeout Callback in C """ -- 1.7.4.4

On Wed, Jun 15, 2011 at 09:23:19PM -0400, Cole Robinson wrote:
These functions aren't intended to be called directly by users, so mark them as private.
Hum, should that be one _ or two __ ? But ACK to cleanup ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 06/19/2011 09:45 PM, Daniel Veillard wrote:
On Wed, Jun 15, 2011 at 09:23:19PM -0400, Cole Robinson wrote:
These functions aren't intended to be called directly by users, so mark them as private.
Hum, should that be one _ or two __ ? But ACK to cleanup !
Python convention is anything starting with an underscore is considered private. It only attempts to enforce this if using double underscore in the name of a class method/attribute, which causes some auto name mangling. However since these aren't class methods, double vs. single underscore isn't any different. Thanks, Cole

On Mon, Jun 20, 2011 at 03:34:07PM -0400, Cole Robinson wrote:
On 06/19/2011 09:45 PM, Daniel Veillard wrote:
On Wed, Jun 15, 2011 at 09:23:19PM -0400, Cole Robinson wrote:
These functions aren't intended to be called directly by users, so mark them as private.
Hum, should that be one _ or two __ ? But ACK to cleanup !
Python convention is anything starting with an underscore is considered private. It only attempts to enforce this if using double underscore in the name of a class method/attribute, which causes some auto name mangling. However since these aren't class methods, double vs. single underscore isn't any different.
Okay, thanks for the explanations, I remembered we used __ a long time ago, and wasn't sure :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

On 06/20/2011 08:45 PM, Daniel Veillard wrote:
On Mon, Jun 20, 2011 at 03:34:07PM -0400, Cole Robinson wrote:
On 06/19/2011 09:45 PM, Daniel Veillard wrote:
On Wed, Jun 15, 2011 at 09:23:19PM -0400, Cole Robinson wrote:
These functions aren't intended to be called directly by users, so mark them as private.
Hum, should that be one _ or two __ ? But ACK to cleanup !
Python convention is anything starting with an underscore is considered private. It only attempts to enforce this if using double underscore in the name of a class method/attribute, which causes some auto name mangling. However since these aren't class methods, double vs. single underscore isn't any different.
Okay, thanks for the explanations, I remembered we used __ a long time ago, and wasn't sure :-)
Thanks, pushed V2 of this series (since all patches either didn't change or the new versions were ACKd) - Cole
participants (3)
-
Cole Robinson
-
Daniel P. Berrange
-
Daniel Veillard