Re: [libvirt] [PATCH v2 1/6] libvirtaio: add more debug logging

On Mon, Sep 25, 2017 at 02:43:56PM +0100, Daniel P. Berrange wrote: > On Thu, Aug 24, 2017 at 09:22:49PM +0200, Wojtek Porczyk wrote: > > This logging is helpful for tracing problems with unclosed connections > > and leaking file descriptors. > > > > Signed-off-by: Wojtek Porczyk <woju(a)invisiblethingslab.com&gt; > > --- > > libvirtaio.py | 33 +++++++++++++++++++++++++-------- > > 1 file changed, 25 insertions(+), 8 deletions(-) > > > > diff --git a/libvirtaio.py b/libvirtaio.py > > index 7c8c396..46de9ab 100644 > > --- a/libvirtaio.py > > +++ b/libvirtaio.py > > @@ -74,7 +74,7 @@ class Callback(object): > > def close(self): > > '''Schedule *ff* callback''' > > self.impl.log.debug('callback %d close(), scheduling ff', self.iden) > > - self.impl.schedule_ff_callback(self.opaque) > > + self.impl.schedule_ff_callback(self.iden, self.opaque) > > > > # > > # file descriptors > > @@ -275,6 +275,10 @@ class virEventAsyncIOImpl(object): > > self.descriptors = DescriptorDict(self) > > self.log = logging.getLogger(self.__class__.__name__) > > > > + def __repr__(self): > > + return '<{} callbacks={} descriptors={}>'.format( > > + type(self).__name__, self.callbacks, self.descriptors) > > + > > def register(self): > > '''Register this instance as event loop implementation''' > > # pylint: disable=bad-whitespace > > @@ -284,9 +288,18 @@ class virEventAsyncIOImpl(object): > > self._add_timeout, self._update_timeout, self._remove_timeout) > > return self > > > > - def schedule_ff_callback(self, opaque): > > + def schedule_ff_callback(self, iden, opaque): > > '''Schedule a ff callback from one of the handles or timers''' > > - self.loop.call_soon(libvirt.virEventInvokeFreeCallback, opaque) > > + ensure_future(self._ff_callback(iden, opaque), loop=self.loop) > > This use of ensure_future puts a min python version of 3.4 on the code. Is > there a strong reason to prefer that over the previous call_soon code ? 1) There is no technical reason whatsoever. This is an artifact from the previous iteration. 2) In fact ensure_future() does not make it incompatible with python<3.4 because of the try/except import at the beginning of the file (in python<3.4.3 the function is called asyncio.async, imported as ensure_future).