Mark Wu has posted comments on this change. Change subject: vdsm: Add blkIoTune support at vm creation ......................................................................
Patch Set 8: I would prefer that you didn't submit this (2 inline comments) .................................................... File tests/libvirtvmTests.py Line 403: devConfs = [] Line 404: for i in range(4): Line 405: devConf = basicConf.copy() Line 406: devConf.update({'blkIoTune': tuneConfs[i]}) Line 407: devConfs.append(devConf) It could be more pythonic to use: devConfs = [dict(blkIoTure=tuneConf, **basicConf) for tuneConf in tuneConfs] Line 408: Line 409: expectedExceptMsgs = [ Line 410: 'A non-zero total value and non-zero read/write value for' Line 411: ' iops_sec can not be set at the same time', .................................................... File vdsm/libvirtvm.py Line 1131: self.blkIoTune.get('write_' + category + '_sec', 0)): Line 1132: raise Exception('A non-zero total value and non-zero' Line 1133: ' read/write value for %s_sec can not be' Line 1134: ' set at the same time' % category) Line 1135: I just suggested moving _checkIoTuneParams here. I think it's good to exact into a separate function. Why did you merge it ? Line 1136: def getXML(self): Line 1137: """ Line 1138: Create domxml for disk/cdrom/floppy. Line 1139: -- To view, visit http://gerrit.ovirt.org/14636 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie4034648620ed9212c06c12607bb889d97cca9d6 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Mei Liu <liu...@linux.vnet.ibm.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Giuseppe Vallarelli <gvall...@redhat.com> Gerrit-Reviewer: Mark Wu <wu...@linux.vnet.ibm.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Mei Liu <liu...@linux.vnet.ibm.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches