[libvirt] [PATCH 09/10] python: events: Fix C->Python handle callback prototype
Daniel Veillard
veillard at redhat.com
Tue Jun 21 00:42:55 UTC 2011
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 at 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 at veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
More information about the libvir-list
mailing list