On Mon, 2010-10-18 at 15:13 +0200, Michael Goldish wrote:
> Instead of _get_command_output() and friends, introduce the following methods:
>
> * QMP:
> - _send(): send raw data without waiting for a response
> - _get_response(): get the response to a previously sent command
> - cmd(): send a command with arguments, return response, raise an exception
> if the command fails
> - cmd_raw(): send a raw string, return response dict without postprocessing
> - cmd_obj(): send a Python object (converted to JSON), return response dict
> without postprocessing
> - cmd_qmp(): send a command with arguments, return response dict without
> postprocessing
>
> cmd() is useful for common monitor usage. cmd_raw(), cmd_obj() and
> cmd_qmp() are required by Luiz Capitulino's test suite. The difference
> between cmd() and cmd_qmp() is that the latter does not perform any checks
> on the response dict. Note that cmd_raw() is functionally equivalent to
> send() from Luiz's patch. I propose that we use the name cmd_raw() because
> send() implies sending data without caring about the response, whereas
> cmd_raw() implies doing exactly what cmd_obj() and cmd_qmp() do, but using
> raw data.
>
> * Human monitor:
> - _send(): send raw data without waiting for a response
> - cmd(): send a command, return response
This patch does look reasonable to me. Luiz, what about rewriting your
patchset after I apply this to upstream?
> Signed-off-by: Michael Goldish <mgoldish@xxxxxxxxxx>
> ---
> client/tests/kvm/kvm_monitor.py | 235 ++++++++++++++++++++++----------------
> 1 files changed, 136 insertions(+), 99 deletions(-)
>
> diff --git a/client/tests/kvm/kvm_monitor.py b/client/tests/kvm/kvm_monitor.py
> index 6bff07f..b887060 100644
> --- a/client/tests/kvm/kvm_monitor.py
> +++ b/client/tests/kvm/kvm_monitor.py
> @@ -130,7 +130,7 @@ class HumanMonitor(Monitor):
> suppress_exceptions is False
> @raise MonitorProtocolError: Raised if the initial (qemu) prompt isn't
> found and suppress_exceptions is False
> - @note: Other exceptions may be raised. See _get_command_output's
> + @note: Other exceptions may be raised. See cmd()'s
> docstring.
> """
> try:
> @@ -146,7 +146,7 @@ class HumanMonitor(Monitor):
> "Output so far: %r" % o)
>
> # Save the output of 'help' for future use
> - self._help_str = self._get_command_output("help")
> + self._help_str = self.cmd("help")
>
> except MonitorError, e:
> if suppress_exceptions:
> @@ -173,31 +173,32 @@ class HumanMonitor(Monitor):
> return False, "\n".join(o.splitlines())
>
>
> - def _send_command(self, command):
> + def _send(self, cmd):
> """
> Send a command without waiting for output.
>
> - @param command: Command to send
> - @return: True if successful, False otherwise
> + @param cmd: Command to send
> @raise MonitorLockError: Raised if the lock cannot be acquired
> @raise MonitorSendError: Raised if the command cannot be sent
> """
> if not self._acquire_lock(20):
> raise MonitorLockError("Could not acquire exclusive lock to send "
> - "monitor command '%s'" % command)
> + "monitor command '%s'" % cmd)
>
> try:
> try:
> - self._socket.sendall(command + "\n")
> + self._socket.sendall(cmd + "\n")
> except socket.error:
> raise MonitorSendError("Could not send monitor command '%s'" %
> - command)
> + cmd)
>
> finally:
> self._lock.release()
>
>
> - def _get_command_output(self, command, timeout=20):
> + # Public methods
> +
> + def cmd(self, command, timeout=20):
> """
> Send command to the monitor.
>
> @@ -217,7 +218,7 @@ class HumanMonitor(Monitor):
> # Read any data that might be available
> self._recvall()
> # Send command
> - self._send_command(command)
> + self._send(command)
> # Read output
> s, o = self._read_up_to_qemu_prompt(timeout)
> # Remove command echo from output
> @@ -234,8 +235,6 @@ class HumanMonitor(Monitor):
> self._lock.release()
>
>
> - # Public methods
> -
> def is_responsive(self):
> """
> Make sure the monitor is responsive by sending a command.
> @@ -243,7 +242,7 @@ class HumanMonitor(Monitor):
> @return: True if responsive, False otherwise
> """
> try:
> - self._get_command_output("info status")
> + self.cmd("info status")
> return True
> except MonitorError:
> return False
> @@ -252,39 +251,22 @@ class HumanMonitor(Monitor):
> # Command wrappers
> # Notes:
> # - All of the following commands raise exceptions in a similar manner to
> - # cmd() and _get_command_output().
> + # cmd().
> # - A command wrapper should use self._help_str if it requires information
> # about the monitor's capabilities.
>
> - def cmd(self, command, timeout=20):
> - """
> - Send a simple command with no parameters and return its output.
> - Should only be used for commands that take no parameters and are
> - implemented under the same name for both the human and QMP monitors.
> -
> - @param command: Command to send
> - @param timeout: Time duration to wait for (qemu) prompt after command
> - @return: The output of the command
> - @raise MonitorLockError: Raised if the lock cannot be acquired
> - @raise MonitorSendError: Raised if the command cannot be sent
> - @raise MonitorProtocolError: Raised if the (qemu) prompt cannot be
> - found after sending the command
> - """
> - return self._get_command_output(command, timeout)
> -
> -
> def quit(self):
> """
> Send "quit" without waiting for output.
> """
> - self._send_command("quit")
> + self._send("quit")
>
>
> def info(self, what):
> """
> Request info about something and return the output.
> """
> - return self._get_command_output("info %s" % what)
> + return self.cmd("info %s" % what)
>
>
> def query(self, what):
> @@ -301,7 +283,7 @@ class HumanMonitor(Monitor):
> @param filename: Location for the screendump
> @return: The command's output
> """
> - return self._get_command_output("screendump %s" % filename)
> + return self.cmd("screendump %s" % filename)
>
>
> def migrate(self, uri, full_copy=False, incremental_copy=False, wait=False):
> @@ -323,7 +305,7 @@ class HumanMonitor(Monitor):
> if incremental_copy:
> cmd += " -i"
> cmd += " %s" % uri
> - return self._get_command_output(cmd)
> + return self.cmd(cmd)
>
>
> def migrate_set_speed(self, value):
> @@ -333,7 +315,7 @@ class HumanMonitor(Monitor):
> @param value: Speed in bytes/sec
> @return: The command's output
> """
> - return self._get_command_output("migrate_set_speed %s" % value)
> + return self.cmd("migrate_set_speed %s" % value)
>
>
> def sendkey(self, keystr, hold_time=1):
> @@ -344,7 +326,7 @@ class HumanMonitor(Monitor):
> @param hold_time: Hold time in ms (should normally stay 1 ms)
> @return: The command's output
> """
> - return self._get_command_output("sendkey %s %s" % (keystr, hold_time))
> + return self.cmd("sendkey %s %s" % (keystr, hold_time))
>
>
> def mouse_move(self, dx, dy):
> @@ -355,7 +337,7 @@ class HumanMonitor(Monitor):
> @param dy: Y amount
> @return: The command's output
> """
> - return self._get_command_output("mouse_move %d %d" % (dx, dy))
> + return self.cmd("mouse_move %d %d" % (dx, dy))
>
>
> def mouse_button(self, state):
> @@ -365,7 +347,7 @@ class HumanMonitor(Monitor):
> @param state: Button state (1=L, 2=M, 4=R)
> @return: The command's output
> """
> - return self._get_command_output("mouse_button %d" % state)
> + return self.cmd("mouse_button %d" % state)
>
>
> class QMPMonitor(Monitor):
> @@ -387,7 +369,7 @@ class QMPMonitor(Monitor):
> @raise MonitorNotSupportedError: Raised if json isn't available and
> suppress_exceptions is False
> @note: Other exceptions may be raised if the qmp_capabilities command
> - fails. See _get_command_output's docstring.
> + fails. See cmd()'s docstring.
> """
> try:
> Monitor.__init__(self, name, filename)
> @@ -417,7 +399,7 @@ class QMPMonitor(Monitor):
> raise MonitorProtocolError("No QMP greeting message received")
>
> # Issue qmp_capabilities
> - self._get_command_output("qmp_capabilities")
> + self.cmd("qmp_capabilities")
>
> except MonitorError, e:
> if suppress_exceptions:
> @@ -470,33 +452,46 @@ class QMPMonitor(Monitor):
> return objs
>
>
> - def _send_command(self, cmd, args=None, id=None):
> + def _send(self, data):
> """
> - Send command without waiting for response.
> + Send raw data without waiting for response.
>
> - @param cmd: Command to send
> - @param args: A dict containing command arguments, or None
> - @raise MonitorLockError: Raised if the lock cannot be acquired
> - @raise MonitorSendError: Raised if the command cannot be sent
> + @param data: Data to send
> + @raise MonitorSendError: Raised if the data cannot be sent
> """
> - if not self._acquire_lock(20):
> - raise MonitorLockError("Could not acquire exclusive lock to send "
> - "QMP command '%s'" % cmd)
> -
> try:
> - cmdobj = self._build_cmd(cmd, args, id)
> - try:
> - self._socket.sendall(json.dumps(cmdobj) + "\n")
> - except socket.error:
> - raise MonitorSendError("Could not send QMP command '%s'" % cmd)
> + self._socket.sendall(data)
> + except socket.error:
> + raise MonitorSendError("Could not send data: %r" % data)
>
> - finally:
> - self._lock.release()
>
> + def _get_response(self, id=None, timeout=20):
> + """
> + Read a response from the QMP monitor.
>
> - def _get_command_output(self, cmd, args=None, timeout=20):
> + @param id: If not None, look for a response with this id
> + @param timeout: Time duration to wait for response
> + @return: The response dict, or None if none was found
> """
> - Send monitor command and wait for response.
> + end_time = time.time() + timeout
> + while time.time() < end_time:
> + for obj in self._read_objects():
> + if isinstance(obj, dict):
> + if id is not None and obj.get("id") != id:
> + continue
> + if "return" in obj or "error" in obj:
> + return obj
> + time.sleep(0.1)
> +
> +
> + # Public methods
> +
> + def cmd(self, cmd, args=None, timeout=20):
> + """
> + Send a QMP monitor command and return the response.
> +
> + Note: an id is automatically assigned to the command and the response
> + is checked for the presence of the same id.
>
> @param cmd: Command to send
> @param args: A dict containing command arguments, or None
> @@ -518,28 +513,86 @@ class QMPMonitor(Monitor):
> self._read_objects()
> # Send command
> id = kvm_utils.generate_random_string(8)
> - self._send_command(cmd, args, id)
> + self._send(json.dumps(self._build_cmd(cmd, args, id)) + "\n")
> # Read response
> - end_time = time.time() + timeout
> - while time.time() < end_time:
> - for obj in self._read_objects():
> - if isinstance(obj, dict) and obj.get("id") == id:
> - if "return" in obj:
> - return obj["return"]
> - elif "error" in obj:
> - raise QMPCmdError("QMP command '%s' failed" % cmd,
> - obj["error"])
> - time.sleep(0.1)
> - # No response found
> - raise MonitorProtocolError("Received no response to QMP command "
> - "'%s', or received a response with an "
> - "incorrect id" % cmd)
> + r = self._get_response(id, timeout)
> + if r is None:
> + raise MonitorProtocolError("Received no response to QMP "
> + "command '%s', or received a "
> + "response with an incorrect id"
> + % cmd)
> + if "return" in r:
> + return r["return"]
> + if "error" in r:
> + raise QMPCmdError("QMP command '%s' failed" % cmd, r["error"])
>
> finally:
> self._lock.release()
>
>
> - # Public methods
> + def cmd_raw(self, data, timeout=20):
> + """
> + Send a raw string to the QMP monitor and return the response.
> + Unlike cmd(), return the raw response dict without performing any
> + checks on it.
> +
> + @param data: The data to send
> + @param timeout: Time duration to wait for response
> + @return: The response received
> + @raise MonitorLockError: Raised if the lock cannot be acquired
> + @raise MonitorSendError: Raised if the command cannot be sent
> + @raise MonitorProtocolError: Raised if no response is received
> + """
> + if not self._acquire_lock(20):
> + raise MonitorLockError("Could not acquire exclusive lock to send "
> + "data: %r" % data)
> +
> + try:
> + self._read_objects()
> + self._send(data)
> + r = self._get_response(None, timeout)
> + if r is None:
> + raise MonitorProtocolError("Received no response to data: %r" %
> + data)
> + return r
> +
> + finally:
> + self._lock.release()
> +
> +
> + def cmd_obj(self, obj, timeout=20):
> + """
> + Transform a Python object to JSON, send the resulting string to the QMP
> + monitor, and return the response.
> + Unlike cmd(), return the raw response dict without performing any
> + checks on it.
> +
> + @param obj: The object to send
> + @param timeout: Time duration to wait for response
> + @return: The response received
> + @raise MonitorLockError: Raised if the lock cannot be acquired
> + @raise MonitorSendError: Raised if the command cannot be sent
> + @raise MonitorProtocolError: Raised if no response is received
> + """
> + return self.cmd_raw(json.dumps(obj) + "\n")
> +
> +
> + def cmd_qmp(self, cmd, args=None, id=None, timeout=20):
> + """
> + Build a QMP command from the passed arguments, send it to the monitor
> + and return the response.
> + Unlike cmd(), return the raw response dict without performing any
> + checks on it.
> +
> + @param obj: The object to send
> + @param timeout: Time duration to wait for response
> + @return: The response received
> + @raise MonitorLockError: Raised if the lock cannot be acquired
> + @raise MonitorSendError: Raised if the command cannot be sent
> + @raise MonitorProtocolError: Raised if no response is received
> + """
> + return self.cmd_obj(self._build_cmd(cmd, args, id), timeout)
> +
>
> def is_responsive(self):
> """
> @@ -548,7 +601,7 @@ class QMPMonitor(Monitor):
> @return: True if responsive, False otherwise
> """
> try:
> - self._get_command_output("query-status")
> + self.cmd("query-status")
> return True
> except MonitorError:
> return False
> @@ -599,36 +652,20 @@ class QMPMonitor(Monitor):
>
> # Command wrappers
> # Note: all of the following functions raise exceptions in a similar manner
> - # to cmd() and _get_command_output().
> -
> - def cmd(self, command, timeout=20):
> - """
> - Send a simple command with no parameters and return its output.
> - Should only be used for commands that take no parameters and are
> - implemented under the same name for both the human and QMP monitors.
> -
> - @param command: Command to send
> - @param timeout: Time duration to wait for response
> - @return: The response to the command
> - @raise MonitorLockError: Raised if the lock cannot be acquired
> - @raise MonitorSendError: Raised if the command cannot be sent
> - @raise MonitorProtocolError: Raised if no response is received
> - """
> - return self._get_command_output(command, timeout=timeout)
> -
> + # to cmd().
>
> def quit(self):
> """
> Send "quit" and return the response.
> """
> - return self._get_command_output("quit")
> + return self.cmd("quit")
>
>
> def info(self, what):
> """
> Request info about something and return the response.
> """
> - return self._get_command_output("query-%s" % what)
> + return self.cmd("query-%s" % what)
>
>
> def query(self, what):
> @@ -646,7 +683,7 @@ class QMPMonitor(Monitor):
> @return: The response to the command
> """
> args = {"filename": filename}
> - return self._get_command_output("screendump", args)
> + return self.cmd("screendump", args)
>
>
> def migrate(self, uri, full_copy=False, incremental_copy=False, wait=False):
> @@ -662,7 +699,7 @@ class QMPMonitor(Monitor):
> args = {"uri": uri,
> "blk": full_copy,
> "inc": incremental_copy}
> - return self._get_command_output("migrate", args)
> + return self.cmd("migrate", args)
>
>
> def migrate_set_speed(self, value):
> @@ -673,4 +710,4 @@ class QMPMonitor(Monitor):
> @return: The response to the command
> """
> args = {"value": value}
> - return self._get_command_output("migrate_set_speed", args)
> + return self.cmd("migrate_set_speed", args)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
