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