On Fri, 2012-06-15 at 06:56 +0530, Pradeep Kumar wrote:
> This patch to test kvm event tracing using "perf kvm". 
> 
> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=817284
> https://lkml.org/lkml/2012/6/1/176
> http://www.linux-kvm.org/page/Perf_events
> 

Hi Pradeep, thanks for your work! I have some comments, see below:

>  Signed-off-by: pradeep K Surisetty <psuri...@linux.vnet.ibm.com>
> 
>       new file:   client/tests/kvm/tests/perf_kvm.py
>       modified:   client/virt/subtests.cfg.sample
> ---
>  client/tests/kvm/tests/perf_kvm.py |   42 
> ++++++++++++++++++++++++++++++++++++
>  client/virt/subtests.cfg.sample    |    7 ++++++
>  2 files changed, 49 insertions(+)
>  create mode 100644 client/tests/kvm/tests/perf_kvm.py
> 
> diff --git a/client/tests/kvm/tests/perf_kvm.py 
> b/client/tests/kvm/tests/perf_kvm.py
> new file mode 100644
> index 0000000..f4f8e5f
> --- /dev/null
> +++ b/client/tests/kvm/tests/perf_kvm.py
> @@ -0,0 +1,42 @@
> +import os, commands, glob
> +from autotest.client.shared import error
> +from autotest.client.virt import virt_test_utils
> +
> +
> +def run_perf_kvm(test, params, env):
> +    """
> +    run perf tool to get kvm events info
> +
> +    @param test: kvm test object
> +    @param params: Dictionary with the test parameters
> +    @param env: Dictionary with test environment.
> +    """
> +    vm = env.get_vm(params["main_vm"])
> +    vm.verify_alive()
> +
> +    test_timeout = int(params.get("test_timeout", 240))
> +    login_timeout = int(params.get("login_timeout", 360))
> +    transfer_timeout = int(params.get("transfer_timeout", 240))
> +    perf_record_timeout = int(params.get("perf_record_timeout", 240))
> +    vm_kallsyms_path = "/tmp/guest_kallsyms"
> +    vm_modules_path = "/tmp/guest_modules"
> +
> +    # Prepare test environment in guest
> +    session = vm.wait_for_login(timeout=login_timeout)
> +
> +    session.cmd("cat /proc/kallsyms > %s" % vm_kallsyms_path)
> +    session.cmd("cat /proc/modules > %s" % vm_modules_path)
> +
> +    vm.copy_files_from("/tmp/guest_kallsyms", "/tmp", 
> timeout=transfer_timeout)
> +    vm.copy_files_from("/tmp/guest_modules", "/tmp", 
> timeout=transfer_timeout)
> +
> +    perf_record_cmd = "perf kvm --host --guest --guestkallsyms=%s" % 
> vm_kallsyms_path
> +    perf_record_cmd += " --guestmodules=%s record -a -o /tmp/perf.data sleep 
> %s " % (vm_modules_path, perf_record_timeout)
> +    perf_report_cmd = "perf kvm --host --guest --guestkallsyms=%s" % 
> vm_kallsyms_path
> +    perf_report_cmd += " --guestmodules=%s report -i /tmp/perf.data --force 
> " % vm_modules_path
> +    
> +    os.system(perf_record_cmd)
> +    os.system(perf_report_cmd)    

So, should we just execute the perf command and not check for its exit
code? In such case, this test can never fail (it can only error, in case
the previous vm operations fail due to some bug). Is that the intended
behavior?

If that's not the intended behavior, that is, we want to fail the test
if the perf record/report commands fail, changing the os.system call to
utils.system() would fix the problem. When calling utils.system() with
ignore_status=False (the default), this function will throw a CmdError
exception in case the return code is !=0.

The rest looks reasonable to me. I was wondering here if perf kvm could
turn into a true autotest profiler, but the fact we need this
guest_modules and guest_kallsyms files might make things hard (what if
it's a windows guest)...

> +    session.close()
> +
> diff --git a/client/virt/subtests.cfg.sample b/client/virt/subtests.cfg.sample
> index 46ebe6b..4f0f8ab 100644
> --- a/client/virt/subtests.cfg.sample
> +++ b/client/virt/subtests.cfg.sample
> @@ -895,6 +895,13 @@ variants:
>          type = smbios_table
>          start_vm = no
>  
> +    - perf_kvm: install setup image_copy unattended_install.cdrom
> +     type = perf_kvm
> +     kallsyms_cmd = "cat /proc/kallsyms > /tmp/guest_kallsyms"
> +     modules_cmd = "cat /proc/modules > /tmp/geust_modules"
> +     transfer_timeout = 100
> +     perf_record_timeout =  100
> +
>      - softlockup: install setup unattended_install.cdrom
>          only Linux
>          type = softlockup


_______________________________________________
Autotest mailing list
Autotest@test.kernel.org
http://test.kernel.org/cgi-bin/mailman/listinfo/autotest

Reply via email to