Timothy Asir has posted comments on this change.
Change subject: Provide device details using python-blivet module
......................................................................
Patch Set 4:
(10 comments)
http://gerrit.ovirt.org/#/c/35028/4/vdsm/gluster/storagedev.py
File vdsm/gluster/storagedev.py:
Line 20:
Line 21: import blivet
Line 22: from . import makePublic
Line 23:
Line 24: _bb = None
1. You could user better name like _blivet_env ?
1. Done
2. No
Line 25:
Line 26:
Line 27: def _getBlivet():
Line 28: global _bb
Line 23:
Line 24: _bb = None
Line 25:
Line 26:
Line 27: def _getBlivet():
If you change global name, change this function name accordingly.
Done
Line 28: global _bb
Line 29: if _bb:
Line 30: return _bb
Line 31:
Line 30: return _bb
Line 31:
Line 32: _bb = blivet.Blivet()
Line 33: _bb.reset()
Line 34: return _bb
This could be done even simpler/cleaner like
Done
Line 35:
Line 36:
Line 37: @makePublic
Line 38: def hostDeviceList(devType):
Line 34: return _bb
Line 35:
Line 36:
Line 37: @makePublic
Line 38: def hostDeviceList(devType):
Please change this name to storageDeviceList which gives more meaning
Done
Line 39: b = _getBlivet()
Line 40: deviceInfo = []
Line 41: for device in b.devices:
Line 42: if devType != '':
Line 38: def hostDeviceList(devType):
Line 39: b = _getBlivet()
Line 40: deviceInfo = []
Line 41: for device in b.devices:
Line 42: if devType != '':
1. you should use
Done
Line 43: if devType != device.type:
Line 44: continue
Line 45: info = {}
Line 46: info['name'] = device.name
Line 45: info = {}
Line 46: info['name'] = device.name
Line 47: if device.parents:
Line 48: info['parent'] = device.parents[0].name
Line 49: info['kids'] = device.kids
What does device.kids have? If they are available as b.devices, why
do we
device.kids provides no of dependent count which is a numeric value.
This will helps to know whether we can use this device or not.
Line 50: if hasattr(device.format, 'mountpoint'):
Line 51: info['mount'] = device.format.mountpoint
Line 52: info['mountopts'] = device.format.mountopts
Line 53: info['fileSystem'] = device.format.type
Line 49: info['kids'] = device.kids
Line 50: if hasattr(device.format, 'mountpoint'):
Line 51: info['mount'] = device.format.mountpoint
Line 52: info['mountopts'] = device.format.mountopts
Line 53: info['fileSystem'] = device.format.type
I believe you should send empty values when device.format is missing
to eng
Done
Line 54: info['model'] = device.model
Line 55: info['type'] = device.type
Line 56: if device.format.uuid:
Line 57: info['uuid'] = device.format.uuid
Line 53: info['fileSystem'] = device.format.type
Line 54: info['model'] = device.model
Line 55: info['type'] = device.type
Line 56: if device.format.uuid:
Line 57: info['uuid'] = device.format.uuid
same here
Done
Line 58: if device.uuid:
Line 59: info['devUuid'] = device.uuid
Line 60: info['size'] = device.size
Line 61: deviceInfo.append(info)
Line 55: info['type'] = device.type
Line 56: if device.format.uuid:
Line 57: info['uuid'] = device.format.uuid
Line 58: if device.uuid:
Line 59: info['devUuid'] = device.uuid
same here
Done
Line 60: info['size'] = device.size
Line 61: deviceInfo.append(info)
Line 56: if device.format.uuid:
Line 57: info['uuid'] = device.format.uuid
Line 58: if device.uuid:
Line 59: info['devUuid'] = device.uuid
Line 60: info['size'] = device.size
what is the unit of the size and what is expected unit of the size?
byte, expected unit size is also byte
Line 61: deviceInfo.append(info)
--
To view, visit
http://gerrit.ovirt.org/35028
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I85f520c6f476731cb5982d8256cb701387be87cf
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Nishanth Thomas <nishusemail(a)gmail.com>
Gerrit-Reviewer: Sahina Bose <sabose(a)redhat.com>
Gerrit-Reviewer: Shubhendu Tripathi <shtripat(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes