On 03/13/2014 01:15 PM, Giuseppe Scrivano wrote: > Read the list of CPU models trough getCPUModelNames instead of > accessing directly the file cpu_map.xml. > > Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1060316 > > Signed-off-by: Giuseppe Scrivano <gscrivan@xxxxxxxxxx> > --- > thanks for the comments. > > What about this version? > > tests/capabilities.py | 15 ++++++++++----- > virtManager/details.py | 3 ++- > virtinst/capabilities.py | 50 +++++++++++++++++++++++++++++++++++++----------- > virtinst/support.py | 4 +++- > 4 files changed, 54 insertions(+), 18 deletions(-) > > diff --git a/tests/capabilities.py b/tests/capabilities.py > index c062e4c..db69c6f 100644 > --- a/tests/capabilities.py > +++ b/tests/capabilities.py > @@ -18,6 +18,7 @@ > import os > import unittest > > +from tests import utils > from virtinst import CapabilitiesParser as capabilities > > > @@ -221,13 +222,12 @@ class TestCapabilities(unittest.TestCase): > > def testCPUMap(self): > caps = self._buildCaps("libvirt-0.7.6-qemu-caps.xml") > - cpu_64 = caps.get_cpu_values("x86_64") > - cpu_32 = caps.get_cpu_values("i486") > - cpu_random = caps.get_cpu_values("mips") > + cpu_64 = caps.get_cpu_values(None, "x86_64") > + cpu_32 = caps.get_cpu_values(None, "i486") > + cpu_random = caps.get_cpu_values(None, "mips") > > def test_cpu_map(cpumap, cpus): > - cpunames = sorted([c.model for c in cpumap.cpus], > - key=str.lower) > + cpunames = sorted([c.model for c in cpumap], key=str.lower) > > for c in cpus: > self.assertTrue(c in cpunames) > @@ -243,5 +243,10 @@ class TestCapabilities(unittest.TestCase): > test_cpu_map(cpu_64, x86_cpunames) > test_cpu_map(cpu_random, []) > > + conn = utils.open_testdriver() > + cpu_64 = caps.get_cpu_values(conn, "x86_64") > + self.assertTrue(len(cpu_64) > 0) > + > + > if __name__ == "__main__": > unittest.main() > diff --git a/virtManager/details.py b/virtManager/details.py > index 56a0d60..14c77fe 100644 > --- a/virtManager/details.py > +++ b/virtManager/details.py > @@ -975,7 +975,8 @@ class vmmDetails(vmmGObjectUI): > no_default = not self.is_customize_dialog > > try: > - cpu_names = caps.get_cpu_values(self.vm.get_arch()).cpus > + cpu_names = caps.get_cpu_values(self.conn.get_backend(), > + self.vm.get_arch()) > except: > cpu_names = [] > logging.exception("Error populating CPU model list") > diff --git a/virtinst/capabilities.py b/virtinst/capabilities.py > index 2ab39cd..0bedd65 100644 > --- a/virtinst/capabilities.py > +++ b/virtinst/capabilities.py > @@ -77,13 +77,33 @@ class CPUValuesArch(object): > > class CPUValues(object): > """ > - Lists valid values for domain <cpu> parameters, parsed from libvirt's > - local cpu_map.xml > + Lists valid values for cpu models obtained trough libvirt's getCPUModelNames > """ > - def __init__(self, cpu_filename=None): > + def __init__(self): > + self._cpus = None > + > + def get_cpus(self, arch, conn): > + if self._cpus is not None: > + return self._cpus > + > + if (conn and > + conn.check_support(conn.SUPPORT_CONN_CPU_MODEL_NAMES)): > + self._cpus = [CPUValuesModel(i) for i in > + conn.libvirtconn.getCPUModelNames(arch, 0)] > + return self._cpus > + > + return None I think 'return None' should be 'return []', since it might affect virt-manager's use of get_cpu_values > + > + > +class CPUMapFileValues(CPUValues): > + """ > + Fallback method to lists cpu models, parsed directly from libvirt's local > + cpu_map.xml > + """ > + def __init__(self): > + CPUValues.__init__(self) > self.archmap = {} > - if not cpu_filename: > - cpu_filename = "/usr/share/libvirt/cpu_map.xml" > + cpu_filename = "/usr/share/libvirt/cpu_map.xml" > xml = file(cpu_filename).read() > > util.parse_node_helper(xml, "cpus", > @@ -99,7 +119,8 @@ class CPUValues(object): > > child = child.next > > - def get_arch(self, arch): > + def get_cpus(self, arch, conn): > + ignore = conn > if not arch: > return None > if re.match(r'i[4-9]86', arch): > @@ -112,7 +133,7 @@ class CPUValues(object): > cpumap = CPUValuesArch(arch) > self.archmap[arch] = cpumap > > - return cpumap > + return cpumap.cpus > > > class Features(object): > @@ -595,12 +616,19 @@ class Capabilities(object): > self.guests.append(Guest(child)) > child = child.next > > - def get_cpu_values(self, arch): > - if not self._cpu_values: > - self._cpu_values = CPUValues() > + def get_cpu_values(self, conn, arch): > + if self._cpu_values: > + return self._cpu_values.get_cpus(arch, conn) > > - return self._cpu_values.get_arch(arch) > + # Iterate over the available methods until a set of CPU models is found > + for mode in (CPUValues, CPUMapFileValues): > + cpu_values = mode() > + cpus = cpu_values.get_cpus(arch, conn) > + if cpus: > + self._cpu_values = cpu_values > + return cpus > > + return None > Same here. ACK with that fix, feel free to just push it without reposting. - Cole _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
