KVM test: Make the profiler could be configurated
PATCH 1/2 KVM test: Make the profiler could be configurated
The patch let the profilers could be specified through configuration file. kvm_stat was kept as the default profiler.
--- client/tests/kvm/kvm_utils.py | 23 ++++++++++------------- client/tests/kvm/tests_base.cfg.sample | 2 +- 2 files changed, 11 insertions(+), 14 deletions(-)
client/tests/kvm/kvm_utils.py +10 -13
@@ -866,24 +866,21 @@ def run_tests(test_list, job): if dependencies_satisfied: test_iterations = int(dict.get("iterations", 1)) test_tag = dict.get("shortname") - # Setting up kvm_stat profiling during test execution. - # We don't need kvm_stat profiling on the build tests. - if dict.get("run_kvm_stat") == "yes": - profile = True - else: - # None because it's the default value on the base_test class - # and the value None is specifically checked there. - profile = None + # Setting up profilers during test execution. + profilers = dict.get("profilers") + if profilers is not None: + for profiler in profilers.split(): + job.profilers.add(profiler) - if profile: - job.profilers.add('kvm_stat') # We need only one execution, profiled, hence we're passing # the profile_only parameter to job.run_test(). current_status = job.run_test("kvm", params=dict, tag=test_tag, iterations=test_iterations, - profile_only=profile) - if profile: - job.profilers.delete('kvm_stat') + profile_only= profilers is not None) + + if profilers is not None: + for profiler in profilers.split(): + job.profilers.delete(profiler) if not current_status: failed = True
client/tests/kvm/tests_base.cfg.sample +1 -1
@@ -41,7 +41,7 @@ nic_script = scripts/qemu-ifup address_index = 0 # Misc -run_kvm_stat = yes +profilers = "kvm_stat " # Tests
PATCH 2/2 KVM test: Add the hwclock test into guest test
Hwclock is useful to do the basic testing of emulated RTC.
--- client/tests/kvm/autotest_control/hwclock.control | 12 ++++++++++++ client/tests/kvm/tests_base.cfg.sample | 3 +++ 2 files changed, 15 insertions(+), 0 deletions(-) create mode 100644 client/tests/kvm/autotest_control/hwclock.control
client/tests/kvm/autotest_control/hwclock.control new file +12 -0
@@ -0,0 +1,12 @@ +AUTHOR = "Martin J. Bligh <[email protected]>" +NAME = "Hwclock" +TIME = "SHORT" +TEST_CATEGORY = "Functional" +TEST_CLASS = "General" +TEST_TYPE = "client" + +DOC = """ +This test checks that we can set and read the hwclock successfully +""" + +job.run_test('hwclock')
client/tests/kvm/tests_base.cfg.sample +3 -0
@@ -145,6 +145,9 @@ variants: - scrashme: test_name = scrashme test_control_file = scrashme.control + - hwclock: + test_name = hwclock + test_control_file = hwclock.control - linux_s3: install setup unattended_install type = linux_s3
Quoted 2 lines · ----- "Jason Wang
The patch let the profilers could be specified through configuration file.
kvm_statwas kept as the default profiler.
Looks good. Some minor style comments:
Quoted 17 lines
Signed-off-by Jason Wang <[email protected]>--- client/tests/kvm/kvm_utils.py | 23 ++++++++++------------- client/tests/kvm/tests_base.cfg.sample | 2 +- 2 files changed, 11 insertions(+), 14 deletions(-)client/tests/kvm/kvm_utils.py +0
-1@@ -866,24 +866,21 @@ def run_tests(test_list, job): if dependencies_satisfied: test_iterations = int(dict.get("iterations", 1)) test_tag = dict.get("shortname") - # Setting up kvm_stat profiling during test execution.
client/tests/kvm/kvm_utils.py def run_tests(test_list, job): · line 871
Quoted 10 lines
client/tests/kvm/kvm_utils.py +3
-7- # We don't need kvm_stat profiling on the build tests. - if dict.get("run_kvm_stat") == "yes": - profile = True - else: - # None because it's the default value on the base_test class - # and the value None is specifically checked there. - profile = None + # Setting up profilers during test execution. + profilers = dict.get("profilers") + if profilers is not None:
I think it's nicer and shorter to say "if profilers" instead of "if profilers is not None".
Better yet, use 'profilers = dict.get("profilers", "")' so that if profilers isn't defined, or if the user said 'profilers = ""', you can still call profilers.split(), i.e.:
profilers = dict.get("profilers", "")
for profiler in profilers.split():
job.profilers.add(profiler)
and then you don't need the 'if'.
This is also relevant to the job.profilers.delete() code below.
client/tests/kvm/kvm_utils.py def run_tests(test_list, job): · line 879
Quoted 13 lines
client/tests/kvm/kvm_utils.py +3
-5+ for profiler in profilers.split(): + job.profilers.add(profiler) - if profile: - job.profilers.add('kvm_stat') # We need only one execution, profiled, hence we're passing # the profile_only parameter to job.run_test(). current_status = job.run_test("kvm", params=dict, tag=test_tag, iterations=test_iterations, - profile_only=profile) - if profile: - job.profilers.delete('kvm_stat') + profile_only= profilers is not None)
AFAIK, profile_only needs to be either True or None (Lucas, please correct me if I'm wrong).
In that case, it would be appropriate to use
profile_only=bool(profilers) or None
so that if profilers is e.g. "kvm_stat", profile_only will be True, and if profilers is "", profile_only will be None.
Quoted 13 lines
client/tests/kvm/kvm_utils.py +4
-0+ + if profilers is not None: + for profiler in profilers.split(): + job.profilers.delete(profiler) if not current_status: failed = Trueclient/tests/kvm/tests_base.cfg.sample +0
-0
client/tests/kvm/tests_base.cfg.sample nic_script = scripts/qemu-ifup · line 44
Quoted 5 lines
client/tests/kvm/tests_base.cfg.sample +1
-1@@ -41,7 +41,7 @@ nic_script = scripts/qemu-ifup address_index = 0 # Misc -run_kvm_stat = yes +profilers = "kvm_stat "
We don't need the quotes here now. We'll need them later if we add more profilers. So it's OK to use
profilers = kvm_stat
and then later if we need another profiler:
profilers += " some_other_profiler"
Quoted 8 lines
client/tests/kvm/tests_base.cfg.sample +0
-1# Tests --To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 10 lines · Michael Goldish
Quoted 3 lines · ----- "Jason Wang
The patch let the profilers could be specified through configuration file.
kvm_statwas kept as the default profiler.Looks good. Some minor style comments:
Thanks for the comment, would re-send the patch.
Quoted 63 lines · Michael Goldish
Quoted 2 lines · ----- "Jason Wang
The patch let the profilers could be specified through configuration file.
kvm_statwas kept as the default profiler.Looks good. Some minor style comments:
Quoted 19 lines
Signed-off-by Jason Wang <[email protected]>---
client/tests/kvm/kvm_utils.py | 23
++++++++++-------------
client/tests/kvm/tests_base.cfg.sample | 2 +-2 files changed, 11 insertions(+), 14 deletions(-)client/tests/kvm/kvm_utils.py +0
-0@@ -866,24 +866,21 @@ def run_tests(test_list, job):if
dependencies_satisfied:
test_iterations =int(dict.get("iterations", 1))test_tag=dict.get("shortname")
- # Setting upkvm_statprofiling during test execution.
- # We don't needkvm_statprofiling on the build tests.
- ifdict.get("run_kvm_stat")== "yes":Quoted 8 lines
- profile = True
- else:
- # None because it's the default value on thebase_testclass
- # and the value None is specifically checked there.
- profile = None
+ # Setting up profilers during test execution.
+ profilers =dict.get("profilers")
+ if profilers is not None:I think it's nicer and shorter to say "if profilers" instead of "if profilers is not None".
Better yet, use 'profilers =dict.get("profilers", "")' so that if profilers isn't defined, or if the user said 'profilers = ""', you can still callprofilers.split(), i.e.:profilers =
dict.get("profilers", "")
for profiler inprofilers.split():
job.profilers.add(profiler)and then you don't need the 'if'.
This is also relevant to thejob.profilers.delete()code below.Quoted 6 lines
+ for profiler in
profilers.split():
+ job.profilers.add(profiler)- if profile:
- job.profilers.add('kvm_stat')
# We need only one execution, profiled, hence we're passingQuoted 7 lines
# the profile_only parameter to job.run_test(). current_status = job.run_test("kvm", params=dict, tag=test_tag, iterations=test_iterations, - profile_only=profile) - if profile: - job.profilers.delete('kvm_stat') + profile_only= profilers is not None)
Quoted 2 lines
AFAIK,
profile_onlyneeds to be either True or None (Lucas, please correct me if I'm wrong).
You're right, Michael, the test class code checks specifically for None. I have just reviewed the 2nd version of the patch and it looks good. Applied: