diff mbox series

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

Message ID 20191227134101.244496-5-wainersm@redhat.com (mailing list archive)
State New, archived
Headers show
Series python/qemu: qmp: Fix, delint and improvements | expand

Commit Message

Wainer dos Santos Moschetta Dec. 27, 2019, 1:41 p.m. UTC
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 <wainersm@redhat.com>
---
 python/qemu/qmp.py | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

John Snow Jan. 9, 2020, 12:23 a.m. UTC | #1
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 <wainersm@redhat.com>
> ---
>  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?

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

>  
>      def settimeout(self, timeout):
>          """
>
Wainer dos Santos Moschetta Jan. 29, 2020, 8:07 p.m. UTC | #2
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 <wainersm@redhat.com>
>> ---
>>   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):
>>           """
>>
diff mbox series

Patch

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()
 
     def settimeout(self, timeout):
         """