On 07/27/2010 08:56 AM, Lucas Meneghel Rodrigues wrote: > On Thu, 2010-07-08 at 13:14 +0800, Yiqiao Pu wrote: >> Use ntp service by getting offset to calcaulate time drift instead of date. >> cmdlines are : >> * ntpdate -q ntp_server(Linux) >> * w32tm /stripchart /computer:ntp_server /samples:1(Windows) >> Add the function ntp_offset(session, time_command, ntp_serve) to >> kvm_test_utils > Hi Yinqiao, I've reviewed your patch, sorry it took so long to get it > done. I have one high level doubt about it: > > * Could you summarize what would be the advantages of getting the time > drift through ntp instead of simply picking up the date for guest and > host? I couldn't see any particular advantage [1], and we impose an > additional restriction on the test, which is to use a utility that will > query a time server to get the job done. > > I am not quite convinced that we should do that. Please help me to > understand the possible benefits. > > [1] Maybe the idea is to get a sort of 'absolute reference' when it > comes to the time drift measurements, but in fact we are more concerned > with the perceived time drift when you compare the host and guest clock, > so I don't see a significant gain here. > > Also, I have some notes on the code, see below: > Hi Lucas, Use ntp service for the timedrift tests aims for reducing the time drift cause by our test case. When using date to get the time there will be a login and command send time which are including in to the guest time. It is not so obviously in the good condition, but when the machine is busy it may cause a mistake of our test case. So we try to limited the time drift caused by test cases as much as we can. I also found that the first version of my patch can not express the advantage of using ntp service and do not consider the lack of query a time server. Now I want to using the same way to produce data as the get_time function, keep the timedrift testcase as original version. I just add the ntp service as an option way to get time. You can find it in my new patch. So if there is network, we can get a more precise result with ntp service, and if our tests are running in a condition without network, it can using the original way to get time by date. We just need to switch them by configuration files.
About "w32tm" in Windows, I test it in Win2003(32,64), Win2008(32,64), Win7(32,64), WinXP(32) and Win2008r2(64). All these systems has this command as default. Best Regard Joy Pu >> changes from v1: >> Change the ntp server in tests_base.cfg.sample to a globe one, and it can >> change >> by user who is execute this test. >> >> change from v2: >> Switch time drift calcualate from using date to ntp service lead to a change >> when calculate the percentage of time drift in with_load case. Should add the >> sleep time to the delta. >> >> Signed-off-by: Yiqiao Pu<[email protected]> >> --- >> client/tests/kvm/kvm_test_utils.py | 37 >> ++++++++++++++++++++ >> client/tests/kvm/tests/timedrift.py | 27 ++++++-------- >> client/tests/kvm/tests/timedrift_with_migration.py | 17 +++++---- >> client/tests/kvm/tests/timedrift_with_reboot.py | 17 +++++---- >> client/tests/kvm/tests/timedrift_with_stop.py | 17 +++++---- >> client/tests/kvm/tests_base.cfg.sample | 6 ++-- >> 6 files changed, 78 insertions(+), 43 deletions(-) >> >> diff --git a/client/tests/kvm/kvm_test_utils.py >> b/client/tests/kvm/kvm_test_utils.py >> index 53c11ae..16766ee 100644 >> --- a/client/tests/kvm/kvm_test_utils.py >> +++ b/client/tests/kvm/kvm_test_utils.py >> @@ -278,6 +278,43 @@ def get_time(session, time_command, time_filter_re, >> time_format): >> guest_time = time.mktime(time.strptime(s, time_format)) >> return (host_time, guest_time) >> >> +def ntp_offset(session, time_command, ntp_server): >> + """ >> + Return the offset of host time and guest time to the ntp server. If the >> + offset of guest time cannot be fetched a TestError exception is raised. >> + >> + Note that the shell session should be ready to receive commands >> + (i.e. should "display" a command prompt and should be done with all >> + previous commands). >> + >> + @param session: A shell session. >> + @param time_command: Command to issue to get the current guest time. >> + @param ntp_server: NTP server name to calculate the offset >> + @return: A tuple containing the host time and guest time. >> + """ >> + def offset_process(offset_string, cmd): >> + ret = None >> + if re.match('w32tm', cmd): >> + ret = re.findall("o:(.*)s", offset_string)[0] >> + elif re.match('ntpdate', cmd): >> + ret = re.findall("offset (.*),", offset_string)[0] >> + else: >> + raise logging.error("unknow command: %s" % cmd) > ^ Here we should raise an error.TestError, as the docstring itself > points out. Also, please correct the little typo "unknow" > >> + return float(ret) >> + >> + s, o = commands.getstatusoutput("ntpdate -q %s" % ntp_server) >> + host_time_offset = offset_process(o, "ntpdate -q %s" % ntp_server) >> + if s!= 0 or host_time_offset is None: >> + raise error.TestError("Could not get host time: %s", o) >> + s , o = session.get_command_status_output("%s%s" % \ > ^ Here there's no need to end the line with a backslash > >> + (time_command, ntp_server)) >> + guest_time_offset = offset_process(o, time_command) >> + if s != 0 or guest_time_offset is None: >> + raise error.TestError("Could not get guest time: %s" % o) >> + >> + logging.debug("host time offset:%s\n guest time offset:%s" %\ > ^ Idem, no need of backslash. Python has implicit line continuation when > you use parenthesis > >> + (host_time_offset, guest_time_offset)) >> + return (host_time_offset, guest_time_offset) >> >> def get_memory_info(lvms): >> """ >> diff --git a/client/tests/kvm/tests/timedrift.py >> b/client/tests/kvm/tests/timedrift.py >> index a6d3076..dd0f7bd 100644 >> --- a/client/tests/kvm/tests/timedrift.py >> +++ b/client/tests/kvm/tests/timedrift.py >> @@ -52,6 +52,7 @@ def run_timedrift(test, params, env): >> for tid, mask in prev_masks.items(): >> commands.getoutput("taskset -p %s %s" % (mask, tid)) >> >> + ntp_server = params.get("ntp_server") >> vm = kvm_test_utils.get_living_vm(env, params.get("main_vm")) >> timeout = int(params.get("login_timeout", 360)) >> session = kvm_test_utils.wait_for_login(vm, timeout=timeout) >> @@ -101,10 +102,8 @@ def run_timedrift(test, params, env): >> >> # Get time before load >> # (ht stands for host time, gt stands for guest time) >> - (ht0, gt0) = kvm_test_utils.get_time(session, >> - time_command, >> - time_filter_re, >> - time_format) >> + (ht0, gt0) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> >> # Run some load on the guest >> for load_session in guest_load_sessions: >> @@ -127,14 +126,12 @@ def run_timedrift(test, params, env): >> time.sleep(load_duration) >> >> # Get time delta after load >> - (ht1, gt1) = kvm_test_utils.get_time(session, >> - time_command, >> - time_filter_re, >> - time_format) >> + (ht1, gt1) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> >> # Report results >> - host_delta = ht1 - ht0 >> - guest_delta = gt1 - gt0 >> + host_delta = ht1 - ht0 + load_duration >> + guest_delta = gt1 - gt0 + load_duration > I didn't understand why you had to add load_duration to the delta > calculations, I mean, we get the offset in t0 and in t1, I fail to see > why we should add t1 - t0 to the deltas. > >> drift = 100.0 * (host_delta - guest_delta) / host_delta >> logging.info("Host duration: %.2f" % host_delta) >> logging.info("Guest duration: %.2f" % guest_delta) >> @@ -158,17 +155,15 @@ def run_timedrift(test, params, env): >> time.sleep(rest_duration) >> >> # Get time after rest >> - (ht2, gt2) = kvm_test_utils.get_time(session, >> - time_command, >> - time_filter_re, >> - time_format) >> + (ht2, gt2) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> >> finally: >> session.close() >> >> # Report results >> - host_delta_total = ht2 - ht0 >> - guest_delta_total = gt2 - gt0 >> + host_delta_total = ht2 - ht0 + load_duration + rest_duration >> + guest_delta_total = gt2 - gt0 + load_duration + rest_duration >> drift_total = 100.0 * (host_delta_total - guest_delta_total) / >> host_delta >> logging.info("Total host duration including rest: %.2f" % >> host_delta_total) >> logging.info("Total guest duration including rest: %.2f" % >> guest_delta_total) >> diff --git a/client/tests/kvm/tests/timedrift_with_migration.py >> b/client/tests/kvm/tests/timedrift_with_migration.py >> index e953ed3..125c1a7 100644 >> --- a/client/tests/kvm/tests/timedrift_with_migration.py >> +++ b/client/tests/kvm/tests/timedrift_with_migration.py >> @@ -18,6 +18,7 @@ def run_timedrift_with_migration(test, params, env): >> @param env: Dictionary with the test environment. >> """ >> vm = kvm_test_utils.get_living_vm(env, params.get("main_vm")) >> + ntp_server = params.get("ntp_server") >> timeout = int(params.get("login_timeout", 360)) >> session = kvm_test_utils.wait_for_login(vm, timeout=timeout) >> >> @@ -35,14 +36,14 @@ def run_timedrift_with_migration(test, params, env): >> try: >> # Get initial time >> # (ht stands for host time, gt stands for guest time) >> - (ht0, gt0) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, time_format) >> + (ht0, gt0) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> >> # Migrate >> for i in range(migration_iterations): >> # Get time before current iteration >> - (ht0_, gt0_) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, >> time_format) >> + (ht0_, gt0_) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> session.close() >> # Run current iteration >> logging.info("Migrating: iteration %d of %d..." % >> @@ -55,8 +56,8 @@ def run_timedrift_with_migration(test, params, env): >> raise error.TestFail("Could not log in after migration") >> logging.info("Logged in after migration") >> # Get time after current iteration >> - (ht1_, gt1_) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, >> time_format) >> + (ht1_, gt1_) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> # Report iteration results >> host_delta = ht1_ - ht0_ >> guest_delta = gt1_ - gt0_ >> @@ -73,8 +74,8 @@ def run_timedrift_with_migration(test, params, env): >> "%.2f seconds" % (i + 1, drift)) >> >> # Get final time >> - (ht1, gt1) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, time_format) >> + (ht1, gt1) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> >> finally: >> if session: >> diff --git a/client/tests/kvm/tests/timedrift_with_reboot.py >> b/client/tests/kvm/tests/timedrift_with_reboot.py >> index 22dfd45..781de18 100644 >> --- a/client/tests/kvm/tests/timedrift_with_reboot.py >> +++ b/client/tests/kvm/tests/timedrift_with_reboot.py >> @@ -18,6 +18,7 @@ def run_timedrift_with_reboot(test, params, env): >> @param env: Dictionary with the test environment. >> """ >> vm = kvm_test_utils.get_living_vm(env, params.get("main_vm")) >> + ntp_server = params.get("ntp_server") >> timeout = int(params.get("login_timeout", 360)) >> session = kvm_test_utils.wait_for_login(vm, timeout=timeout) >> >> @@ -35,21 +36,21 @@ def run_timedrift_with_reboot(test, params, env): >> try: >> # Get initial time >> # (ht stands for host time, gt stands for guest time) >> - (ht0, gt0) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, time_format) >> + (ht0, gt0) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> >> # Reboot >> for i in range(reboot_iterations): >> # Get time before current iteration >> - (ht0_, gt0_) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, >> time_format) >> + (ht0_, gt0_) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> # Run current iteration >> logging.info("Rebooting: iteration %d of %d..." % >> (i + 1, reboot_iterations)) >> session = kvm_test_utils.reboot(vm, session) >> # Get time after current iteration >> - (ht1_, gt1_) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, >> time_format) >> + (ht1_, gt1_) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> # Report iteration results >> host_delta = ht1_ - ht0_ >> guest_delta = gt1_ - gt0_ >> @@ -66,8 +67,8 @@ def run_timedrift_with_reboot(test, params, env): >> "%.2f seconds" % (i + 1, drift)) >> >> # Get final time >> - (ht1, gt1) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, time_format) >> + (ht1, gt1) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> >> finally: >> if session: >> diff --git a/client/tests/kvm/tests/timedrift_with_stop.py >> b/client/tests/kvm/tests/timedrift_with_stop.py >> index fe98571..08e9f09 100644 >> --- a/client/tests/kvm/tests/timedrift_with_stop.py >> +++ b/client/tests/kvm/tests/timedrift_with_stop.py >> @@ -20,6 +20,7 @@ def run_timedrift_with_stop(test, params, env): >> @param env: Dictionary with the test environment. >> """ >> login_timeout = int(params.get("login_timeout", 360)) >> + ntp_server = params.get("ntp_server") >> vm = kvm_test_utils.get_living_vm(env, params.get("main_vm")) >> session = kvm_test_utils.wait_for_login(vm, timeout=login_timeout) >> >> @@ -38,14 +39,14 @@ def run_timedrift_with_stop(test, params, env): >> try: >> # Get initial time >> # (ht stands for host time, gt stands for guest time) >> - (ht0, gt0) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, time_format) >> + (ht0, gt0) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> >> # Stop the guest >> for i in range(stop_iterations): >> # Get time before current iteration >> - (ht0_, gt0_) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, >> time_format) >> + (ht0_, gt0_) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> # Run current iteration >> logging.info("Stop %s second: iteration %d of %d..." % >> (stop_time, i + 1, stop_iterations)) >> @@ -55,8 +56,8 @@ def run_timedrift_with_stop(test, params, env): >> vm.monitor.cmd("cont") >> >> # Get time after current iteration >> - (ht1_, gt1_) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, >> time_format) >> + (ht1_, gt1_) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> # Report iteration results >> host_delta = ht1_ - ht0_ >> guest_delta = gt1_ - gt0_ >> @@ -73,8 +74,8 @@ def run_timedrift_with_stop(test, params, env): >> "%.2f seconds" % (i + 1, drift)) >> >> # Get final time >> - (ht1, gt1) = kvm_test_utils.get_time(session, time_command, >> - time_filter_re, time_format) >> + (ht1, gt1) = kvm_test_utils.ntp_offset(session, time_command, >> + ntp_server) >> >> finally: >> if session: >> diff --git a/client/tests/kvm/tests_base.cfg.sample >> b/client/tests/kvm/tests_base.cfg.sample >> index 62936d4..a2cc9d7 100644 >> --- a/client/tests/kvm/tests_base.cfg.sample >> +++ b/client/tests/kvm/tests_base.cfg.sample >> @@ -56,7 +56,7 @@ run_tcpdump = yes >> >> # Misc >> profilers = kvm_stat >> - >> +ntp_server = clock.redhat.com >> >> # Tests >> variants: >> @@ -508,7 +508,7 @@ variants: >> cpu_chk_cmd = grep -c processor /proc/cpuinfo >> timedrift: >> extra_params += " -no-kvm-pit-reinjection" >> - time_command = date +'TIME: %a %m/%d/%Y %H:%M:%S.%N' >> + time_command = "ntpdate -q " >> time_filter_re = "(?:TIME: \w\w\w )(.{19})(?:\.\d\d)" >> time_format = "%m/%d/%Y %H:%M:%S" >> guest_load_command = "dd if=/dev/urandom of=/dev/null" >> @@ -1052,7 +1052,7 @@ variants: >> # Timedrift compensation on Windows with hpet does not happen >> disable_hpet = yes >> extra_params += " -rtc-td-hack" >> - time_command = "echo TIME: %date% %time%" >> + time_command = "w32tm /stripchart /samples:1 /computer:" > Can we count on this thing available on all windows versions we care > about? > >> time_filter_re = "(?<=TIME: \w\w\w ).{19}(?=\.\d\d)" >> time_format = "%m/%d/%Y %H:%M:%S" >> # For this to work, the cdrom at d: should contain vlc >> (d:\vlc\vlc.exe) and a video (d:\ED_1024.avi) > _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
