Change in vdsm[master]: vdsm: introduce cpuinfo module
danken at redhat.com
danken at redhat.com
Tue Nov 17 12:02:08 UTC 2015
Dan Kenigsberg has posted comments on this change.
Change subject: vdsm: introduce cpuinfo module
......................................................................
Patch Set 13: Code-Review-1
(6 comments)
https://gerrit.ovirt.org/#/c/46912/13/lib/vdsm/cpuarch.py
File lib/vdsm/cpuarch.py:
Line 33: _CPU_ARCH_TO_ARCHITECTURE = {
Line 34: 'x86_64': Architecture.X86_64,
Line 35: 'ppc64': Architecture.PPC64,
Line 36: 'ppc64le': Architecture.PPC64LE
Line 37: }
this confuses me, as the dictionary maps x->x for all keys.
This seems to be used only as a set of known archs. Could you limit it to its single use case inside current()?
Line 38:
Line 39: # CPU_MAP is the actual libvirt cpu_map.xml; the values x86 and ppc64 are used
Line 40: # instead of x86_64 or ppc64le.
Line 41: _ARCHITECTURE_TO_CPU_MAP = {
Line 79:
Line 80:
Line 81: def current(target_arch=None):
Line 82: '''
Line 83: Get the CPU architecture.
sorry, I do not understand the subtle differences between current() and target(). Can you rewrite the docstring?
Line 84:
Line 85: Arguments:
Line 86:
Line 87: target_arch Optional. Accepts a string of one of the supported
https://gerrit.ovirt.org/#/c/46912/13/lib/vdsm/cpuinfo.py
File lib/vdsm/cpuinfo.py:
Line 43: _CPUS_LOCK
I don't think we need the lock here. Instead, have this function RETURN _CPUS and trust memoized() to do its thing properly.
Line 67: raises UnsupportedArchitecture exception or KeyError if cpuinfo format
Line 68: is invalid.
Line 69:
Line 70: '''
Line 71: if not _CPUS:
it should be simpler to do
_CPUS = parse()
here.
Line 72: parse()
Line 73:
Line 74: if cpuarch.current() == cpuarch.Architecture.X86_64:
Line 75: return _CPUS[0]['flags'].split()
https://gerrit.ovirt.org/#/c/46912/13/lib/vdsm/utils.py
File lib/vdsm/utils.py:
Line 783:
this has been dropped by this patch. it might be harmless, but please take note.
https://gerrit.ovirt.org/#/c/46912/13/tests/nettestlib.py
File tests/nettestlib.py:
Line 155: _IFF_TAP = 0x0002
Line 156: _IFF_NO_PI = 0x1000
Line 157: if cpuarch.current() == cpuarch.Architecture.X86_64:
Line 158: _TUNSETIFF = 0x400454ca
Line 159: elif cpuarch.current() == 'ppc64':
why not use the cpuarch.Architecture enum here?
Line 160: _TUNSETIFF = 0x800454ca
Line 161: else:
Line 162: raise SkipTest("Unsupported Architecture %s" % cpuarch.current())
Line 163:
--
To view, visit https://gerrit.ovirt.org/46912
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa702b05f3825ebdcfed16d86d39a8c38fcf224c
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <mbetak at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list