Change in vdsm[master]: vdsm: introduce cpuinfo module
nsoffer at redhat.com
nsoffer at redhat.com
Sun Jan 17 01:23:44 UTC 2016
Nir Soffer has posted comments on this change.
Change subject: vdsm: introduce cpuinfo module
......................................................................
Patch Set 22:
(11 comments)
Much nicer then the previous code, but can be even simpler.
https://gerrit.ovirt.org/#/c/46912/22/lib/vdsm/cpuinfo.py
File lib/vdsm/cpuinfo.py:
Line 1: #
Line 2: # Copyright 2015 Red Hat, Inc.
Its 2016 now.
Line 3: #
Line 4: # This program is free software; you can redistribute it and/or modify
Line 5: # it under the terms of the GNU General Public License as published by
Line 6: # the Free Software Foundation; either version 2 of the License, or
Line 24:
Line 25: from . import cpuarch
Line 26:
Line 27:
Line 28: _SOURCE = '/proc/cpuinfo'
This looks more like _PATH
Line 29: _CPU_TEMPLATE = namedtuple('_CPU_TEMPLATE', 'flags, frequency, model')
Line 30:
Line 31:
Line 32: @utils.memoized
Line 25: from . import cpuarch
Line 26:
Line 27:
Line 28: _SOURCE = '/proc/cpuinfo'
Line 29: _CPU_TEMPLATE = namedtuple('_CPU_TEMPLATE', 'flags, frequency, model')
Why not CpuInfo? why use constant name for a class?
Line 30:
Line 31:
Line 32: @utils.memoized
Line 33: def parse(source=_SOURCE):
Line 29: _CPU_TEMPLATE = namedtuple('_CPU_TEMPLATE', 'flags, frequency, model')
Line 30:
Line 31:
Line 32: @utils.memoized
Line 33: def parse(source=_SOURCE):
source parameter is not needed. For testing, we can monkeypatch cpuinfo._SOURCE.
Also, why is this public? this should be a private function.
Finally, if you rename it to _cpuinfo(), we can write the accessors like this:
def flags():
return _cpuinfo().flags
Which is little nicer.
Line 34: '''
Line 35: Parse cpuinfo-like file, keeping the values in module's runtime variables.
Line 36:
Line 37: Arguments:
Line 38:
Line 39: source Optional. Accepts a string indicating path to the cpuinfo-like
Line 40: file. If not supplied, default path (/proc/cpuinfo) is used.
Line 41: '''
Line 42: parsed_fields = [['powernv'], None, None]
Use a dict:
fields = {}
On ppc, add the flags before parsing:
if cpuarch.is_ppc(arch):
fields["flags"] = ["powernv"]
Line 43: counter_stop = len(parsed_fields)
Line 44: counter = 0
Line 45:
Line 46: if cpuarch.is_ppc(cpuarch.real()):
Line 39: source Optional. Accepts a string indicating path to the cpuinfo-like
Line 40: file. If not supplied, default path (/proc/cpuinfo) is used.
Line 41: '''
Line 42: parsed_fields = [['powernv'], None, None]
Line 43: counter_stop = len(parsed_fields)
Unneeded
Line 44: counter = 0
Line 45:
Line 46: if cpuarch.is_ppc(cpuarch.real()):
Line 47: counter += 1
Line 40: file. If not supplied, default path (/proc/cpuinfo) is used.
Line 41: '''
Line 42: parsed_fields = [['powernv'], None, None]
Line 43: counter_stop = len(parsed_fields)
Line 44: counter = 0
Uneeded
Line 45:
Line 46: if cpuarch.is_ppc(cpuarch.real()):
Line 47: counter += 1
Line 48:
Line 50: for line in info:
Line 51: if not line.strip():
Line 52: continue
Line 53:
Line 54: key, value = map(str.strip, line.split(':', 1))
Use:
[part.strip() for part in line.split(":", 1)]
Line 55:
Line 56: if key == 'flags':
Line 57: parsed_fields[0] = value.split()
Line 58: counter += 1
Line 69: parsed_fields[2] = value
Line 70: counter += 1
Line 71:
Line 72: if counter == counter_stop:
Line 73: break
Use instead:
if len(fields) == 3:
break
Line 74:
Line 75: return _CPU_TEMPLATE(*parsed_fields)
Line 76:
Line 77:
Line 71:
Line 72: if counter == counter_stop:
Line 73: break
Line 74:
Line 75: return _CPU_TEMPLATE(*parsed_fields)
Use:
return _CPU_TEMPLATE(**fields)
Line 76:
Line 77:
Line 78: def flags():
Line 79: '''
Line 89: '''
Line 90: return parse(_SOURCE).flags
Line 91:
Line 92:
Line 93: def mhz():
Rename to frequency()
Line 94: '''
Line 95: Get the CPU frequency.
Line 96:
Line 97: Returns:
--
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: 22
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