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 <[email protected]> Gerrit-Reviewer: Doron Fediuck <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
