Change in vdsm[master]: vdsm: Add tuneBlockDevIo interface

wudxw at linux.vnet.ibm.com wudxw at linux.vnet.ibm.com
Tue Aug 20 13:44:04 UTC 2013


Mark Wu has posted comments on this change.

Change subject: vdsm: Add tuneBlockDevIo interface
......................................................................


Patch Set 10: Code-Review-1

(6 comments)

....................................................
File client/vdsClient.py
Line 1745:         return status['status']['code'], status['status']['message']
Line 1746: 
Line 1747:     def tuneBlockDevIo(self, args):
Line 1748:         params = {}
Line 1749:         if len(args) > 2:
if you check the number of args,  it should fail early.  Otherwise, you could just use the args without check.
Line 1750:             params = self._eqSplit(args[2:])
Line 1751:         params['vmId'] = args[0]
Line 1752:         params['dev'] = args[1]
Line 1753:         return self.ExecAndExit(self.s.tuneBlockDevIo(params))


....................................................
File vdsm/vm.py
Line 4289:         if msg is None:
Line 4290:             error = errCode[key]
Line 4291:         else:
Line 4292:             error = {'status': {'code': errCode[key]
Line 4293:                      ['status']['code'], 'message': msg}}
i think the code of build customized error status belongs to define.py itself.
It should be resolved in a separate patch.
Line 4294:         return error
Line 4295: 
Line 4296:     def _getBalloonInfo(self):
Line 4297:         for dev in self.conf['devices']:


Line 4337:         for device in self.conf['devices']:
Line 4338:             if device.get('device') == 'disk' and device.get('name'):
Line 4339:                 devName = device['name']
Line 4340:                 result = (device.get('specParams', {}).get('ioTune', {}))
Line 4341:                 results[devName] = result
I think we could only fill the tune info for the device tuned before. We should not extend the length of vm's running stats for useless info. Does it make sense?
Line 4342:         return results
Line 4343: 
Line 4344:     def _checkIoTuneInvalidParams(self, params):
Line 4345:         validKeys = set(('total_bytes_sec', 'read_bytes_sec',


Line 4344:     def _checkIoTuneInvalidParams(self, params):
Line 4345:         validKeys = set(('total_bytes_sec', 'read_bytes_sec',
Line 4346:                          'write_bytes_sec', 'total_iops_sec',
Line 4347:                          'write_iops_sec', 'read_iops_sec'))
Line 4348:         params_set = set(params)
paramsSet,   we should use consistent variable naming style.
Line 4349: 
Line 4350:         invalidParams = params_set - validKeys.intersection(params_set)
Line 4351:         invalidParamNames = ','.join(invalidParams)
Line 4352:         return invalidParamNames


Line 4346:                          'write_bytes_sec', 'total_iops_sec',
Line 4347:                          'write_iops_sec', 'read_iops_sec'))
Line 4348:         params_set = set(params)
Line 4349: 
Line 4350:         invalidParams = params_set - validKeys.intersection(params_set)
you could just use params_set - validKeys
Line 4351:         invalidParamNames = ','.join(invalidParams)
Line 4352:         return invalidParamNames
Line 4353: 
Line 4354:     def _checkIoTuneCategories(self, params):


Line 4347:                          'write_iops_sec', 'read_iops_sec'))
Line 4348:         params_set = set(params)
Line 4349: 
Line 4350:         invalidParams = params_set - validKeys.intersection(params_set)
Line 4351:         invalidParamNames = ','.join(invalidParams)
', '  is better.
Line 4352:         return invalidParamNames
Line 4353: 
Line 4354:     def _checkIoTuneCategories(self, params):
Line 4355:         categories = ("bytes", "iops")


-- 
To view, visit http://gerrit.ovirt.org/14394
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb33510a081d221af0f69d4dd2d55adf0b79efd2
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mei Liu <liumbj at linux.vnet.ibm.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck at redhat.com>
Gerrit-Reviewer: Eli Mesika <emesika 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
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list