From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lucas Meneghel Rodrigues Subject: Re: [Autotest] [Autotest PATCH] KVM-test: Simple stop/continue test Date: Fri, 29 Apr 2011 02:05:27 -0300 Message-ID: References: <20110421062112.9564.51395.stgit@t> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: autotest@test.kernel.org, kvm@vger.kernel.org To: Amos Kong Return-path: Received: from mail-ey0-f174.google.com ([209.85.215.174]:53073 "EHLO mail-ey0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751309Ab1D2FFa convert rfc822-to-8bit (ORCPT ); Fri, 29 Apr 2011 01:05:30 -0400 Received: by eyx24 with SMTP id 24so1035301eyx.19 for ; Thu, 28 Apr 2011 22:05:28 -0700 (PDT) In-Reply-To: <20110421062112.9564.51395.stgit@t> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Apr 21, 2011 at 3:21 AM, Amos Kong wrote: > Change guest state by monitor cmd, verify guest status, > and try to login guest by network. I don't like the way you're handling human monitor and QMP monitors in this tests... comments below: > Signed-off-by: Jason Wang > Signed-off-by: Amos Kong > --- > =A0client/tests/kvm/tests/stop_continue.py | =A0 52 +++++++++++++++++= ++++++++++++++ > =A0client/tests/kvm/tests_base.cfg.sample =A0| =A0 =A04 ++ > =A02 files changed, 56 insertions(+), 0 deletions(-) > =A0create mode 100644 client/tests/kvm/tests/stop_continue.py > > diff --git a/client/tests/kvm/tests/stop_continue.py b/client/tests/k= vm/tests/stop_continue.py > new file mode 100644 > index 0000000..c7d8025 > --- /dev/null > +++ b/client/tests/kvm/tests/stop_continue.py > @@ -0,0 +1,52 @@ > +import logging > +from autotest_lib.client.common_lib import error > + > + > +def run_stop_continue(test, params, env): > + =A0 =A0""" > + =A0 =A0Suspend a running Virtual Machine and verify its state. > + > + =A0 =A01) Boot the vm > + =A0 =A02) Suspend the vm through stop command > + =A0 =A03) Verify the state through info status command > + =A0 =A04) Check is the ssh session to guest is still responsive, > + =A0 =A0 =A0 if succeed, fail the test. > + > + =A0 =A0@param test: Kvm test object > + =A0 =A0@param params: Dictionary with the test parameters > + =A0 =A0@param env: Dictionary with test environment. > + =A0 =A0""" > + =A0 =A0vm =3D env.get_vm(params["main_vm"]) > + =A0 =A0vm.verify_alive() > + =A0 =A0timeout =3D float(params.get("login_timeout", 240)) > + =A0 =A0session =3D vm.wait_for_login(timeout=3Dtimeout) > + > + =A0 =A0try: > + =A0 =A0 =A0 =A0logging.info("Suspend the virtual machine") > + =A0 =A0 =A0 =A0vm.monitor.cmd("stop") > + > + =A0 =A0 =A0 =A0logging.info("Verifying the status of virtual machin= e through monitor") > + =A0 =A0 =A0 =A0o =3D vm.monitor.info("status") > + =A0 =A0 =A0 =A0if 'paused' not in o and ( "u'running': False" not i= n str(o)): ^ Here, it's not clear what means a paused qmp monitor or a hmp monitor. this statement is unnecessarily confusing. Here 'paused' not in o -> Is what would define a 'not paused hmp monitor' "u'running': False" not in str(o) -> This defines a 'not paused qmp mon= itor' why we are checking one _and_ the other, as one monitor can't be hmp and qmp at the same time? It would be at least _or_. And like I said, it's non trivial to flow this assertion made. It seems to me that a better (although involving more code changes) app= roach is: 1) Introduce VM methods is_paused and verify_paused, which would internally for the kvm vm class, call monitor methods also called is_paused and verify_paused, with implementations for both hmp and qmp. verify_paused would throw an exception in case of a failure, while is_paused would return a boolean. > + =A0 =A0 =A0 =A0 =A0 =A0logging.error(o) > + =A0 =A0 =A0 =A0 =A0 =A0raise error.TestFail("Fail to suspend throug= h monitor command line") > + > + =A0 =A0 =A0 =A0logging.info("Check the session responsiveness") > + =A0 =A0 =A0 =A0if session.is_responsive(): > + =A0 =A0 =A0 =A0 =A0 =A0raise error.TestFail("Session is still respo= nsive after stop") > + > + =A0 =A0 =A0 =A0logging.info("Try to resume the guest") > + =A0 =A0 =A0 =A0vm.monitor.cmd("cont") > + > + =A0 =A0 =A0 =A0o =3D vm.monitor.info("status") > + =A0 =A0 =A0 =A0m_type =3D params.get("monitor_type", "human") > + =A0 =A0 =A0 =A0if ('human' in m_type and 'running' not in o) or\ > + =A0 =A0 =A0 =A0 =A0 ('qmp' in m_type and "u'running': True" not in = str(o)): ^ same here, we should have is_running and verify_running methods on VM that would call monitor methods with the same names. Now, of course I might be really mistaken here, would like to hear your opinion on that subject. --=20 Lucas