Re: [Autotest] [PATCH 1/2] KVM Test: Update cmd() help function in kvm_monitor.py to support parameters.

[Michael Goldish <mgoldish@redhat.com>]

On 06/28/2010 10:45 AM, Feng Yang wrote:
> This function allow user send qmp command with parameters.
> e.g.  balloon value=1073741824
> Also log command to debug. 
> 
> Signed-off-by: Feng Yang <fyang@redhat.com>
> ---
>  client/tests/kvm/kvm_monitor.py |   32 ++++++++++++++++++++++++--------
>  1 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/client/tests/kvm/kvm_monitor.py b/client/tests/kvm/kvm_monitor.py
> index 8440835..af9ff21 100644
> --- a/client/tests/kvm/kvm_monitor.py
> +++ b/client/tests/kvm/kvm_monitor.py
> @@ -188,6 +188,7 @@ class HumanMonitor(Monitor):
>  
>          try:
>              try:
> +                logging.debug("Send command: %s" % command)
>                  self._socket.sendall(command + "\n")
>              except socket.error:
>                  raise MonitorSendError("Could not send monitor command '%s'" %
> @@ -258,9 +259,8 @@ class HumanMonitor(Monitor):
>  
>      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.
> +        Send a simple command with/without parameters and return its output.
> +        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
> @@ -486,6 +486,7 @@ class QMPMonitor(Monitor):
>          try:
>              cmdobj = self._build_cmd(cmd, args, id)
>              try:
> +                logging.debug("Send command: %s" % cmdobj)
>                  self._socket.sendall(json.dumps(cmdobj) + "\n")
>              except socket.error:
>                  raise MonitorSendError("Could not send QMP command '%s'" % cmd)
> @@ -601,11 +602,13 @@ class QMPMonitor(Monitor):
>      # Note: all of the following functions raise exceptions in a similar manner
>      # to cmd() and _get_command_output().
>  
> -    def cmd(self, command, timeout=20):
> +    def cmd(self, cmdline, 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.
> +        Send a simple command with/without parameters and return its output.
> +        Implemented under the same name for both the human and QMP monitors.
> +        Command with parameters should in following format e.g.:
> +        'memsave val=0 size=10240 filename=memsave'
> +        Command without parameter: 'memsave'
>  
>          @param command: Command to send
>          @param timeout: Time duration to wait for response
> @@ -614,7 +617,20 @@ class QMPMonitor(Monitor):
>          @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)
> +        cmdargs = cmdline.split()
> +        command = cmdargs[0]
> +        args = {}
> +        for arg in cmdargs[1:]:
> +            opt = arg.split('=')
> +            try:
> +                try:
> +                    value = int(opt[1])
> +                except ValueError:
> +                    value = opt[1]
> +                args[opt[0]] = value
> +            except:
> +                  logging.debug("Fail to create args, please check command")
> +        return self._get_command_output(command, args, timeout=timeout)
>  
>  
>      def quit(self):

Why not add a wrapper for the command you're interested in?
If we do it your way, a test that uses cmd() with parameters will have
to handle the human case and the QMP case separately.  For example, if a
human monitor is used the test will have to use

  vm.monitor.cmd("screendump scr.ppm")

and if QMP is used the test will have to use

  vm.monitor.cmd("screendump filename=scr.ppm").

but if we use a wrapper,

  vm.monitor.screendump("scr.ppm")

will work in both cases.