[libvirt] [PATCH v2 2/2] Add asyncio event loop implementation

[Daniel P. Berrange]

On Fri, Mar 17, 2017 at 02:35:53PM +0100, Wojtek Porczyk wrote:

> +class Callback(object):
> +    '''Base class for holding callback
> +
> +    :param virEventAsyncIOImpl impl: the implementation in which we run
> +    :param cb: the callback itself
> +    :param opaque: the opaque tuple passed by libvirt
> +    '''
> +    # pylint: disable=too-few-public-methods
> +
> +    _iden_counter = itertools.count()
> +
> +    def __init__(self, impl, cb, opaque, *args, **kwargs):
> +        super().__init__(*args, **kwargs)
> +        self.iden = next(self._iden_counter)
> +        self.impl = impl
> +        self.cb = cb
> +        self.opaque = opaque
> +
> +        assert self.iden not in self.impl.callbacks, \
> +            'found {} callback: {!r}'.format(
> +                self.iden, self.impl.callbacks[self.iden])
> +        self.impl.callbacks[self.iden] = self
> +
> +    def __repr__(self):
> +        return '<{} iden={}>'.format(self.__clas__.__name__, self.iden)

This looks like it should be 'self.__class__'

> +class TimeoutCallback(Callback):
> +    '''Callback for timer'''
> +    def __init__(self, *args, timeout, **kwargs):
> +        super().__init__(*args, **kwargs)
> +        self.timeout = timeout
> +        self._task = None
> +
> +    def __repr__(self):
> +        return '<{} iden={} timeout={}>'.format(
> +            self.__class__.__name__, self.iden, self.timeout)
> +
> +    @asyncio.coroutine
> +    def _timer(self):
> +        '''An actual timer running on the event loop.
> +
> +        This is a coroutine.
> +        '''
> +        while True:
> +            assert self.timeout >= 0, \
> +                'invalid timeout {} for running timer'.format(self.timeout)

When I test, this assert always fires. It seems that when we call 'close',
setting timeout==-1, this _timer coroutine continues for one more iteration
before CancelledError is triggered.

> +
> +            try:
> +                if self.timeout > 0:
> +                    timeout = self.timeout * 1e-3
> +                    self.impl.log.debug('sleeping %r', timeout)
> +                    yield from asyncio.sleep(timeout)
> +                else:
> +                    # scheduling timeout for next loop iteration
> +                    yield
> +
> +            except asyncio.CancelledError:
> +                self.impl.log.debug('timer %d cancelled', self.iden)
> +                break
> +
> +            self.cb(self.iden, self.opaque)
> +            self.impl.log.debug('timer %r callback ended', self.iden)
> +
> +    def update(self, *, timeout=None):
> +        '''Start or the timer, possibly updating timeout'''
> +        if timeout is not None:
> +            self.timeout = timeout
> +

Using timeout=None as the default looks wrong to me - It should
be either mandatory, or -1 IMHO.

> +        if self.timeout >= 0 and self._task is None:
> +            self.impl.log.debug('timer %r start', self.iden)
> +            self._task = ensure_future(self._timer(),
> +                loop=self.impl.loop)
> +
> +        elif self.timeout < 0 and self._task is not None:
> +            self.impl.log.debug('timer %r stop', self.iden)
> +            self._task.cancel()  # pylint: disable=no-member
> +            self._task = None
> +
> +    def close(self):
> +        '''Stop the timer and call ff callback'''
> +        self.timeout = -1
> +        self.update()
> +        super().close()

> diff --git a/setup.py b/setup.py
> index 120ddd5..bac9010 100755
> --- a/setup.py
> +++ b/setup.py
> @@ -14,6 +14,7 @@ import sys
>  import os
>  import os.path
>  import re
> +import shutil
>  import time
>  
>  MIN_LIBVIRT = "0.9.11"
> @@ -50,6 +51,12 @@ def have_libvirt_lxc():
>      except DistutilsExecError:
>          return False
>  
> +def have_libvirtaio():
> +    # This depends on asyncio, which in turn depends on "yield from" syntax.
> +    # The asyncio module itself is in standard library since 3.4, but there is
> +    # an out-of-tree version compatible with 3.3.
> +    return sys.version_info >= (3, 3)
> +
>  def get_pkgconfig_data(args, mod, required=True):
>      """Run pkg-config to and return content associated with it"""
>      f = os.popen("%s %s %s" % (get_pkgcfg(), " ".join(args), mod))
> @@ -124,6 +131,9 @@ def get_module_lists():
>          c_modules.append(modulelxc)
>          py_modules.append("libvirt_lxc")
>  
> +    if have_libvirtaio():
> +        py_modules.append("libvirtaio")
> +
>      return c_modules, py_modules
>  
>  
> @@ -141,6 +151,8 @@ class my_build(build):
>          self.spawn([sys.executable, "generator.py", "libvirt-qemu", apis[1]])
>          if have_libvirt_lxc():
>              self.spawn([sys.executable, "generator.py", "libvirt-lxc", apis[2]])
> +        if have_libvirtaio():
> +            shutil.copy('libvirtaio.py', 'build')
>  
>          build.run(self)

We also need to add libvirtaio.py to MANIFEST.in to ensure it gets into
the dist

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|