[PATCH 1/2] KVM test: Make the profiler could be configurated

[PATCH 1/2] KVM test: Make the profiler could be configurated

[Jason Wang]

The patch let the profilers could be specified through configuration
file. kvm_stat was kept as the default profiler.

Signed-off-by: Jason Wang <jasowang@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

[PATCH 2/2] KVM test: Add the hwclock test into guest test

[Jason Wang]

Hwclock is useful to do the basic testing of emulated RTC.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 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

diff --git a/client/tests/kvm/autotest_control/hwclock.control b/client/tests/kvm/autotest_control/hwclock.control
new file mode 100644
index 0000000..bf3e9d3
--- /dev/null
+++ b/client/tests/kvm/autotest_control/hwclock.control
@@ -0,0 +1,12 @@
+AUTHOR = "Martin J. Bligh <mbligh@mbligh.org>"
+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')
diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
index cc10713..00b80c3 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -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

Re: [PATCH 1/2] KVM test: Make the profiler could be configurated

[Michael Goldish]

----- "Jason Wang" <jasowang@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 <jasowang@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 majordomo@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]

Michael Goldish wrote:
> ----- "Jason Wang" <jasowang@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.

Re: [PATCH 1/2] KVM test: Make the profiler could be configurated

[Lucas Meneghel Rodrigues]

On Tue, Mar 30, 2010 at 2:02 PM, Michael Goldish <mgoldish@redhat.com> wrote:
>
> ----- "Jason Wang" <jasowang@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 <jasowang@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