public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2 RESEND] KVM Test: Introduce qmp_monitor property in 'kvm_vm.VM'
       [not found] <1294383461-9425-1-git-send-email-qzhou@redhat.com>
@ 2011-01-10 10:47 ` qzhou
  2011-01-10 17:52   ` Michael Goldish
  2011-01-10 10:48 ` [PATCH 2/2 v2] KVM Test: Fix qmp_basic test failure in qmp-kvm-0.12.* qzhou
  1 sibling, 1 reply; 6+ messages in thread
From: qzhou @ 2011-01-10 10:47 UTC (permalink / raw)
  To: autotest; +Cc: kvm

From: Qingtang Zhou <qzhou@redhat.com>

Introduce qmp_monitor property can help us easily get a qmp monitor
of guest.

CC to Luiz and kvm@vger.kernel.org

Signed-off-by: Qingtang Zhou <qzhou@redhat.com>
---
 client/tests/kvm/kvm_vm.py          |   10 ++++++++++
 client/tests/kvm/tests/qmp_basic.py |   15 +++++++++------
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
index f6f1684..0ba2db4 100755
--- a/client/tests/kvm/kvm_vm.py
+++ b/client/tests/kvm/kvm_vm.py
@@ -870,6 +870,16 @@ class VM:
         if self.monitors and not self.params.get("main_monitor"):
             return self.monitors[0]
 
+    @property
+    def qmp_monitor(self):
+        """
+        Return the first QMP monitor.
+        If no QMP monitor exist, return None.
+        """
+        for m in self.monitors:
+            if isinstance(m, kvm_monitor.QMPMonitor):
+                return m
+        return None
 
     def is_alive(self):
         """
diff --git a/client/tests/kvm/tests/qmp_basic.py b/client/tests/kvm/tests/qmp_basic.py
index 985ad15..36cbd78 100644
--- a/client/tests/kvm/tests/qmp_basic.py
+++ b/client/tests/kvm/tests/qmp_basic.py
@@ -383,13 +383,16 @@ def run_qmp_basic(test, params, env):
 
     vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
 
+    if vm.qmp_monitor is None:
+        raise error.TestError("Could not find a QMP monitor, abort test.")
+
     # Run all suites
-    greeting_suite(vm.monitor)
-    input_object_suite(vm.monitor)
-    argument_checker_suite(vm.monitor)
-    unknown_commands_suite(vm.monitor)
-    json_parsing_errors_suite(vm.monitor)
+    greeting_suite(vm.qmp_monitor)
+    input_object_suite(vm.qmp_monitor)
+    argument_checker_suite(vm.qmp_monitor)
+    unknown_commands_suite(vm.qmp_monitor)
+    json_parsing_errors_suite(vm.qmp_monitor)
 
     # check if QMP is still alive
-    if not vm.monitor.is_responsive():
+    if not vm.qmp_monitor.is_responsive():
         raise error.TestFail('QEMU is not alive after QMP testing')
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2 v2] KVM Test: Fix qmp_basic test failure in qmp-kvm-0.12.*
       [not found] <1294383461-9425-1-git-send-email-qzhou@redhat.com>
  2011-01-10 10:47 ` [PATCH 1/2 RESEND] KVM Test: Introduce qmp_monitor property in 'kvm_vm.VM' qzhou
@ 2011-01-10 10:48 ` qzhou
  2011-01-11 16:58   ` Lucas Meneghel Rodrigues
  1 sibling, 1 reply; 6+ messages in thread
From: qzhou @ 2011-01-10 10:48 UTC (permalink / raw)
  To: autotest; +Cc: kvm

From: Qingtang Zhou <qzhou@redhat.com>

QMP in qemu-kvm-0.12.* has some difference from QMP in qemu-kvm-0.13.*.
These difference cause 'qmp_basic' test fail while running on older qemu-kvm.

This patch will fix these failures, make 'qmp_basic' runs happily.

Changelog from v1:
 - fix errors in 'test_version' function, make sure qmp verrsion can be got
   currectly in qemu-kvm-0.13.*

Signed-off-by: Qingtang Zhou <qzhou@redhat.com>
---
 client/tests/kvm/tests/qmp_basic.py |  114 ++++++++++++++++++++++------------
 1 files changed, 74 insertions(+), 40 deletions(-)

diff --git a/client/tests/kvm/tests/qmp_basic.py b/client/tests/kvm/tests/qmp_basic.py
index 985ad15..11091da 100644
--- a/client/tests/kvm/tests/qmp_basic.py
+++ b/client/tests/kvm/tests/qmp_basic.py
@@ -1,4 +1,4 @@
-import kvm_test_utils
+import kvm_test_utils, logging
 from autotest_lib.client.common_lib import error
 
 def run_qmp_basic(test, params, env):
@@ -29,6 +29,8 @@ def run_qmp_basic(test, params, env):
         o Are all those check_*() functions really needed? Wouldn't a
           specialized class (eg. a Response class) do better?
     """
+    qmp_version = []
+
     def fail_no_key(qmp_dict, key):
         if not isinstance(qmp_dict, dict):
             raise error.TestFail("qmp_dict is not a dict (it's '%s')" %
@@ -49,21 +51,24 @@ def run_qmp_basic(test, params, env):
         If any of these checks fails, error.TestFail is raised.
         """
         fail_no_key(qmp_dict, key)
-        if not isinstance(qmp_dict[key], keytype):
-            raise error.TestFail("'%s' key is not of type '%s', it's '%s'" %
-                                 (key, keytype, type(qmp_dict[key])))
+        if isinstance(qmp_dict[key], keytype):
+            return True
+
+        logging.error("'%s' key is not of type '%s', it's '%s'",
+                      key, keytype, type(qmp_dict[key]))
+        return False
 
 
     def check_key_is_dict(qmp_dict, key):
-        check_dict_key(qmp_dict, key, dict)
+        return check_dict_key(qmp_dict, key, dict)
 
 
     def check_key_is_list(qmp_dict, key):
-        check_dict_key(qmp_dict, key, list)
+        return check_dict_key(qmp_dict, key, list)
 
 
     def check_key_is_str(qmp_dict, key):
-        check_dict_key(qmp_dict, key, unicode)
+        return check_dict_key(qmp_dict, key, unicode)
 
 
     def check_str_key(qmp_dict, keyname, value=None):
@@ -76,11 +81,10 @@ def run_qmp_basic(test, params, env):
     def check_key_is_int(qmp_dict, key):
         fail_no_key(qmp_dict, key)
         try:
-            value = int(qmp_dict[key])
+            int(qmp_dict[key])
         except:
-            raise error.TestFail("'%s' key is not of type int, it's '%s'" %
-                                 (key, type(qmp_dict[key])))
-
+            return False
+        return True
 
     def check_bool_key(qmp_dict, keyname, value=None):
         check_dict_key(qmp_dict, keyname, bool)
@@ -110,6 +114,7 @@ def run_qmp_basic(test, params, env):
         @param classname: Expected error class name
         @param datadict: Expected error data dictionary
         """
+        logging.debug("resp %s", str(resp))
         check_key_is_dict(resp, "error")
         check_key_is_str(resp["error"], "class")
         if classname and resp["error"]["class"] != classname:
@@ -129,9 +134,25 @@ def run_qmp_basic(test, params, env):
         { "qemu": { "major": json-int, "minor": json-int, "micro": json-int }
           "package": json-string }
         """
-        check_key_is_dict(version, "qemu")
-        for key in [ "major", "minor", "micro" ]:
-            check_key_is_int(version["qemu"], key)
+        success = check_key_is_dict(version, "qemu")
+        if success:
+            for key in [ "major", "minor", "micro" ]:
+                success = check_key_is_int(version["qemu"], key)
+                if not success:
+		    raise error.TestFail("'%s' key is not of type int, "
+                                         "it's '%s'" %
+					 (key, type(version["qemu"][key])))
+
+	        qmp_version.append(int(version["qemu"][key]))
+
+        else:
+            success = check_key_is_str(version, "qemu")
+            if not success:
+                raise error.TestFail("'qemu' key is neither 'dict' nor 'str'")
+            qmp_version.extend(map(int, version["qemu"].split('.')))
+
+        logging.debug("got qemu version %s", str(qmp_version))
+
         check_key_is_str(version, "package")
 
 
@@ -224,8 +245,13 @@ def run_qmp_basic(test, params, env):
         names must be detected.
         """
         resp = monitor.cmd_obj({ "execute": "eject", "foobar": True })
-        check_error_resp(resp, "QMPExtraInputObjectMember",
-                         { "member": "foobar" })
+        if qmp_version[1] > 12:
+            expected_error = "QMPExtraInputObjectMember"
+            data_dict = {"member": "foobar"}
+        else:
+            expected_error = "MissingParameter"
+            data_dict = {"name": "device"}
+        check_error_resp(resp, expected_error, data_dict)
 
 
     def test_bad_arguments_key_type(monitor):
@@ -318,18 +344,37 @@ def run_qmp_basic(test, params, env):
         command used doesn't matter much as QMP performs argument checking
         _before_ calling the command.
         """
-        # stop doesn't take arguments
-        resp = monitor.cmd_qmp("stop", { "foo": 1 })
-        check_error_resp(resp, "InvalidParameter", { "name": "foo" })
-
-        # required argument omitted
-        resp = monitor.cmd_qmp("screendump")
-        check_error_resp(resp, "MissingParameter", { "name": "filename" })
+        # qmp 0.12.* is different from 0.13.*:
+        # 1. 'stop' command just return {} evenif stop have arguments.
+        # 2. there is no 'screendump' command.
+        # 3. argument isn't checked in 'device' command.
+        # so skip these tests in qmp 0.12.*.
+        if qmp_version[1] > 12:
+            # stop doesn't take arguments
+            resp = monitor.cmd_qmp("stop", { "foo": 1 })
+            check_error_resp(resp, "InvalidParameter", { "name": "foo" })
+
+            # required argument omitted
+            resp = monitor.cmd_qmp("screendump")
+            check_error_resp(resp, "MissingParameter", { "name": "filename" })
+
+            # 'bar' is not a valid argument
+            resp = monitor.cmd_qmp("screendump", { "filename": "outfile",
+                                                   "bar": "bar" })
+            check_error_resp(resp, "InvalidParameter", { "name": "bar"})
+
+            # filename argument must be a json-string
+            for arg in [ {}, [], 1, True ]:
+                resp = monitor.cmd_qmp("screendump", { "filename": arg })
+                check_error_resp(resp, "InvalidParameterType",
+                                 { "name": "filename", "expected": "string" })
+
+            # force argument must be a json-bool
+            for arg in [ {}, [], 1, "foo" ]:
+                resp = monitor.cmd_qmp("eject", { "force": arg, "device": "foo" })
+                check_error_resp(resp, "InvalidParameterType",
+                                 { "name": "force", "expected": "bool" })
 
-        # 'bar' is not a valid argument
-        resp = monitor.cmd_qmp("screendump", { "filename": "outfile",
-                                               "bar": "bar" })
-        check_error_resp(resp, "InvalidParameter", { "name": "bar"})
 
         # test optional argument: 'force' is omitted, but it's optional, so
         # the handler has to be called. Test this happens by checking an
@@ -337,18 +382,6 @@ def run_qmp_basic(test, params, env):
         resp = monitor.cmd_qmp("eject", { "device": "foobar" })
         check_error_resp(resp, "DeviceNotFound")
 
-        # filename argument must be a json-string
-        for arg in [ {}, [], 1, True ]:
-            resp = monitor.cmd_qmp("screendump", { "filename": arg })
-            check_error_resp(resp, "InvalidParameterType",
-                             { "name": "filename", "expected": "string" })
-
-        # force argument must be a json-bool
-        for arg in [ {}, [], 1, "foo" ]:
-            resp = monitor.cmd_qmp("eject", { "force": arg, "device": "foo" })
-            check_error_resp(resp, "InvalidParameterType",
-                             { "name": "force", "expected": "bool" })
-
         # val argument must be a json-int
         for arg in [ {}, [], True, "foo" ]:
             resp = monitor.cmd_qmp("memsave", { "val": arg, "filename": "foo",
@@ -377,8 +410,9 @@ def run_qmp_basic(test, params, env):
         """
         # We also call a HMP-only command, to be sure it will fail as expected
         for cmd in [ "bar", "query-", "query-foo", "q", "help" ]:
+            data_dict = { "name": cmd }
             resp = monitor.cmd_qmp(cmd)
-            check_error_resp(resp, "CommandNotFound", { "name": cmd })
+            check_error_resp(resp, "CommandNotFound", data_dict)
 
 
     vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
-- 
1.7.3.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2 RESEND] KVM Test: Introduce qmp_monitor property in 'kvm_vm.VM'
  2011-01-10 10:47 ` [PATCH 1/2 RESEND] KVM Test: Introduce qmp_monitor property in 'kvm_vm.VM' qzhou
@ 2011-01-10 17:52   ` Michael Goldish
  2011-01-11  3:55     ` Meneghel Rodrigues
  2011-01-11  4:00     ` Lucas Meneghel Rodrigues
  0 siblings, 2 replies; 6+ messages in thread
From: Michael Goldish @ 2011-01-10 17:52 UTC (permalink / raw)
  To: qzhou; +Cc: autotest, kvm, lcapitulino

On 01/10/2011 12:47 PM, qzhou@redhat.com wrote:
> From: Qingtang Zhou<qzhou@redhat.com>
>
> Introduce qmp_monitor property can help us easily get a qmp monitor
> of guest.
>
> CC to Luiz and kvm@vger.kernel.org
>
> Signed-off-by: Qingtang Zhou<qzhou@redhat.com>

The monitor property returns the main monitor.  Isn't it fair to assume 
that qmp_basic is always run with main_monitor set to a qmp monitor?

> ---
>   client/tests/kvm/kvm_vm.py          |   10 ++++++++++
>   client/tests/kvm/tests/qmp_basic.py |   15 +++++++++------
>   2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index f6f1684..0ba2db4 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -870,6 +870,16 @@ class VM:
>           if self.monitors and not self.params.get("main_monitor"):
>               return self.monitors[0]
>
> +    @property
> +    def qmp_monitor(self):
> +        """
> +        Return the first QMP monitor.
> +        If no QMP monitor exist, return None.
> +        """
> +        for m in self.monitors:
> +            if isinstance(m, kvm_monitor.QMPMonitor):
> +                return m
> +        return None
>
>       def is_alive(self):
>           """
> diff --git a/client/tests/kvm/tests/qmp_basic.py b/client/tests/kvm/tests/qmp_basic.py
> index 985ad15..36cbd78 100644
> --- a/client/tests/kvm/tests/qmp_basic.py
> +++ b/client/tests/kvm/tests/qmp_basic.py
> @@ -383,13 +383,16 @@ def run_qmp_basic(test, params, env):
>
>       vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
>
> +    if vm.qmp_monitor is None:
> +        raise error.TestError("Could not find a QMP monitor, abort test.")
> +
>       # Run all suites
> -    greeting_suite(vm.monitor)
> -    input_object_suite(vm.monitor)
> -    argument_checker_suite(vm.monitor)
> -    unknown_commands_suite(vm.monitor)
> -    json_parsing_errors_suite(vm.monitor)
> +    greeting_suite(vm.qmp_monitor)
> +    input_object_suite(vm.qmp_monitor)
> +    argument_checker_suite(vm.qmp_monitor)
> +    unknown_commands_suite(vm.qmp_monitor)
> +    json_parsing_errors_suite(vm.qmp_monitor)
>
>       # check if QMP is still alive
> -    if not vm.monitor.is_responsive():
> +    if not vm.qmp_monitor.is_responsive():
>           raise error.TestFail('QEMU is not alive after QMP testing')


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2 RESEND] KVM Test: Introduce qmp_monitor property in 'kvm_vm.VM'
  2011-01-10 17:52   ` Michael Goldish
@ 2011-01-11  3:55     ` Meneghel Rodrigues
  2011-01-11  4:00     ` Lucas Meneghel Rodrigues
  1 sibling, 0 replies; 6+ messages in thread
From: Meneghel Rodrigues @ 2011-01-11  3:55 UTC (permalink / raw)
  To: Michael Goldish; +Cc: autotest, kvm, lcapitulino, qzhou

----- Original Message -----
From: "Michael Goldish" <mgoldish@redhat.com>
To: qzhou@redhat.com
Cc: autotest@test.kernel.org, kvm@vger.kernel.org, lcapitulino@redhat.com
Sent: Monday, January 10, 2011 3:52:53 PM
Subject: Re: [PATCH 1/2 RESEND] KVM Test: Introduce qmp_monitor property in 'kvm_vm.VM'

On 01/10/2011 12:47 PM, qzhou@redhat.com wrote:
> From: Qingtang Zhou<qzhou@redhat.com>
>
> Introduce qmp_monitor property can help us easily get a qmp monitor
> of guest.
>
> CC to Luiz and kvm@vger.kernel.org
>
> Signed-off-by: Qingtang Zhou<qzhou@redhat.com>

The monitor property returns the main monitor.  Isn't it fair to assume 
that qmp_basic is always run with main_monitor set to a qmp monitor?

^ Well, with this patch we have one less requirement, which is to set main_monitor as the primary one, kvm autotest can just find the first appropriate monitor and return it. It's mostly a usability patch, makes easier to figure out when errors happen and put less configuration strain on users. The fact the user will see the message:

> +        raise error.TestError("Could not find a QMP monitor, abort test.")


In case no QMP monitor was found is very helpful.

> ---
>   client/tests/kvm/kvm_vm.py          |   10 ++++++++++
>   client/tests/kvm/tests/qmp_basic.py |   15 +++++++++------
>   2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> index f6f1684..0ba2db4 100755
> --- a/client/tests/kvm/kvm_vm.py
> +++ b/client/tests/kvm/kvm_vm.py
> @@ -870,6 +870,16 @@ class VM:
>           if self.monitors and not self.params.get("main_monitor"):
>               return self.monitors[0]
>
> +    @property
> +    def qmp_monitor(self):
> +        """
> +        Return the first QMP monitor.
> +        If no QMP monitor exist, return None.
> +        """
> +        for m in self.monitors:
> +            if isinstance(m, kvm_monitor.QMPMonitor):
> +                return m
> +        return None
>
>       def is_alive(self):
>           """
> diff --git a/client/tests/kvm/tests/qmp_basic.py b/client/tests/kvm/tests/qmp_basic.py
> index 985ad15..36cbd78 100644
> --- a/client/tests/kvm/tests/qmp_basic.py
> +++ b/client/tests/kvm/tests/qmp_basic.py
> @@ -383,13 +383,16 @@ def run_qmp_basic(test, params, env):
>
>       vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
>
> +    if vm.qmp_monitor is None:
> +        raise error.TestError("Could not find a QMP monitor, abort test.")
> +
>       # Run all suites
> -    greeting_suite(vm.monitor)
> -    input_object_suite(vm.monitor)
> -    argument_checker_suite(vm.monitor)
> -    unknown_commands_suite(vm.monitor)
> -    json_parsing_errors_suite(vm.monitor)
> +    greeting_suite(vm.qmp_monitor)
> +    input_object_suite(vm.qmp_monitor)
> +    argument_checker_suite(vm.qmp_monitor)
> +    unknown_commands_suite(vm.qmp_monitor)
> +    json_parsing_errors_suite(vm.qmp_monitor)
>
>       # check if QMP is still alive
> -    if not vm.monitor.is_responsive():
> +    if not vm.qmp_monitor.is_responsive():
>           raise error.TestFail('QEMU is not alive after QMP testing')

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2 RESEND] KVM Test: Introduce qmp_monitor property in 'kvm_vm.VM'
  2011-01-10 17:52   ` Michael Goldish
  2011-01-11  3:55     ` Meneghel Rodrigues
@ 2011-01-11  4:00     ` Lucas Meneghel Rodrigues
  1 sibling, 0 replies; 6+ messages in thread
From: Lucas Meneghel Rodrigues @ 2011-01-11  4:00 UTC (permalink / raw)
  To: Michael Goldish; +Cc: qzhou, autotest, kvm, lcapitulino

On Mon, 2011-01-10 at 19:52 +0200, Michael Goldish wrote:
> On 01/10/2011 12:47 PM, qzhou@redhat.com wrote:
> > From: Qingtang Zhou<qzhou@redhat.com>
> >
> > Introduce qmp_monitor property can help us easily get a qmp monitor
> > of guest.
> >
> > CC to Luiz and kvm@vger.kernel.org
> >
> > Signed-off-by: Qingtang Zhou<qzhou@redhat.com>
> 
> The monitor property returns the main monitor.  Isn't it fair to assume 
> that qmp_basic is always run with main_monitor set to a qmp monitor?

^ Well, with this patch we have one less requirement, which is to set
main_monitor as the primary one, kvm autotest can just find the first
appropriate monitor and return it. It's mostly a usability patch, makes
easier to figure out when errors happen and put less configuration
strain on users. The fact the user will see the message:

> +        raise error.TestError("Could not find a QMP monitor, abort test.")

In case no QMP monitor was found is very helpful.

> > ---
> >   client/tests/kvm/kvm_vm.py          |   10 ++++++++++
> >   client/tests/kvm/tests/qmp_basic.py |   15 +++++++++------
> >   2 files changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py
> > index f6f1684..0ba2db4 100755
> > --- a/client/tests/kvm/kvm_vm.py
> > +++ b/client/tests/kvm/kvm_vm.py
> > @@ -870,6 +870,16 @@ class VM:
> >           if self.monitors and not self.params.get("main_monitor"):
> >               return self.monitors[0]
> >
> > +    @property
> > +    def qmp_monitor(self):
> > +        """
> > +        Return the first QMP monitor.
> > +        If no QMP monitor exist, return None.
> > +        """
> > +        for m in self.monitors:
> > +            if isinstance(m, kvm_monitor.QMPMonitor):
> > +                return m
> > +        return None
> >
> >       def is_alive(self):
> >           """
> > diff --git a/client/tests/kvm/tests/qmp_basic.py b/client/tests/kvm/tests/qmp_basic.py
> > index 985ad15..36cbd78 100644
> > --- a/client/tests/kvm/tests/qmp_basic.py
> > +++ b/client/tests/kvm/tests/qmp_basic.py
> > @@ -383,13 +383,16 @@ def run_qmp_basic(test, params, env):
> >
> >       vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> >
> > +    if vm.qmp_monitor is None:
> > +        raise error.TestError("Could not find a QMP monitor, abort test.")
> > +
> >       # Run all suites
> > -    greeting_suite(vm.monitor)
> > -    input_object_suite(vm.monitor)
> > -    argument_checker_suite(vm.monitor)
> > -    unknown_commands_suite(vm.monitor)
> > -    json_parsing_errors_suite(vm.monitor)
> > +    greeting_suite(vm.qmp_monitor)
> > +    input_object_suite(vm.qmp_monitor)
> > +    argument_checker_suite(vm.qmp_monitor)
> > +    unknown_commands_suite(vm.qmp_monitor)
> > +    json_parsing_errors_suite(vm.qmp_monitor)
> >
> >       # check if QMP is still alive
> > -    if not vm.monitor.is_responsive():
> > +    if not vm.qmp_monitor.is_responsive():
> >           raise error.TestFail('QEMU is not alive after QMP testing')
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2 v2] KVM Test: Fix qmp_basic test failure in qmp-kvm-0.12.*
  2011-01-10 10:48 ` [PATCH 2/2 v2] KVM Test: Fix qmp_basic test failure in qmp-kvm-0.12.* qzhou
@ 2011-01-11 16:58   ` Lucas Meneghel Rodrigues
  0 siblings, 0 replies; 6+ messages in thread
From: Lucas Meneghel Rodrigues @ 2011-01-11 16:58 UTC (permalink / raw)
  To: qzhou; +Cc: autotest, kvm

On Mon, 2011-01-10 at 18:48 +0800, qzhou@redhat.com wrote:
> From: Qingtang Zhou <qzhou@redhat.com>
> 
> QMP in qemu-kvm-0.12.* has some difference from QMP in qemu-kvm-0.13.*.
> These difference cause 'qmp_basic' test fail while running on older qemu-kvm.
> 
> This patch will fix these failures, make 'qmp_basic' runs happily.

After talking to Luiz and Michael, we think it is a better idea instead
of handling all testing in qmp_basic, we should rather write a special
test for RHEL 6's qmp. Please write an alternate test qmp_simple_rhel6
or any better name you can find, and not change qmp_basic, OK?

As for the 1st patch on the series, we think it is unnecessary to add a
new property to the VM class only for this use case. I recreated the
patch moving the verification code only to qmp_basic and will apply it.

Thanks!

> Changelog from v1:
>  - fix errors in 'test_version' function, make sure qmp verrsion can be got
>    currectly in qemu-kvm-0.13.*
> 
> Signed-off-by: Qingtang Zhou <qzhou@redhat.com>
> ---
>  client/tests/kvm/tests/qmp_basic.py |  114 ++++++++++++++++++++++------------
>  1 files changed, 74 insertions(+), 40 deletions(-)
> 
> diff --git a/client/tests/kvm/tests/qmp_basic.py b/client/tests/kvm/tests/qmp_basic.py
> index 985ad15..11091da 100644
> --- a/client/tests/kvm/tests/qmp_basic.py
> +++ b/client/tests/kvm/tests/qmp_basic.py
> @@ -1,4 +1,4 @@
> -import kvm_test_utils
> +import kvm_test_utils, logging
>  from autotest_lib.client.common_lib import error
>  
>  def run_qmp_basic(test, params, env):
> @@ -29,6 +29,8 @@ def run_qmp_basic(test, params, env):
>          o Are all those check_*() functions really needed? Wouldn't a
>            specialized class (eg. a Response class) do better?
>      """
> +    qmp_version = []
> +
>      def fail_no_key(qmp_dict, key):
>          if not isinstance(qmp_dict, dict):
>              raise error.TestFail("qmp_dict is not a dict (it's '%s')" %
> @@ -49,21 +51,24 @@ def run_qmp_basic(test, params, env):
>          If any of these checks fails, error.TestFail is raised.
>          """
>          fail_no_key(qmp_dict, key)
> -        if not isinstance(qmp_dict[key], keytype):
> -            raise error.TestFail("'%s' key is not of type '%s', it's '%s'" %
> -                                 (key, keytype, type(qmp_dict[key])))
> +        if isinstance(qmp_dict[key], keytype):
> +            return True
> +
> +        logging.error("'%s' key is not of type '%s', it's '%s'",
> +                      key, keytype, type(qmp_dict[key]))
> +        return False
>  
> 
>      def check_key_is_dict(qmp_dict, key):
> -        check_dict_key(qmp_dict, key, dict)
> +        return check_dict_key(qmp_dict, key, dict)
>  
> 
>      def check_key_is_list(qmp_dict, key):
> -        check_dict_key(qmp_dict, key, list)
> +        return check_dict_key(qmp_dict, key, list)
>  
> 
>      def check_key_is_str(qmp_dict, key):
> -        check_dict_key(qmp_dict, key, unicode)
> +        return check_dict_key(qmp_dict, key, unicode)
>  
> 
>      def check_str_key(qmp_dict, keyname, value=None):
> @@ -76,11 +81,10 @@ def run_qmp_basic(test, params, env):
>      def check_key_is_int(qmp_dict, key):
>          fail_no_key(qmp_dict, key)
>          try:
> -            value = int(qmp_dict[key])
> +            int(qmp_dict[key])
>          except:
> -            raise error.TestFail("'%s' key is not of type int, it's '%s'" %
> -                                 (key, type(qmp_dict[key])))
> -
> +            return False
> +        return True
>  
>      def check_bool_key(qmp_dict, keyname, value=None):
>          check_dict_key(qmp_dict, keyname, bool)
> @@ -110,6 +114,7 @@ def run_qmp_basic(test, params, env):
>          @param classname: Expected error class name
>          @param datadict: Expected error data dictionary
>          """
> +        logging.debug("resp %s", str(resp))
>          check_key_is_dict(resp, "error")
>          check_key_is_str(resp["error"], "class")
>          if classname and resp["error"]["class"] != classname:
> @@ -129,9 +134,25 @@ def run_qmp_basic(test, params, env):
>          { "qemu": { "major": json-int, "minor": json-int, "micro": json-int }
>            "package": json-string }
>          """
> -        check_key_is_dict(version, "qemu")
> -        for key in [ "major", "minor", "micro" ]:
> -            check_key_is_int(version["qemu"], key)
> +        success = check_key_is_dict(version, "qemu")
> +        if success:
> +            for key in [ "major", "minor", "micro" ]:
> +                success = check_key_is_int(version["qemu"], key)
> +                if not success:
> +		    raise error.TestFail("'%s' key is not of type int, "
> +                                         "it's '%s'" %
> +					 (key, type(version["qemu"][key])))
> +
> +	        qmp_version.append(int(version["qemu"][key]))
> +
> +        else:
> +            success = check_key_is_str(version, "qemu")
> +            if not success:
> +                raise error.TestFail("'qemu' key is neither 'dict' nor 'str'")
> +            qmp_version.extend(map(int, version["qemu"].split('.')))
> +
> +        logging.debug("got qemu version %s", str(qmp_version))
> +
>          check_key_is_str(version, "package")
>  
> 
> @@ -224,8 +245,13 @@ def run_qmp_basic(test, params, env):
>          names must be detected.
>          """
>          resp = monitor.cmd_obj({ "execute": "eject", "foobar": True })
> -        check_error_resp(resp, "QMPExtraInputObjectMember",
> -                         { "member": "foobar" })
> +        if qmp_version[1] > 12:
> +            expected_error = "QMPExtraInputObjectMember"
> +            data_dict = {"member": "foobar"}
> +        else:
> +            expected_error = "MissingParameter"
> +            data_dict = {"name": "device"}
> +        check_error_resp(resp, expected_error, data_dict)
>  
> 
>      def test_bad_arguments_key_type(monitor):
> @@ -318,18 +344,37 @@ def run_qmp_basic(test, params, env):
>          command used doesn't matter much as QMP performs argument checking
>          _before_ calling the command.
>          """
> -        # stop doesn't take arguments
> -        resp = monitor.cmd_qmp("stop", { "foo": 1 })
> -        check_error_resp(resp, "InvalidParameter", { "name": "foo" })
> -
> -        # required argument omitted
> -        resp = monitor.cmd_qmp("screendump")
> -        check_error_resp(resp, "MissingParameter", { "name": "filename" })
> +        # qmp 0.12.* is different from 0.13.*:
> +        # 1. 'stop' command just return {} evenif stop have arguments.
> +        # 2. there is no 'screendump' command.
> +        # 3. argument isn't checked in 'device' command.
> +        # so skip these tests in qmp 0.12.*.
> +        if qmp_version[1] > 12:
> +            # stop doesn't take arguments
> +            resp = monitor.cmd_qmp("stop", { "foo": 1 })
> +            check_error_resp(resp, "InvalidParameter", { "name": "foo" })
> +
> +            # required argument omitted
> +            resp = monitor.cmd_qmp("screendump")
> +            check_error_resp(resp, "MissingParameter", { "name": "filename" })
> +
> +            # 'bar' is not a valid argument
> +            resp = monitor.cmd_qmp("screendump", { "filename": "outfile",
> +                                                   "bar": "bar" })
> +            check_error_resp(resp, "InvalidParameter", { "name": "bar"})
> +
> +            # filename argument must be a json-string
> +            for arg in [ {}, [], 1, True ]:
> +                resp = monitor.cmd_qmp("screendump", { "filename": arg })
> +                check_error_resp(resp, "InvalidParameterType",
> +                                 { "name": "filename", "expected": "string" })
> +
> +            # force argument must be a json-bool
> +            for arg in [ {}, [], 1, "foo" ]:
> +                resp = monitor.cmd_qmp("eject", { "force": arg, "device": "foo" })
> +                check_error_resp(resp, "InvalidParameterType",
> +                                 { "name": "force", "expected": "bool" })
>  
> -        # 'bar' is not a valid argument
> -        resp = monitor.cmd_qmp("screendump", { "filename": "outfile",
> -                                               "bar": "bar" })
> -        check_error_resp(resp, "InvalidParameter", { "name": "bar"})
>  
>          # test optional argument: 'force' is omitted, but it's optional, so
>          # the handler has to be called. Test this happens by checking an
> @@ -337,18 +382,6 @@ def run_qmp_basic(test, params, env):
>          resp = monitor.cmd_qmp("eject", { "device": "foobar" })
>          check_error_resp(resp, "DeviceNotFound")
>  
> -        # filename argument must be a json-string
> -        for arg in [ {}, [], 1, True ]:
> -            resp = monitor.cmd_qmp("screendump", { "filename": arg })
> -            check_error_resp(resp, "InvalidParameterType",
> -                             { "name": "filename", "expected": "string" })
> -
> -        # force argument must be a json-bool
> -        for arg in [ {}, [], 1, "foo" ]:
> -            resp = monitor.cmd_qmp("eject", { "force": arg, "device": "foo" })
> -            check_error_resp(resp, "InvalidParameterType",
> -                             { "name": "force", "expected": "bool" })
> -
>          # val argument must be a json-int
>          for arg in [ {}, [], True, "foo" ]:
>              resp = monitor.cmd_qmp("memsave", { "val": arg, "filename": "foo",
> @@ -377,8 +410,9 @@ def run_qmp_basic(test, params, env):
>          """
>          # We also call a HMP-only command, to be sure it will fail as expected
>          for cmd in [ "bar", "query-", "query-foo", "q", "help" ]:
> +            data_dict = { "name": cmd }
>              resp = monitor.cmd_qmp(cmd)
> -            check_error_resp(resp, "CommandNotFound", { "name": cmd })
> +            check_error_resp(resp, "CommandNotFound", data_dict)
>  
> 
>      vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-01-11 16:58 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1294383461-9425-1-git-send-email-qzhou@redhat.com>
2011-01-10 10:47 ` [PATCH 1/2 RESEND] KVM Test: Introduce qmp_monitor property in 'kvm_vm.VM' qzhou
2011-01-10 17:52   ` Michael Goldish
2011-01-11  3:55     ` Meneghel Rodrigues
2011-01-11  4:00     ` Lucas Meneghel Rodrigues
2011-01-10 10:48 ` [PATCH 2/2 v2] KVM Test: Fix qmp_basic test failure in qmp-kvm-0.12.* qzhou
2011-01-11 16:58   ` Lucas Meneghel Rodrigues

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox