Change in vdsm[master]: vdsm: introduce cpuinfo module
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: vdsm: introduce cpuinfo module
......................................................................
Patch Set 4: Code-Review-1
(10 comments)
nice patch, deceptively large: most of it seems plain renames.
A few comments inside, -1 for visibility, but I want this (or an evolution of) in.
https://gerrit.ovirt.org/#/c/46912/4//COMMIT_MSG
Commit Message:
Line 3: AuthorDate: 2015-10-01 16:18:42 +0200
Line 4: Commit: Martin Polednik <mpolednik(a)redhat.com>
Line 5: CommitDate: 2015-10-02 16:24:22 +0200
Line 6:
Line 7: vdsm: introduce cpuinfo module
maybe
lib: introduce cpuinfo module
Line 8:
Line 9: The objective of the new module is encapsulating functionality related
Line 10: to CPU from bare-metal standpoint. That means parsing cpuinfo and
Line 11: exposing host architecture. This approach cleans up 'caps' module and,
Line 9: The objective of the new module is encapsulating functionality related
Line 10: to CPU from bare-metal standpoint. That means parsing cpuinfo and
Line 11: exposing host architecture. This approach cleans up 'caps' module and,
Line 12: more importantly, moves the code to vdsm library - allowing hooks to
Line 13: work with cpu info and architecture.
so we can have fakekvm/fake* as hook, right? If so (excellent goal BTW, we're chasing this since long time), it is good to state that in the commit message (one line is enough).
Line 14:
Line 15: Also adds tests for POWER8 cpuinfo and unknown architectures (should we
Line 16: encounter them).
Line 17:
https://gerrit.ovirt.org/#/c/46912/4/lib/vdsm/cpuinfo.py
File lib/vdsm/cpuinfo.py:
Line 20:
Line 21: import platform
Line 22:
Line 23:
Line 24: class UnsupportedArchitecture(RuntimeError):
Why RuntimeError and not Exception?
Line 25: def __init__(self, targetarch=None):
Line 26: if not targetarch:
Line 27: targetarch = arch()
Line 28:
Line 43: if targetarch == Architecture.X86_64:
Line 44: return 'x86'
Line 45:
Line 46: if targetarch in Architecture.POWER:
Line 47: return 'ppc64'
otherwise returns None. Which may be fine and intentional, but better explicit than implicit.
Line 48:
Line 49: _cpu_arch_to_architecture = {
Line 50: 'x86_64': Architecture.X86_64,
Line 51: 'ppc64': Architecture.PPC64,
Line 49: _cpu_arch_to_architecture = {
Line 50: 'x86_64': Architecture.X86_64,
Line 51: 'ppc64': Architecture.PPC64,
Line 52: 'ppc64le': Architecture.PPC64LE
Line 53: }
this is a module level constant, right? so _IT_SHOULD_BE_IN_UPPERCASE
Line 54:
Line 55: _cpus = {}
Line 56:
Line 57:
Line 54:
Line 55: _cpus = {}
Line 56:
Line 57:
Line 58: def parse(source='/proc/cpuinfo'):
I wonder how it will look if we require to call this explicitely, when the application starts.
Line 59: with open(source) as cpuinfo:
Line 60: cpu = 0
Line 61:
Line 62: for line in cpuinfo:
https://gerrit.ovirt.org/#/c/46912/4/lib/vdsm/utils.py
File lib/vdsm/utils.py:
Line 808: if os.path.exists(constants.P_VDSM_NODE_ID):
Line 809: with open(constants.P_VDSM_NODE_ID) as f:
Line 810: __hostUUID = f.readline().replace("\n", "")
Line 811: else:
Line 812: if cpuinfo.arch() in (cpuinfo.Architecture.X86_64):
feel free to drop this if you don't like it.
for a future patch:
maybe
cpuinfo.is_x86_64()
?
we are not gonna support a ton of arches, after all, and this reads a little nicer, especially for POWER
Line 813: ret, out, err = execCmd([constants.EXT_DMIDECODE,
Line 814: "-s",
Line 815: "system-uuid"],
Line 816: raw=True,
https://gerrit.ovirt.org/#/c/46912/4/tests/Makefile.am
File tests/Makefile.am:
Line 20:
Line 21: include $(top_srcdir)/build-aux/Makefile.subs
Line 22:
Line 23: SUBDIRS = \
Line 24: cpu\
cpuinfo maybe?
Line 25: functional \
Line 26: devices \
Line 27: integration \
Line 28: $(NULL)
https://gerrit.ovirt.org/#/c/46912/4/tests/cpuinfoTests.py
File tests/cpuinfoTests.py:
Line 27:
Line 28: from vdsm import cpuinfo
Line 29:
Line 30:
Line 31: class TestCpuinfo(TestCaseBase):
IMO better CpuInfoTests()
Line 32:
Line 33: @MonkeyPatch(platform, 'machine', lambda: cpuinfo.Architecture.X86_64)
Line 34: def test_cpuinfo_E5649_x86_64(self):
Line 35: self._load_cpuinfo('cpuinfo_E5649_x86_64.out')
Line 33: @MonkeyPatch(platform, 'machine', lambda: cpuinfo.Architecture.X86_64)
Line 34: def test_cpuinfo_E5649_x86_64(self):
Line 35: self._load_cpuinfo('cpuinfo_E5649_x86_64.out')
Line 36: self.assertEqual(set(cpuinfo.flags()),
Line 37: set('''fpu vme de pse tsc msr pae mce cx8 apic mtrr pge
if you're gonna using split(), why don't split this already manually?
set((
'fpu',
'vme',
...
Line 38: mca cmov pat pse36 clflush dts acpi mmx fxsr sse
Line 39: sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm
Line 40: constant_tsc arch_perfmon pebs bts rep_good
Line 41: xtopology nonstop_tsc aperfmperf pni pclmulqdq
--
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 7 months
Change in vdsm[master]: vdsm: introduce cpuinfo module
by automation＠ovirt.org
automation(a)ovirt.org has posted comments on this change.
Change subject: vdsm: introduce cpuinfo module
......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
--
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: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8 years, 7 months
Change in vdsm[master]: migration: added support for convergance schedule
by automation＠ovirt.org
automation(a)ovirt.org has posted comments on this change.
Change subject: migration: added support for convergance schedule
......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
--
To view, visit https://gerrit.ovirt.org/46940
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I989cff12d08ef1cab36bd10df7daaa999a8dac14
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjelinek(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8 years, 7 months
Change in vdsm[ovirt-3.6]: caps: get host uuid on ppc64le
by Martin Polednik
Martin Polednik has uploaded a new change for review.
Change subject: caps: get host uuid on ppc64le
......................................................................
caps: get host uuid on ppc64le
On Power, the uuid is determined by parsing the device tree. The code
for parsing lacks ppc64le architecture that needs to be added in order
not to return None as UUID (breaking getVdsCaps).
Change-Id: I411a635fcf724e255a8f000bdd491874c331c4a4
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1201513
Signed-off-by: Martin Polednik <mpolednik(a)redhat.com>
---
M lib/vdsm/utils.py
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/00/46900/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py
index 796ae0b..e435e10 100644
--- a/lib/vdsm/utils.py
+++ b/lib/vdsm/utils.py
@@ -825,7 +825,7 @@
__hostUUID = out.strip()
else:
logging.warning('Could not find host UUID.')
- elif arch in ('ppc', 'ppc64'):
+ elif arch in ('ppc', 'ppc64', 'ppc64le'):
# eg. output IBM,03061C14A
try:
with open('/proc/device-tree/system-id') as f:
--
To view, visit https://gerrit.ovirt.org/46900
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I411a635fcf724e255a8f000bdd491874c331c4a4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com>
8 years, 7 months
Change in vdsm[master]: periodic: explicitely track domain availability
by automation＠ovirt.org
automation(a)ovirt.org has posted comments on this change.
Change subject: periodic: explicitely track domain availability
......................................................................
Patch Set 21:
* Update tracker::#1250839::OK
* Check Bug-Url::OK
* Check Public Bug::#1250839::OK, public bug
* Check Product::#1250839::OK, Correct classification oVirt
* Check TR::SKIP, not in a monitored branch (ovirt-3.5 ovirt-3.4 ovirt-3.3 ovirt-3.2)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
--
To view, visit https://gerrit.ovirt.org/44544
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1b1d5173bac8e288474581092b8132dc0df03ad4
Gerrit-PatchSet: 21
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8 years, 7 months
Change in vdsm[master]: host stats: Add cpusStatus to vdsStats
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: host stats: Add cpusStatus to vdsStats
......................................................................
Patch Set 7:
+1 because of inline comments. Seems OK.
--
To view, visit https://gerrit.ovirt.org/46270
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I73c92f563c7c3e8ffbbcf639eba27fbe55389994
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Roman Mohr <rmohr(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofrenkel(a)redhat.com>
Gerrit-Reviewer: Roman Mohr <rmohr(a)redhat.com>
Gerrit-Reviewer: Roy Golan <rgolan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8 years, 7 months
Change in vdsm[master]: host stats: Add cpusStatus to vdsStats
by fromani＠redhat.com
Francesco Romani has posted comments on this change.
Change subject: host stats: Add cpusStatus to vdsStats
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/46270/7/tests/samplingTests.py
File tests/samplingTests.py:
Line 271: self._hs = sampling.HostStatsThread(self.log)
Line 272: self.assertEquals(self._hs.get(), expected)
Line 273:
Line 274: def testCpusStatus(self):
Line 275: cpusStatus = [True, False, True, False]
do we really need this temporary? Please inline it
Line 276:
Line 277: def get():
Line 278: class LibvirtConnection(object):
Line 279: def getCPUMap(self):
--
To view, visit https://gerrit.ovirt.org/46270
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I73c92f563c7c3e8ffbbcf639eba27fbe55389994
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Roman Mohr <rmohr(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofrenkel(a)redhat.com>
Gerrit-Reviewer: Roman Mohr <rmohr(a)redhat.com>
Gerrit-Reviewer: Roy Golan <rgolan(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 7 months
Change in vdsm[master]: vdsm: isolate cpu info and architecture in a new module
by automation＠ovirt.org
automation(a)ovirt.org has posted comments on this change.
Change subject: vdsm: isolate cpu info and architecture in a new module
......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
--
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8 years, 7 months
Change in vdsm[master]: vdsm: isolate cpu info and architecture in a new module
by Martin Polednik
Martin Polednik has posted comments on this change.
Change subject: vdsm: isolate cpu info and architecture in a new module
......................................................................
Patch Set 2:
(2 comments)
https://gerrit.ovirt.org/#/c/46912/2/tests/nettestlib.py
File tests/nettestlib.py:
Line 101: _IFF_NO_PI = 0x1000
Line 102: if cpuinfo.arch() == cpuinfo.Architecture.X86_64:
Line 103: _TUNSETIFF = 0x400454ca
Line 104: elif cpuinfo.arch() in 'ppc64':
Line 105: _TUNSETIFF = 0x800454ca
!remindme of code smell
Line 106: else:
Line 107: raise cpuinfo.UnsupportedArchitecture
Line 108:
Line 109: _deviceListener = None
https://gerrit.ovirt.org/#/c/46912/2/vdsm/caps.py
File vdsm/caps.py:
Line 390: arch = 'x86'
Line 391:
Line 392: # Same goes for ppc64le
Line 393: if cpuinfo.arch() in cpuinfo.Architecture.POWER:
Line 394: arch = 'ppc64'
Might make sense to move the non-standard mappings to cpuinfo.
Line 395:
Line 396: architectureElement = None
Line 397:
Line 398: architectureElements = cpu_map.findall('arch')
--
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
8 years, 7 months
Change in vdsm[master]: vdsm: isolate cpu info and architecture in a new module
by automation＠ovirt.org
automation(a)ovirt.org has posted comments on this change.
Change subject: vdsm: isolate cpu info and architecture in a new module
......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
--
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No
8 years, 7 months