Change in vdsm[master]: vdsm: Add blkIoTune support at vm creation
gvallare at redhat.com
gvallare at redhat.com
Wed May 15 07:51:51 UTC 2013
Giuseppe Vallarelli has posted comments on this change.
Change subject: vdsm: Add blkIoTune support at vm creation
......................................................................
Patch Set 5: I would prefer that you didn't submit this
(3 inline comments)
....................................................
File vdsm/libvirtvm.py
Line 1178: not utils.getUserPermissions(constants.QEMU_PROCESS_USER,
Line 1179: self.path)['write']):
Line 1180: diskelem.appendChildWithArgs('readonly')
Line 1181:
Line 1182: if hasattr(self, 'blkIoTune'):
Now the code is more robust, but I would move all the validation logic to an external function and leave here only the XMLElement creation logic. Having a few more tests for the validation logic would be good as well.
Line 1183: validParamNames = ('total_bytes_sec', 'read_bytes_sec',
Line 1184: 'write_bytes_sec', 'total_iops_sec',
Line 1185: 'write_iops_sec', 'read_iops_sec')
Line 1186: for key, value in self.blkIoTune.iteritems():
Line 1179: self.path)['write']):
Line 1180: diskelem.appendChildWithArgs('readonly')
Line 1181:
Line 1182: if hasattr(self, 'blkIoTune'):
Line 1183: validParamNames = ('total_bytes_sec', 'read_bytes_sec',
These are disk device tune params, I would use the same name or something closer to it, used in the json schema.
Line 1184: 'write_bytes_sec', 'total_iops_sec',
Line 1185: 'write_iops_sec', 'read_iops_sec')
Line 1186: for key, value in self.blkIoTune.iteritems():
Line 1187: try:
Line 1198: except ValueError:
Line 1199: raise Exception('an integer is required for blkIoTune'
Line 1200: ' parameter %s' % key)
Line 1201:
Line 1202: categories = ("bytes", "iops")
I would extract a function checkCategories
Line 1203: for i, category in enumerate(categories):
Line 1204: if self.blkIoTune.get('total_' + category + '_sec', 0) and \
Line 1205: self.blkIoTune.get('read_' + category + '_sec', 0) or \
Line 1206: self.blkIoTune.get('total_' + category + '_sec', 0) \
--
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mei Liu <liumbj at linux.vnet.ibm.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Mei Liu <liumbj at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
More information about the vdsm-patches
mailing list