QMP: always return the command result
authorApollon Oikonomopoulos <apoikos@gmail.com>
Mon, 7 Apr 2014 13:37:43 +0000 (16:37 +0300)
committerThomas Thrainer <thomasth@google.com>
Tue, 8 Apr 2014 09:15:17 +0000 (11:15 +0200)
According to the QEMU Machine Protocol Specification, the messages sent
by QMP as a response to a command can be of two types: either an error
message (identified by the "error" key), or a success message
(identified by the "return" key).

Since we explicitly handle errors by raising a HypervisorError, there is
no reason to return the whole response message; instead we return only
the response part, which is the dual behavior of accepting commands from
the callers without the surrounding {"execute": ...} object.

We also unexport the RETURN_KEY constant, which is now only used
internally.

Signed-off-by: Apollon Oikonomopoulos <apoikos@gmail.com>
Signed-off-by: Thomas Thrainer <thomasth@google.com>
Reviewed-by: Thomas Thrainer <thomasth@google.com>

lib/hypervisor/hv_kvm/__init__.py
lib/hypervisor/hv_kvm/monitor.py
test/py/ganeti.hypervisor.hv_kvm_unittest.py

index 592b81d..4a80b2d 100644 (file)
@@ -856,10 +856,10 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     try:
       qmp = QmpConnection(self._InstanceQmpMonitor(instance_name))
       qmp.connect()
-      vcpus = len(qmp.Execute("query-cpus")[qmp.RETURN_KEY])
+      vcpus = len(qmp.Execute("query-cpus"))
       # Will fail if ballooning is not enabled, but we can then just resort to
       # the value above.
-      mem_bytes = qmp.Execute("query-balloon")[qmp.RETURN_KEY][qmp.ACTUAL_KEY]
+      mem_bytes = qmp.Execute("query-balloon")[qmp.ACTUAL_KEY]
       memory = mem_bytes / 1048576
     except errors.HypervisorError:
       pass
index 353a707..8305219 100644 (file)
@@ -189,7 +189,7 @@ class QmpConnection(MonitorSocket):
   _FIRST_MESSAGE_KEY = "QMP"
   _EVENT_KEY = "event"
   _ERROR_KEY = "error"
-  _RETURN_KEY = RETURN_KEY = "return"
+  _RETURN_KEY = "return"
   _ACTUAL_KEY = ACTUAL_KEY = "actual"
   _ERROR_CLASS_KEY = "class"
   _ERROR_DESC_KEY = "desc"
@@ -333,7 +333,7 @@ class QmpConnection(MonitorSocket):
 
     """
     result = self.Execute(self._QUERY_COMMANDS)
-    return frozenset(com["name"] for com in result[self._RETURN_KEY])
+    return frozenset(com["name"] for com in result)
 
   def Execute(self, command, arguments=None):
     """Executes a QMP command and returns the response of the server.
@@ -362,8 +362,11 @@ class QmpConnection(MonitorSocket):
       message[self._ARGUMENTS_KEY] = arguments
     self._Send(message)
 
-    # Events can occur between the sending of the command and the reception
-    # of the response, so we need to filter out messages with the event key.
+    # According the the QMP specification, there are only two reply types to a
+    # command: either error (containing the "error" key) or success (containing
+    # the "return" key). There is also a third possibility, that of an
+    # (unrelated to the command) asynchronous event notification, identified by
+    # the "event" key.
     while True:
       response = self._Recv()
       err = response[self._ERROR_KEY]
@@ -374,5 +377,8 @@ class QmpConnection(MonitorSocket):
                                       err[self._ERROR_DESC_KEY],
                                       err[self._ERROR_CLASS_KEY]))
 
-      elif not response[self._EVENT_KEY]:
-        return response
+      elif response[self._EVENT_KEY]:
+        # Filter-out any asynchronous events
+        continue
+
+      return response[self._RETURN_KEY]
index bc153ba..aebf1f6 100755 (executable)
@@ -189,10 +189,10 @@ class TestQmp(testutils.GanetiTestCase):
       ]
 
     expected_responses = [
-      {"return": {"enabled": True, "present": True}},
-      {"return": {}},
-      {"return": [{"name": "quit"}, {"name": "eject"}]},
-      {"return": {"running": True, "singlestep": False}},
+      {"enabled": True, "present": True},
+      {},
+      [{"name": "quit"}, {"name": "eject"}],
+      {"running": True, "singlestep": False},
       ]
 
     # Set up the stub
@@ -207,11 +207,12 @@ class TestQmp(testutils.GanetiTestCase):
 
     # Format the script
     for request, expected_response in zip(requests, expected_responses):
-      response = qmp_connection.Execute(request)
-      msg = hv_kvm.QmpMessage(expected_response)
+      response = qmp_connection.Execute(request["execute"],
+                                        request["arguments"])
+      self.assertEqual(response, expected_response)
+      msg = hv_kvm.QmpMessage({"return": expected_response})
       self.assertEqual(len(str(msg).splitlines()), 1,
                        msg="Got multi-line message")
-      self.assertEqual(response, msg)
 
     self.assertRaises(monitor.QmpCommandNotSupported,
                       qmp_connection.Execute,