On Tue, Oct 13, 2009 at 05:29:40PM -0300, Lucas Meneghel Rodrigues wrote:
> Hi Yolkfull and Chen:
>
> Thanks for your test! I have some comments and doubts to clear, most
> of them are about content of the messages delivered for the user and
> some other details.
>
> On Sun, Sep 27, 2009 at 6:11 AM, Yolkfull Chow <yzhou@xxxxxxxxxx> wrote:
> > For this case, Ken Cao wrote the linux part previously and I did extensive
> > modifications on Windows platform support.
> >
> > Signed-off-by: Ken Cao <kcao@xxxxxxxxxx>
> > Signed-off-by: Yolkfull Chow <yzhou@xxxxxxxxxx>
> > ---
> > client/tests/kvm/kvm_tests.cfg.sample | 14 +++++++
> > client/tests/kvm/tests/guest_s4.py | 66 +++++++++++++++++++++++++++++++++
> > 2 files changed, 80 insertions(+), 0 deletions(-)
> > create mode 100644 client/tests/kvm/tests/guest_s4.py
> >
> > diff --git a/client/tests/kvm/kvm_tests.cfg.sample b/client/tests/kvm/kvm_tests.cfg.sample
> > index 285a38f..f9ecb61 100644
> > --- a/client/tests/kvm/kvm_tests.cfg.sample
> > +++ b/client/tests/kvm/kvm_tests.cfg.sample
> > @@ -94,6 +94,14 @@ variants:
> > - linux_s3: install setup
> > type = linux_s3
> >
> > + - guest_s4:
> > + type = guest_s4
> > + check_s4_support_cmd = grep -q disk /sys/power/state
> > + test_s4_cmd = "cd /tmp/;nohup tcpdump -q -t ip host localhost"
> > + check_s4_cmd = pgrep tcpdump
> > + set_s4_cmd = echo disk > /sys/power/state
> > + kill_test_s4_cmd = pkill tcpdump
> > +
> > - timedrift: install setup
> > type = timedrift
> > extra_params += " -rtc-td-hack"
> > @@ -382,6 +390,12 @@ variants:
> > # Alternative host load:
> > #host_load_command = "dd if=/dev/urandom of=/dev/null"
> > host_load_instances = 8
> > + guest_s4:
> > + check_s4_support_cmd = powercfg /hibernate on
> > + test_s4_cmd = start /B ping -n 3000 localhost
> > + check_s4_cmd = tasklist | find /I "ping"
> > + set_s4_cmd = rundll32.exe PowrProf.dll, SetSuspendState
> > + kill_test_s4_cmd = taskkill /IM ping.exe /F
> >
> > variants:
> > - Win2000:
> > diff --git a/client/tests/kvm/tests/guest_s4.py b/client/tests/kvm/tests/guest_s4.py
> > new file mode 100644
> > index 0000000..5d8fbdf
> > --- /dev/null
> > +++ b/client/tests/kvm/tests/guest_s4.py
> > @@ -0,0 +1,66 @@
> > +import logging, time
> > +from autotest_lib.client.common_lib import error
> > +import kvm_test_utils, kvm_utils
> > +
> > +
> > +def run_guest_s4(test, params, env):
> > + """
> > + Suspend guest to disk,supports both Linux & Windows OSes.
> > +
> > + @param test: kvm test object.
> > + @param params: Dictionary with test parameters.
> > + @param env: Dictionary with the test environment.
> > + """
> > + vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> > + session = kvm_test_utils.wait_for_login(vm)
> > +
> > + logging.info("Checking whether VM supports S4")
> > + status = session.get_command_status(params.get("check_s4_support_cmd"))
> > + if status is None:
> > + logging.error("Failed to check if S4 exists")
> > + elif status != 0:
> > + raise error.TestFail("Guest does not support S4")
> > +
> > + logging.info("Waiting for a while for X to start...")
>
> Yes, generally X starts a bit later than the SSH service, so I
> understand the time being here, however:
>
> * In fact we are waiting for all services of the guest to be up and
> functional, so depending on the level of load, I don't think 10s is
> gonna make it. So I suggest something >= 30s
Yeah,reasonable, we did ignore the circumstance with workload. But as
you metioned,it can depend on different level of workload, therefore 30s
may be not enough as well. Your idea that write a utility function
waiting for some services up is good I think, thus it could be something
like:
def wait_services_up(services_list):
...
and for this case:
wait_services_up(["Xorg"]) for Linux and
wait_services_up(["explore.exe"]) for Windows.
> * It's also true that just wait for a given time and hope that it
> will be OK kinda sucks, so ideally we need to write utility functions
> to stablish as well as possible when all services of a host are fully
> booted up. Stated this way, it looks simple, but it's not.
>
> Autotest experience suggests that there's no real sane way to
> determine when a linux box is booted up, but we can take a
> semi-rational approach and verify if all services for the current run
> level have the status up or a similar approach. For windows, I was
> talking to Yaniv Kaul and it seems that processing the output of the
> 'sc query' command might give what we want. Bottom line, I'd like to
> add a TODO item, and write a function to stablish (fairly confidently)
> that a windows/linux guest is booted up.
>
> > + time.sleep(10)
> > +
> > + # Start up a program(tcpdump for linux OS & ping for M$ OS), as a flag.
> > + # If the program died after suspend, then fails this testcase.
> > + test_s4_cmd = params.get("test_s4_cmd")
> > + session.sendline(test_s4_cmd)
> > +
> > + # Get the second session to start S4
> > + session2 = kvm_test_utils.wait_for_login(vm)
> > +
> > + check_s4_cmd = params.get("check_s4_cmd")
> > + if session2.get_command_status(check_s4_cmd):
> > + raise error.TestError("Failed to launch %s background" % test_s4_cmd)
> > + logging.info("Launched command background in guest: %s" % test_s4_cmd)
> > +
> > + # Implement S4
> > + logging.info("Start suspend to disk now...")
> > + session2.sendline(params.get("set_s4_cmd"))
> > +
> > + if not kvm_utils.wait_for(vm.is_dead, 360, 30, 2):
> > + raise error.TestFail("VM refuse to go down,suspend failed")
> > + logging.info("VM suspended successfully.")
> > +
> > + logging.info("VM suspended to disk. sleep 10 seconds to have a break...")
> > + time.sleep(10)
> > +
> > + # Start vm, and check whether the program is still running
> > + logging.info("Restart VM now...")
> > +
> > + if not vm.create():
> > + raise error.TestError("failed to start the vm again.")
> > + if not vm.is_alive():
> > + raise error.TestError("VM seems to be dead; Test requires a live VM.")
> > +
> > + # Check whether test command still alive
> > + if session2.get_command_status(check_s4_cmd):
> > + raise error.TestFail("%s died, indicating that S4 failed" % test_s4_cmd)
>
> My doubt here is if by issuing a get_command_status we are actually
> checking if the first command issued is alive. I am under the
> impression that this will only create a new process with the same
> command line, and hence, we are not checking if the background command
> is alive... Perhaps check if the command is alive using ps/windows
> task manager or something?
We did. As we can see the value of variable 'check_s4_cmd' in config file,
it's 'ps' for Linux guests and 'tasklist' for Windows OSes.
The reason I used a second session(session2) is that for Windows guest,
after executing some command background, it will persistently print
output which will result in get_command_status timeout.
Let me know whether this answers your questions. ;-)
>
> About the wording of some messages, I have here a suggestion (full
> code that I had edited, see below), please take a look at it and let
> me know if you think it's OK.
>
> {{{
> import logging, time
> from autotest_lib.client.common_lib import error
> import kvm_test_utils, kvm_utils
>
>
> def run_guest_s4(test, params, env):
> """
> Suspend guest to disk,supports both Linux & Windows OSes.
>
> @param test: kvm test object.
> @param params: Dictionary with test parameters.
> @param env: Dictionary with the test environment.
> """
> vm = kvm_test_utils.get_living_vm(env, params.get("main_vm"))
> session = kvm_test_utils.wait_for_login(vm)
>
> logging.info("Checking whether VM supports S4")
> status = session.get_command_status(params.get("check_s4_support_cmd"))
> if status is None:
> logging.error("Failed to check if host supports S4")
^^^^ => "guest"
> elif status != 0:
> raise error.TestFail("Guest does not support S4")
>
> logging.info("Wait until all guest services are fully started")
> time.sleep(30)
==>
time.sleep(params.get("services_up_timeout")) ??
Anyway, in my opinion, we could now leave the time as configurable in
config file. And after wait_services_up() utility functions are available,
we send patch to fix this problem at that time.
What do you think?
>
> # Start up a program (tcpdump for linux & ping for Windows), as a flag.
> # If the program died after suspend, then fails this testcase.
> test_s4_cmd = params.get("test_s4_cmd")
> session.sendline(test_s4_cmd)
>
> # Get the second session to start S4
> session2 = kvm_test_utils.wait_for_login(vm)
>
> check_s4_cmd = params.get("check_s4_cmd")
> if session2.get_command_status(check_s4_cmd):
> raise error.TestError("Failed to launch '%s' as a background process" %
> test_s4_cmd)
> logging.info("Launched background command in guest: %s" % test_s4_cmd)
>
> # Suspend to disk
> logging.info("Start suspend to disk now...")
> session2.sendline(params.get("set_s4_cmd"))
>
> if not kvm_utils.wait_for(vm.is_dead, 360, 30, 2):
> raise error.TestFail("VM refuses to go down. Suspend failed")
> logging.info("VM suspended successfully. Wait before booting it again.")
> time.sleep(10)
>
> # Start vm, and check whether the program is still running
> logging.info("Start suspended VM...")
>
> if not vm.create():
> raise error.TestError("Failed to start the VM after suspend to disk")
> if not vm.is_alive():
> raise error.TestError("VM seems to be dead after it was suspended")
>
> # Check whether test command still alive
> logging.info("Checking if background command returned")
> if session2.get_command_status(check_s4_cmd):
> raise error.TestFail("Command %s failed. S4 failed" % test_s4_cmd)
>
> logging.info("VM resumed after S4")
> session2.sendline(params.get("kill_test_s4_cmd"))
> session.close()
> session2.close()
>
> }}}
>
> So I am waiting your comments before I can apply this patch.
Thanks very much for imporving this test. :)
>
> Thanks!
>
> Lucas
--
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
