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?).