On Tue, 2011-06-28 at 17:53 -0400, Cleber Rosa wrote: > The current version of cpu_vendor() works by calling out an external 'grep' > process. This is rather unecessary. Also, having it in virt_utils make > more sense IMHO. Finally, renaming it to get_cpu_vendor() makes the API > more consistent with other functions such as get_vendor_from_pci_id(). > > Also attached is a unittest for virt_utils, that initially tests only > get_cpu_vendor() behaviour. > > Changes from v1: > * Properly remove cpu_vendor() from virt_installer.py > * Fix typo in name of test_vendor_unknown() method
Looks good to me. Same comments as for the previous patch (unittests need import common and execution permissions). The new API name and the unittests are a much welcome change. Applied: http://autotest.kernel.org/changeset/5467 Thank you! > Signed-off-by: Cleber Rosa <[email protected]> > --- > client/virt/kvm_installer.py | 2 +- > client/virt/virt_installer.py | 8 ------ > client/virt/virt_utils.py | 31 ++++++++++++++++++++++++ > client/virt/virt_utils_unittest.py | 46 > ++++++++++++++++++++++++++++++++++++ > 4 files changed, 78 insertions(+), 9 deletions(-) > create mode 100644 client/virt/virt_utils_unittest.py > > diff --git a/client/virt/kvm_installer.py b/client/virt/kvm_installer.py > index dd4ca1d..5a7ca5b 100644 > --- a/client/virt/kvm_installer.py > +++ b/client/virt/kvm_installer.py > @@ -161,7 +161,7 @@ class BaseInstaller(object): > self.extra_modules = eval(params.get("extra_modules", > default_extra_modules)) > > - self.cpu_vendor = virt_installer.cpu_vendor() > + self.cpu_vendor = virt_utils.get_cpu_vendor() > > self.srcdir = test.srcdir > if not os.path.isdir(self.srcdir): > diff --git a/client/virt/virt_installer.py b/client/virt/virt_installer.py > index 1faae5f..5ee128c 100644 > --- a/client/virt/virt_installer.py > +++ b/client/virt/virt_installer.py > @@ -23,14 +23,6 @@ def check_configure_options(script_path): > return option_list > > > -def cpu_vendor(): > - vendor = "intel" > - if os.system("grep vmx /proc/cpuinfo 1>/dev/null") != 0: > - vendor = "amd" > - logging.debug("Detected CPU vendor as '%s'", vendor) > - return vendor > - > - > def save_build(build_dir, dest_dir): > logging.debug('Saving the result of the build on %s', dest_dir) > base_name = os.path.basename(build_dir) > diff --git a/client/virt/virt_utils.py b/client/virt/virt_utils.py > index ce45558..f959825 100644 > --- a/client/virt/virt_utils.py > +++ b/client/virt/virt_utils.py > @@ -1322,6 +1322,37 @@ def get_vendor_from_pci_id(pci_id): > return re.sub(":", " ", commands.getoutput(cmd)) > > > +def get_cpu_flags(): > + """ > + Returns a list of the CPU flags > + """ > + flags_re = re.compile(r'^flags\s*:(.*)') > + for line in open('/proc/cpuinfo').readlines(): > + match = flags_re.match(line) > + if match: > + return match.groups()[0].split() > + return [] > + > + > +def get_cpu_vendor(cpu_flags=[], verbose=True): > + """ > + Returns the name of the CPU vendor, either intel, amd or unknown > + """ > + if not cpu_flags: > + cpu_flags = get_cpu_flags() > + > + if 'vmx' in cpu_flags: > + vendor = 'intel' > + elif 'svm' in cpu_flags: > + vendor = 'amd' > + else: > + vendor = 'unknown' > + > + if verbose: > + logging.debug("Detected CPU vendor as '%s'", vendor) > + return vendor > + > + > class Thread(threading.Thread): > """ > Run a function in a background thread. > diff --git a/client/virt/virt_utils_unittest.py > b/client/virt/virt_utils_unittest.py > new file mode 100644 > index 0000000..b4f6c45 > --- /dev/null > +++ b/client/virt/virt_utils_unittest.py > @@ -0,0 +1,46 @@ > +#!/usr/bin/python > + > +import unittest > +from autotest_lib.client.virt import virt_utils > + > +class virt_utils_test(unittest.TestCase): > + > + > + def test_cpu_vendor_intel(self): > + flags = ['fpu', 'vme', 'de', 'pse', 'tsc', 'msr', 'pae', 'mce', > + 'cx8', 'apic', 'sep', 'mtrr', 'pge', 'mca', 'cmov', > + 'pat', 'pse36', 'clflush', 'dts', 'acpi', 'mmx', 'fxsr', > + 'sse', 'sse2', 'ss', 'ht', 'tm', 'pbe', 'syscall', 'nx', > + 'lm', 'constant_tsc', 'arch_perfmon', 'pebs', 'bts', > + 'rep_good', 'aperfmperf', 'pni', 'dtes64', 'monitor', > + 'ds_cpl', 'vmx', 'smx', 'est', 'tm2', 'ssse3', 'cx16', > + 'xtpr', 'pdcm', 'sse4_1', 'xsave', 'lahf_lm', 'ida', > + 'tpr_shadow', 'vnmi', 'flexpriority'] > + vendor = virt_utils.get_cpu_vendor(flags, False) > + self.assertEqual(vendor, 'intel') > + > + > + def test_cpu_vendor_amd(self): > + flags = ['fpu', 'vme', 'de', 'pse', 'tsc', 'msr', 'pae', 'mce', > + 'cx8', 'apic', 'mtrr', 'pge', 'mca', 'cmov', 'pat', > + 'pse36', 'clflush', 'mmx', 'fxsr', 'sse', 'sse2', > + 'ht', 'syscall', 'nx', 'mmxext', 'fxsr_opt', 'pdpe1gb', > + 'rdtscp', 'lm', '3dnowext', '3dnow', 'constant_tsc', > + 'rep_good', 'nonstop_tsc', 'extd_apicid', 'aperfmperf', > + 'pni', 'monitor', 'cx16', 'popcnt', 'lahf_lm', > + 'cmp_legacy', 'svm', 'extapic', 'cr8_legacy', 'abm', > + 'sse4a', 'misalignsse', '3dnowprefetch', 'osvw', 'ibs', > + 'skinit', 'wdt', 'cpb', 'npt', 'lbrv', 'svm_lock', > + 'nrip_save'] > + vendor = virt_utils.get_cpu_vendor(flags, False) > + self.assertEqual(vendor, 'amd') > + > + > + def test_vendor_unknown(self): > + flags = ['non', 'sense', 'flags'] > + vendor = virt_utils.get_cpu_vendor(flags, False) > + self.assertEqual(vendor, 'unknown') > + > + > +if __name__ == '__main__': > + unittest.main() _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
