On 03/13/2014 08:51 AM, 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> > --- > tests/capabilities.py | 15 ++++++++++----- > virtManager/details.py | 3 ++- > virtinst/capabilities.py | 47 ++++++++++++++++++++++++++++++++++++++--------- > virtinst/support.py | 4 +++- > 4 files changed, 53 insertions(+), 16 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..b137ede 100644 > --- a/virtinst/capabilities.py > +++ b/virtinst/capabilities.py > @@ -77,11 +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, conn): > + self._conn = conn > + self._cpus = None > + I'd recommend not storing 'conn' here, since it might cause some GC issue: VirtinstConnection holds a reference to capabilities which may hold a reference to connection. Just change get_cpus to take the conn explicitly. > + def get_cpus(self, arch): > + if self._cpus is not None: > + return self._cpus > + > + if self._conn and \ > + self._conn.check_support(self._conn.SUPPORT_CONN_CPU_MODEL_NAMES): > + self._cpus = [CPUValuesModel(i) for i in > + self._conn.libvirtconn.getCPUModelNames(arch, 0)] > + return self._cpus > + > + return None Style nit: please drop the \ escape and just wrap the whole thing in parenths. > +class CPUMapFileValues(object): > + """ > + Fallback method to lists cpu models, parsed directly from libvirt's local > + cpu_map.xml > + """ > + def __init__(self, conn=None, cpu_filename=None): > self.archmap = {} > + ignore = conn > if not cpu_filename: > cpu_filename = "/usr/share/libvirt/cpu_map.xml" Looks like passing in cpu_filename in __init__ isn't even used, so you can drop that. Then just have CPUMapFileValues subclass CPUValues so pylint will give us warnings if the API diverges. - Cole _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
