Dan Kenigsberg has posted comments on this change.
Change subject: Adding getBiosInfo API to vdsm cli and merge bios information
with vdsGetCaps
......................................................................
Patch Set 17: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File vdsm_api/vdsmapi-schema.json
Line 697: # @systemUUID: Host's BIOS UUID
Line 698: #
Line 699: # @systemVersion: Host's hardware version
Line 700: #
Line 701: # Since: 4.10.0
nit: it's 4.10.3 at least
Line 702: ##
Line 703: {'type': 'BiosInformation',
Line 704: 'data': {'systemManufacturer': 'str',
Line 705: 'systemProductName': 'str', 'systemVersion': 'str',
....................................................
File vdsm/dmidecodeUtil.py
Line 21: import dmidecode
Line 22: from vdsm import utils
Line 23:
Line 24:
Line 25: def __leafDict(d):
sorry, but I do not understand this function (or its name). It seems to
deepcopy d.
Line 26: ret = {}
Line 27: for k, v in d.iteritems():
Line 28: if isinstance(v, dict):
Line 29: ret.update(__leafDict(v))
Line 35: @utils.memoized
Line 36: def getAllDmidecodeInfo():
Line 37: myLeafDict = {}
Line 38: for k in ('system', 'bios', 'cache', 'processor', 'chassis',
'memory'):
Line 39: myLeafDict[k] = __leafDict(dmidecode.__dict__[k]())
it's considered nicer to do
getattr(dmidecode, k)
Line 40: return myLeafDict
Line 41:
Line 42:
Line 43: @utils.memoized
Line 43: @utils.memoized
Line 44: def getSystemStructure():
Line 45: dmiInfo = getAllDmidecodeInfo()
Line 46: sysStruct = {}
Line 47: for k1, k2 in (('system', 'Manufacturer'),
Do you plan to have anything but the system section returned by
getSystemStructure() ? if so, the name of the function is wrong. if not, the
repeated 'system' here is redundant.
Line 48: ('system', 'Product Name'),
Line 49: ('system', 'Version'),
Line 50: ('system', 'Serial Number'),
Line 51: ('system', 'UUID'),
....................................................
File vdsm/supervdsmServer.py
Line 38: _glusterEnabled = False
Line 39: from parted_utils import getDevicePartedInfo as _getDevicePartedInfo
Line 40: from md_utils import getMdDeviceUuidMap as _getMdDeviceUuidMap
Line 41:
Line 42: import platform
this belongs way up, as it is from stdlib.
Line 43: from lsblk import getLsBlk as _getLsBlk
Line 44: from storage.multipath import getScsiSerial as _getScsiSerial
Line 45: from storage.iscsi import forceIScsiScan as _forceIScsiScan
Line 46: from storage.iscsi import getDevIscsiInfo as _getdeviSCSIinfo
--
To view, visit http://gerrit.ovirt.org/9258
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic429ef101fcf9047c4b552405314dc7ba9ba07a0
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Andrew Cathrow <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Pradipta Banerjee <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: ShaoHe Feng <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
