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

wudxw at linux.vnet.ibm.com wudxw at linux.vnet.ibm.com
Mon May 13 13:18:19 UTC 2013


Mark Wu has posted comments on this change.

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


Patch Set 1: I would prefer that you didn't submit this

(5 inline comments)

....................................................
File vdsm/libvirtvm.py
Line 2759:         except KeyError:
Line 2760:             return errFunc(msg='missing parameter dev')
Line 2761: 
Line 2762:         bytesKeyList = ['total_bytes_sec', 'read_bytes_sec', 'write_bytes_sec']
Line 2763:         iopsKeyList = ['total_iops_sec', 'write_iops_sec', 'read_iops_sec']
bytesKeys = set(('total_bytes_sec', 'read_bytes_sec', 'write_bytes_sec'))

iopsKeys = set(('total_iops_sec', 'write_iops_sec', 'read_iops_sec'))
Line 2764:         set_bytes = False
Line 2765:         set_iops = False
Line 2766:         try:
Line 2767:             for key in params:


Line 2761: 
Line 2762:         bytesKeyList = ['total_bytes_sec', 'read_bytes_sec', 'write_bytes_sec']
Line 2763:         iopsKeyList = ['total_iops_sec', 'write_iops_sec', 'read_iops_sec']
Line 2764:         set_bytes = False
Line 2765:         set_iops = False
params_set = set(params)
set_bytes = True if len(bytesKeys.intersection(params_set)) > 0 else False
set_iops = True if len(iopsKeys.intersection(params_set)) > 0 else False

probably you need validate len(bytesKeys.intersection(params_set)) < 2 to make sure no conflict options.
Line 2766:         try:
Line 2767:             for key in params:
Line 2768:                 if key in bytesKeyList:
Line 2769:                     params[key] = int(params[key])


Line 2763:         iopsKeyList = ['total_iops_sec', 'write_iops_sec', 'read_iops_sec']
Line 2764:         set_bytes = False
Line 2765:         set_iops = False
Line 2766:         try:
Line 2767:             for key in params:
params[key] = int(value) for key, value in params.iteritems():
Line 2768:                 if key in bytesKeyList:
Line 2769:                     params[key] = int(params[key])
Line 2770:                     set_bytes = True
Line 2771:                 elif key in iopsKeyList:


Line 2791:                     if oldInfo:
Line 2792:                         if not set_bytes:
Line 2793:                             for bytesKey in bytesKeyList:
Line 2794:                                 info.update({bytesKey: oldInfo[bytesKey]})
Line 2795:                         elif not set_iops:
should be 'if',  not elif,  right?
Line 2796:                             for bytesKey in iopsKeyList:
Line 2797:                                 info.update({bytesKey: oldInfo[bytesKey]})
Line 2798: 
Line 2799:                     device['blkIoTune'] = info


Line 2799:                     device['blkIoTune'] = info
Line 2800:             # persist the tune value to make it consistent after recovery
Line 2801:             self.saveState()
Line 2802:             return {'status': doneCode}
Line 2803:         except (libvirt.libvirtError, LookupError, TypeError, OverflowError), \
I think you could just capture a generic exception here since you don't have special handling on different kinds of exception.
Line 2804:                 e:
Line 2805:             return errFunc(msg=e.message)
Line 2806: 
Line 2807:     def _getUnderlyingDeviceAddress(self, devXml):


--
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: 1
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: 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