Re: [PATCH 4/5] python/qemu: qmp: Make QEMUMonitorProtocol a context manager

[Wainer dos Santos Moschetta]

On 1/8/20 10:23 PM, John Snow wrote:
> On 12/27/19 8:41 AM, Wainer dos Santos Moschetta wrote:
> > This implement the __enter__ and __exit__ functions on
> > QEMUMonitorProtocol class so that it can be used on 'with'
> > statement and the resources will be free up on block end:
> >
> > with QEMUMonitorProtocol(socket_path) as qmp:
> >      qmp.connect()
> >      qmp.command('query-status')
> >
> > Signed-off-by: Wainer dos Santos Moschetta <address@hidden>
> > ---
> >   python/qemu/qmp.py | 15 +++++++++++++--
> >   1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
> > index 914b8c6774..6d55f53595 100644
> > --- a/python/qemu/qmp.py
> > +++ b/python/qemu/qmp.py
> > @@ -139,6 +139,15 @@ class QEMUMonitorProtocol:
> >                   raise QMPConnectError("Error while reading from socket")
> >               self.__sock.settimeout(None)
> >   
> > +    def __enter__(self):
> > +        # Implement context manager enter function.
> > +        return self
> > +
> > +    def __exit__(self, exc_type, exc_value, exc_traceback):
> > +        # Implement context manager exit function.
> > +        self.close()
> > +        return False
> > +
> >       def connect(self, negotiate=True):
> >           """
> >           Connect to the QMP Monitor and perform capabilities negotiation.
> > @@ -259,8 +268,10 @@ class QEMUMonitorProtocol:
> >           """
> >           Close the socket and socket file.
> >           """
> > -        self.__sock.close()
> > -        self.__sockfile.close()
> > +        if self.__sock:
> > +            self.__sock.close()
> > +        if self.__sockfile:
> > +            self.__sockfile.close()
> Not evident on cold read: does self.close() change self.__sock and
> self.__sockfile such that they are false-ish?


Because self.__exit__() calls self.close() even when a runtime exception 
raises, there isn't any guarantee that self.__sockfile and self.__sock 
were initialized. That's the reason why I added those guards.


> close() I suspect might need to actually unset the __sock and __sockfile
> fields.

The QEMUMonitorProtocol object is designed to be disposed after close() 
being called. So I don't see any reason to unset those fields. Unless I 
am missing something here...

Thanks for the comments!

- Wainer

> >   
> >       def settimeout(self, timeout):
> >           """