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: > 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
