On Wed, Jun 20, 2012 at 12:39 AM, Pradeep Kumar Surisetty <[email protected]> wrote: > * Lucas Meneghel Rodrigues <[email protected]> [2012-06-18 11:47:16]: > >> 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 <[email protected]> >> > >> > 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. > > My intention was to just to test "perf kvm" tool, since its very buggy > these days. Once its stable enough we can make perf kvm as true autotest > profiler.
Fair enough, but my original comment still applies - when you execute the perf command with os.system without checking exit code, even if it fails, the test won't fail. I suggest re-sending replacing os.system calls with utils.system calls. > Between for wndows guest, its not developed yet. > > --Pradeep > >> >> 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 > [email protected] > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest -- Lucas _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
