Re: [PATCH 1/2] KVM test: Make the profiler could be configurated
Michael Goldish wrote: - Jason Wang jasow...@redhat.com wrote: The patch let the profilers could be specified through configuration file. kvm_stat was kept as the default profiler. Looks good. Some minor style comments: Thanks for the comment, would re-send the patch. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Autotest] [PATCH 1/2] KVM test: Make the profiler could be configurated
On Tue, Mar 30, 2010 at 2:02 PM, Michael Goldish mgold...@redhat.com wrote:
- Jason Wang jasow...@redhat.com wrote:
The patch let the profilers could be specified through configuration
file. kvm_stat was kept as the default profiler.
Looks good. Some minor style comments:
Signed-off-by: Jason Wang jasow...@redhat.com
---
client/tests/kvm/kvm_utils.py | 23
++-
client/tests/kvm/tests_base.cfg.sample | 2 +-
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/client/tests/kvm/kvm_utils.py
b/client/tests/kvm/kvm_utils.py
index 8531c79..a73d5d4 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -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:
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.
+ 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).
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:
http://autotest.kernel.org/changeset/4365
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[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.
Signed-off-by: Jason Wang jasow...@redhat.com
---
client/tests/kvm/kvm_utils.py | 23 ++-
client/tests/kvm/tests_base.cfg.sample |2 +-
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/client/tests/kvm/kvm_utils.py b/client/tests/kvm/kvm_utils.py
index 8531c79..a73d5d4 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -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
diff --git a/client/tests/kvm/tests_base.cfg.sample
b/client/tests/kvm/tests_base.cfg.sample
index d162cf8..cc10713 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -41,7 +41,7 @@ nic_script = scripts/qemu-ifup
address_index = 0
# Misc
-run_kvm_stat = yes
+profilers = kvm_stat
# Tests
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] KVM test: Make the profiler could be configurated
- Jason Wang jasow...@redhat.com wrote:
The patch let the profilers could be specified through configuration
file. kvm_stat was kept as the default profiler.
Looks good. Some minor style comments:
Signed-off-by: Jason Wang jasow...@redhat.com
---
client/tests/kvm/kvm_utils.py | 23
++-
client/tests/kvm/tests_base.cfg.sample |2 +-
2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/client/tests/kvm/kvm_utils.py
b/client/tests/kvm/kvm_utils.py
index 8531c79..a73d5d4 100644
--- a/client/tests/kvm/kvm_utils.py
+++ b/client/tests/kvm/kvm_utils.py
@@ -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:
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.
+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.
+
+if profilers is not None:
+for profiler in profilers.split():
+job.profilers.delete(profiler)
if not current_status:
failed = True
diff --git a/client/tests/kvm/tests_base.cfg.sample
b/client/tests/kvm/tests_base.cfg.sample
index d162cf8..cc10713 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -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
# Tests
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
