| Message ID | 20110421062112.9564.51395.stgit@t (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 21, 2011 at 3:21 AM, Amos Kong <akong@redhat.com> 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 <jasowang@redhat.com> > Signed-off-by: Amos Kong <akong@redhat.com> > --- > client/tests/kvm/tests/stop_continue.py | 52 +++++++++++++++++++++++++++++++ > client/tests/kvm/tests_base.cfg.sample | 4 ++ > 2 files changed, 56 insertions(+), 0 deletions(-) > create mode 100644 client/tests/kvm/tests/stop_continue.py > > diff --git a/client/tests/kvm/tests/stop_continue.py b/client/tests/kvm/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): > + """ > + Suspend a running Virtual Machine and verify its state. > + > + 1) Boot the vm > + 2) Suspend the vm through stop command > + 3) Verify the state through info status command > + 4) Check is the ssh session to guest is still responsive, > + if succeed, fail the test. > + > + @param test: Kvm test object > + @param params: Dictionary with the test parameters > + @param env: Dictionary with test environment. > + """ > + vm = env.get_vm(params["main_vm"]) > + vm.verify_alive() > + timeout = float(params.get("login_timeout", 240)) > + session = vm.wait_for_login(timeout=timeout) > + > + try: > + logging.info("Suspend the virtual machine") > + vm.monitor.cmd("stop") > + > + logging.info("Verifying the status of virtual machine through monitor") > + o = vm.monitor.info("status") > + if 'paused' not in o and ( "u'running': False" not in 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 monitor' 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) approach 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. > + logging.error(o) > + raise error.TestFail("Fail to suspend through monitor command line") > + > + logging.info("Check the session responsiveness") > + if session.is_responsive(): > + raise error.TestFail("Session is still responsive after stop") > + > + logging.info("Try to resume the guest") > + vm.monitor.cmd("cont") > + > + o = vm.monitor.info("status") > + m_type = params.get("monitor_type", "human") > + if ('human' in m_type and 'running' not in o) or\ > + ('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.
diff --git a/client/tests/kvm/tests/stop_continue.py b/client/tests/kvm/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): + """ + Suspend a running Virtual Machine and verify its state. + + 1) Boot the vm + 2) Suspend the vm through stop command + 3) Verify the state through info status command + 4) Check is the ssh session to guest is still responsive, + if succeed, fail the test. + + @param test: Kvm test object + @param params: Dictionary with the test parameters + @param env: Dictionary with test environment. + """ + vm = env.get_vm(params["main_vm"]) + vm.verify_alive() + timeout = float(params.get("login_timeout", 240)) + session = vm.wait_for_login(timeout=timeout) + + try: + logging.info("Suspend the virtual machine") + vm.monitor.cmd("stop") + + logging.info("Verifying the status of virtual machine through monitor") + o = vm.monitor.info("status") + if 'paused' not in o and ( "u'running': False" not in str(o)): + logging.error(o) + raise error.TestFail("Fail to suspend through monitor command line") + + logging.info("Check the session responsiveness") + if session.is_responsive(): + raise error.TestFail("Session is still responsive after stop") + + logging.info("Try to resume the guest") + vm.monitor.cmd("cont") + + o = vm.monitor.info("status") + m_type = params.get("monitor_type", "human") + if ('human' in m_type and 'running' not in o) or\ + ('qmp' in m_type and "u'running': True" not in str(o)): + logging.error(o) + raise error.TestFail("Could not continue the execution") + + logging.info("Try to re-log into guest") + session = vm.wait_for_login(timeout=timeout) + + finally: + session.close() diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample index 5d274f8..7333ed0 100644 --- a/client/tests/kvm/tests_base.cfg.sample +++ b/client/tests/kvm/tests_base.cfg.sample @@ -260,6 +260,10 @@ variants: - systemtap: test_control_file = systemtap.control + - stop_continue: + type = stop_continue + kill_vm_on_error = yes + - linux_s3: install setup unattended_install.cdrom only Linux type = linux_s3