On Mon, Jul 26, 2010 at 09:56:46PM -0300, 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.
Current method to get time is using get_command_status(), the network delay would effect test result. But for ntp, it would consider the network delay when compute time offset, so it's better. > 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 _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
