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

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

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

Patch Set 14: Code-Review-1

(3 comments)

....................................................
File vdsm/vm.py
Line 4326:             # persist the target value to make it consistent after recovery
Line 4327:             self.saveState()
Line 4328:             return {'status': doneCode}
Line 4329: 
Line 4330:     def _getTuneBlkDevIoInfo(self):
This might be useless. More info below.
Line 4331:         # Only tuned disk IO information is returned.
Line 4332:         results = {}
Line 4333:         for device in self.conf['devices']:
Line 4334:             if device.get('device') == 'disk' and device.get('name'):

Line 4350:             invalidParamNames = ', '.join(invalidParams)
Line 4351:             raise ValueError('Parameter %s name(s) are invalid' %
Line 4352:                              invalidParamNames)
Line 4353: 
Line 4354:     def _checkIoTuneCategories(self, params):
I am not a big fan of checks re-writes. If this is already checked by libvirt then let's just wait for it to fail when we call setBlockIoTune.
Line 4355:         categories = ("bytes", "iops")
Line 4356:         for category in categories:
Line 4357:             if params.get('total_' + category + '_sec', 0) and \
Line 4358:                     (params.get('read_' + category + '_sec', 0) or

Line 4406:                 errMsg = 'Get new block I/O Tune params failed after setting'
Line 4407:                 self.log.exception(errMsg)
Line 4408:                 return getErrCode('tuneBlkDevIoErr', errMsg)
Line 4409:             else:
Line 4410:                 for device in self.conf['devices']:
I'm not sure I like this because if we crash between setBlockIoTune and saveState loop we basically lose this information.

Best thing here would probably be add blockIoTune in the vm stats thread (and forget about persisting these values). Therefore _getTuneBlkDevIoInfo might be useless and could be removed.
Line 4411:                     if device.get('name') == dev or device.get('path') == dev:
Line 4412:                         device.setdefault('specParams', {})
Line 4413:                         device['specParams']['ioTune'] = info
Line 4414: