[PULL 21/21] python/qemu/qmp.py: Fix settimeout operation

[John Snow]

We enabled callers to interface directly with settimeout, but this
reacts poorly with blocking/nonblocking operation; as they are using the
same internal mechanism.

1. Whenever we change the blocking mechanism temporarily, always set it
back to what it was afterwards.

2. Disallow callers from setting a timeout of "0", which means
Non-blocking mode. This is going to create more weird problems than
anybody wants, so just forbid it.

I opt not to coerce '0' to 'None' to maintain the principal of least
surprise in mirroring the semantics of Python's interface.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20201009175123.249009-4-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/qemu/qmp.py | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/python/qemu/qmp.py b/python/qemu/qmp.py
index 4969e5741c..f64517fb0a 100644
--- a/python/qemu/qmp.py
+++ b/python/qemu/qmp.py
@@ -164,16 +164,19 @@ def __get_events(self, wait: Union[bool, float] = False) 
-> None:
                                 retrieved or if some other error occurred.
         """
 
+        # Current timeout and blocking status
+        current_timeout = self.__sock.gettimeout()
+
         # Check for new events regardless and pull them into the cache:
         try:
-            self.__sock.setblocking(False)
+            self.__sock.settimeout(0)  # i.e. setblocking(False)
             self.__json_read()
         except OSError as err:
             # EAGAIN: No data available; not critical
             if err.errno != errno.EAGAIN:
                 raise
         finally:
-            self.__sock.setblocking(True)
+            self.__sock.settimeout(current_timeout)
 
         # Wait for new events, if needed.
         # if wait is 0.0, this means "no wait" and is also implicitly false.
@@ -187,9 +190,11 @@ def __get_events(self, wait: Union[bool, float] = False) 
-> None:
             except Exception as err:
                 msg = "Error while reading from socket"
                 raise QMPConnectError(msg) from err
+            finally:
+                self.__sock.settimeout(current_timeout)
+
             if ret is None:
                 raise QMPConnectError("Error while reading from socket")
-            self.__sock.settimeout(None)
 
     def __enter__(self) -> 'QEMUMonitorProtocol':
         # Implement context manager enter function.
@@ -219,7 +224,7 @@ def connect(self, negotiate: bool = True) -> 
Optional[QMPMessage]:
             return self.__negotiate_capabilities()
         return None
 
-    def accept(self, timeout: float = 15.0) -> QMPMessage:
+    def accept(self, timeout: Optional[float] = 15.0) -> QMPMessage:
         """
         Await connection from QMP Monitor and perform capabilities negotiation.
 
@@ -338,13 +343,19 @@ def close(self) -> None:
         if self.__sockfile:
             self.__sockfile.close()
 
-    def settimeout(self, timeout: float) -> None:
+    def settimeout(self, timeout: Optional[float]) -> None:
         """
         Set the socket timeout.
 
-        @param timeout (float): timeout in seconds, or None.
+        @param timeout (float): timeout in seconds (non-zero), or None.
         @note This is a wrap around socket.settimeout
+
+        @raise ValueError: if timeout was set to 0.
         """
+        if timeout == 0:
+            msg = "timeout cannot be 0; this engages non-blocking mode."
+            msg += " Use 'None' instead to disable timeouts."
+            raise ValueError(msg)
         self.__sock.settimeout(timeout)
 
     def get_sock_fd(self) -> int:
-- 
2.26.2