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@xxxxxxxxxx> > >> --- > >> 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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list