On Wed, Oct 14, 2009 at 06:58:01AM -0300, Lucas Meneghel Rodrigues wrote:
> On Tue, Oct 13, 2009 at 11:54 PM, Yolkfull Chow <yzhou@xxxxxxxxxx> wrote:
> > 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.
>
> Ok, sounds good to me!
>
> >> * 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. ;-)
>
> Yes, I was just plain mistaken, somewhat my brain ignored the
> check_s4_cmd... it looks very good to me, sorry :)
That's no problem.
And I saw that you had sent the new patch.
Thanks very much for help improving this test. :)
Best Regards,
>
> >>
> >> 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?
>
> Sounds like a plan! Commited as:
>
> http://autotest.kernel.org/changeset/3843
--
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
