Francesco Romani has posted comments on this change. Change subject: Add IO tunables support to updateVmPolicy ......................................................................
Patch Set 22: (1 comment) http://gerrit.ovirt.org/#/c/28715/22/vdsm/virt/vmtune.py File vdsm/virt/vmtune.py: Line 167: Line 168: for el in ioTune.getElementsByTagName("device"): Line 169: try: Line 170: ioTuneByPath[el.getAttribute("path")] = el Line 171: except KeyError: > The path is not mandatory. I see. Then please here and right below in this function add a comment (a copy/paste of the answer above is just fine) stating this, and please switch to the if el.hasAttribute("path"): ioTuneByPath[el.getAttribute("path")] = el idiom like is done above in the code (ioTuneDomToValues). I think in this particular context LBYL is better before EAFP. Line 172: pass Line 173: Line 174: try: Line 175: ioTuneByName[el.getAttribute("name")] = el -- 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 <[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
