Martin Sivák has posted comments on this change. Change subject: Add IO tunables support to updateVmPolicy ......................................................................
Patch Set 19: (9 comments) http://gerrit.ovirt.org/#/c/28715/19/tests/vmTests.py File tests/vmTests.py: Line 875: def testBuildCmdLinePPC64(self): Line 876: self.assertBuildCmdLine(CONF_TO_DOMXML_PPC64) Line 877: Line 878: def _xml_sanitizer(self, text): Line 879: return re.sub(">[\t\n ]+<", "><", text).strip() > this could be a module-private function. No big deal, can be extracted in a I do not believe it is generic enough at this moment, so I would like to improve it in separate patch. Line 880: Line 881: def testUpdateVmPolicy(self): Line 882: with FakeVM() as machine: Line 883: dom = FakeDomain() http://gerrit.ovirt.org/#/c/28715/19/vdsm/rpc/vdsmapi-schema.json File vdsm/rpc/vdsmapi-schema.json: Line 2348: # @VmDiskDeviceTuneLimits: Line 2349: # Line 2350: # Extra parameters for VM disk devices. Line 2351: # Line 2352: # @name: #optional The name of the taget device > typo: taRget Done Line 2353: # Line 2354: # @path: #optional The path of the taget device Line 2355: # Line 2356: # @guaranteed: IO tune parameters - guranteed values Line 2350: # Extra parameters for VM disk devices. Line 2351: # Line 2352: # @name: #optional The name of the taget device Line 2353: # Line 2354: # @path: #optional The path of the taget device > ditto Done Line 2355: # Line 2356: # @guaranteed: IO tune parameters - guranteed values Line 2357: # Line 2358: # @maximum: IO tune parameters - the hard limits Line 2352: # @name: #optional The name of the taget device Line 2353: # Line 2354: # @path: #optional The path of the taget device Line 2355: # Line 2356: # @guaranteed: IO tune parameters - guranteed values > typo: guAranteed Done Line 2357: # Line 2358: # @maximum: IO tune parameters - the hard limits Line 2359: # Line 2360: # Since: 4.15.0 http://gerrit.ovirt.org/#/c/28715/19/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 3682: def _ioTuneValuesToDom(self, values, dom): Line 3683: ops = ["total", "read", "write"] Line 3684: units = ["bytes", "iops"] Line 3685: Line 3686: for op, unit in itertools.product(ops, units): > Nice, but how about an explicit tuple of constants? do you plan to add more Done Line 3687: name = op + "_" + unit + "_sec" Line 3688: if name in values and values[name] >= 0: Line 3689: el = XMLElement(name) Line 3690: el.appendTextNode(str(values[name])) Line 3702: Line 3703: def _ioTuneDomToValues(self, dom): Line 3704: values = {} Line 3705: Line 3706: # Parse device name > redundant comment Done Line 3707: if dom.hasAttribute("name"): Line 3708: values["name"] = dom.getAttribute("name") Line 3709: Line 3710: # Parse device path Line 3706: # Parse device name Line 3707: if dom.hasAttribute("name"): Line 3708: values["name"] = dom.getAttribute("name") Line 3709: Line 3710: # Parse device path > ditto Done Line 3711: if dom.hasAttribute("path"): Line 3712: values["path"] = dom.getAttribute("path") Line 3713: Line 3714: # Parse guranteed and maximum ioTune limits Line 3710: # Parse device path Line 3711: if dom.hasAttribute("path"): Line 3712: values["path"] = dom.getAttribute("path") Line 3713: Line 3714: # Parse guranteed and maximum ioTune limits > ditto Done Line 3715: els = dom.getElementsByTagName("guaranteed") Line 3716: if els: Line 3717: values["guaranteed"] = {} Line 3718: self._collectInnerElements(els[0], values["guaranteed"]) Line 3871: if ("path" in limit_object Line 3872: and limit_object["path"] in ioTuneByPath): Line 3873: ioTuneByPath[limit_object["path"]] = new_tune Line 3874: Line 3875: metadata_modified = True > Most, if not all, of this code seems to not use self, and since vm.py is al Done, I created vmtune.py and put most of the code there. Line 3876: Line 3877: del params['ioTune'] Line 3878: Line 3879: # Check remaining fields in params and report the list of unsupported -- To view, visit http://gerrit.ovirt.org/28715 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4ed108fbb2bf9d9af80577b2905242bf9f8c4221 Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Sivák <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[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
