Change in vdsm[master]: core: Add boot time to the getVdsStats API
danken at redhat.com
danken at redhat.com
Thu Mar 27 17:58:48 UTC 2014
Dan Kenigsberg has posted comments on this change.
Change subject: core: Add boot time to the getVdsStats API
......................................................................
Patch Set 6: Code-Review-1
(3 comments)
http://gerrit.ovirt.org/#/c/25877/6/tests/samplingTests.py
File tests/samplingTests.py:
Line 52: procs_blocked 0
Line 53: """
Line 54:
Line 55: def testBootTimeParsing(self):
Line 56: (f, path) = tempfile.mkstemp()
use
with namedTemporaryDir()
to avoid leaving garbage behind when assertion is raised.
Line 57: os.write(f, self.boot_time_fixture_good)
Line 58: os.close(f)
Line 59: self.assertTrue(sampling.getBootTime(path) == 1395249141)
Line 60: os.remove(path)
http://gerrit.ovirt.org/#/c/25877/6/vdsm/sampling.py
File vdsm/sampling.py:
Line 148:
Line 149:
Line 150: @utils.memoized
Line 151: def getBootTime(filepath=None):
Line 152: if filepath is None:
What's the benefit of this clause over putting /proc/stat on the arg list?
The "None" trick is useful with mutables, not with strings.
Line 153: filepath = '/proc/stat'
Line 154:
Line 155: with file(filepath) as f:
Line 156: btime_line = [l for l in f.readlines() if l.startswith('btime')][0]
Line 460: stats['cpuLoad'] = hs1.cpuLoad
Line 461:
Line 462: stats['diskStats'] = hs1.diskStats
Line 463: stats['thpState'] = hs1.thpState
Line 464: if hasattr(hs1, 'bootTime'):
how could it possibly be that bootTime is missing?
More importantly: getBootTime() can never hang and can never change. It should not be copied repeatedly on each sample; Why not have a simple
stats['bootTime'] = getBootTime()
here?
Line 465: stats['bootTime'] = hs1.bootTime
Line 466: return stats
Line 467:
Line 468: def _getInterfacesStats(self):
--
To view, visit http://gerrit.ovirt.org/25877
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I319e619cdaecac2f86d0154e3adbb3beda9c57d6
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov <dkuznets at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list