Martin Sivák has posted comments on this change. Change subject: Add IO tunables support to updateVmPolicy ......................................................................
Patch Set 22: (4 comments) http://gerrit.ovirt.org/#/c/28715/22/vdsm/virt/vmtune.py File vdsm/virt/vmtune.py: Line 151: Line 152: return result Line 153: Line 154: Line 155: def createDeviceIndex(ioTune): > Why 'Device'? Maybe something like This can actually be applied to the devices structure as well (it also has name and path) and it maps device name and device path to the device object (iotune in this case, but..). I can change it, but I find both names to be quite legible. Line 156: """ Line 157: Create by name / by path dictionaries from the XML representation. Line 158: Returns a tuple (by_name, by_path) where the items are the respective Line 159: dictionaries. Line 167: Line 168: for el in ioTune.getElementsByTagName("device"): Line 169: try: Line 170: ioTuneByPath[el.getAttribute("path")] = el Line 171: except KeyError: > Sorry, I don't quite get what can trigger a KeyError here The path is not mandatory. Line 172: pass Line 173: Line 174: try: Line 175: ioTuneByName[el.getAttribute("name")] = el Line 171: except KeyError: Line 172: pass Line 173: Line 174: try: Line 175: ioTuneByName[el.getAttribute("name")] = el > ditto The name is not mandatory, only one of path/name has to be specified. Line 176: except KeyError: Line 177: pass Line 178: Line 179: return ioTuneByName, ioTuneByPath Line 224: if ("path" in limit_object Line 225: and limit_object["path"] in ioTuneByPath): Line 226: ioTuneByPath[limit_object["path"]] = new_tune Line 227: Line 228: return count > Do we care of the actual number of tunables changed? Doesn't seem so. If th Well we do not care about it right now, but the number might give us a way of telling the user that some device was not found for example (future enhancement?). -- 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: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
