Change in vdsm[master]: Utility methods and conf for blkio limits MOM integration

nsoffer at redhat.com nsoffer at redhat.com
Tue Jun 10 20:41:16 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: Utility methods and conf for blkio limits MOM integration
......................................................................


Patch Set 1:

(7 comments)

Too many TODOs

http://gerrit.ovirt.org/#/c/28576/1/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:

Line 173:         ('vm_sample_cpu_tune_interval', '15', None),
Line 174: 
Line 175:         ('vm_sample_cpu_tune_window', '2', None),
Line 176: 
Line 177:         ('vm_sample_blkio_tune_interval', '15', None),
Please add help message explaining this value and specifying the default value.
Line 178: 
Line 179:         ('vm_sample_blkio_tune_window', '2', None),
Line 180: 
Line 181:         ('trust_store_path', '@TRUSTSTORE@',


Line 175:         ('vm_sample_cpu_tune_window', '2', None),
Line 176: 
Line 177:         ('vm_sample_blkio_tune_interval', '15', None),
Line 178: 
Line 179:         ('vm_sample_blkio_tune_window', '2', None),
Please add help message explaining this value and specifying the default value.
Line 180: 
Line 181:         ('trust_store_path', '@TRUSTSTORE@',
Line 182:             'Where the certificates and keys are situated.'),
Line 183: 


http://gerrit.ovirt.org/#/c/28576/1/tests/functional/momTests.py
File tests/functional/momTests.py:

Line 131:             self.assertEqual(status, SUCCESS, msg)
Line 132: 
Line 133:     def _setBlkIoTune(self, disk, blkioMap):
Line 134:         # Get disk's statistics before the operation.
Line 135: #        TODO
Why not:

    # TODO: ...
Line 136:         self.assertEqual(status, SUCCESS, msg)
Line 137: 
Line 138:         candidateStats = filter(self._statsOK, statsList)
Line 139: 


Line 132: 
Line 133:     def _setBlkIoTune(self, disk, blkioMap):
Line 134:         # Get disk's statistics before the operation.
Line 135: #        TODO
Line 136:         self.assertEqual(status, SUCCESS, msg)
Are you sure about this line?
Line 137: 
Line 138:         candidateStats = filter(self._statsOK, statsList)
Line 139: 
Line 140:         for stats in candidateStats:


Line 140:         for stats in candidateStats:
Line 141:             status, msg = self.s.setDiskBlkioTuneMap(
Line 142:                 stats['vmId'],
Line 143:                 disk,
Line 144:                 #TODO#)
TODO?
Line 145:             self.assertEqual(status, SUCCESS, msg)
Line 146: 
Line 147:     def _setPolicy(self, policy):
Line 148:         curpath = os.path.dirname(__file__)


http://gerrit.ovirt.org/#/c/28576/1/vdsm/mom.d/05-blkiotune.policy
File vdsm/mom.d/05-blkiotune.policy:

Line 1: ### Auto-blkioTune ###############################################################
Line 2: #
Line 3: #TODO
Do you expect this to be merged?


http://gerrit.ovirt.org/#/c/28576/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 288:         return infos
Line 289: 
Line 290:     def _sampleBlkIoTune(self):
Line 291:         infos = self._dom.schedulerParameters()
Line 292:         #TODO _getDiskBlkioTuneMap(self, disk, stats)
Really?
Line 293:         return infos
Line 294: 
Line 295:     def _diff(self, prev, curr, val):
Line 296:         return prev[val] - curr[val]


-- 
To view, visit http://gerrit.ovirt.org/28576
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I43be5627e5365fee1145a75e91fb5b7714e46aaf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchaplik at redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck at redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer 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