Aravinda VK has posted comments on this change.
Change subject: Gluster: Add gluster volume top functionality
......................................................................
Patch Set 10: (5 inline comments)
....................................................
Commit Message
Line 24: [{'brick': BRICK-NAME,
Line 25: 'currentOpen': int,
Line 26: 'files': [{FILE-NAME: {'count': int}}, ...],
Line 27: 'maxOpen': int,
Line 28: 'maxOpenTime': TIME}, ...]
We can have common format,
[{'nfs': Boolean, True if nfs else false
'name': SERVER_NAME if nfs else BRICK_NAME
'currentOpen': int,
'files': [{FILE-NAME: {'count': int}}, ...],
'maxOpen': int,
'maxOpenTime': TIME}, ...]
OR
[{'type': 'nfs'
'name': SERVER_NAME if nfs else BRICK_NAME
'currentOpen': int,
'files': [{FILE-NAME: {'count': int}}, ...],
'maxOpen': int,
'maxOpenTime': TIME}, ...]
Line 29:
Line 30: * glusterVolumeTopRead
Line 31: output structure:
Line 32: When nfs is False:
....................................................
File vdsm/gluster/cli.py
Line 416: name = hostName
Line 417: if nfs:
Line 418: brick['nfs'] = name
Line 419: else:
Line 420: brick['brick'] = name
can be simplified.
brick['nfs' if nfs else 'brick'] = name
Line 421:
Line 422: if mode == 'topopen':
Line 423: brick['currentOpen'] = tag.find('currentOpen').text
Line 424: brick['maxOpen'] = tag.find('maxOpen').text
Line 436:
Line 437: if mode == 'dir':
Line 438: brick['dirs'] = fileList
Line 439: else:
Line 440: brick['files'] = fileList
Can be simplified.
brick['dirs' if mode == 'dir' else 'files'] = fileList
Line 441: bricks.append(brick)
Line 442: return bricks
Line 443:
Line 444:
Line 985: command += ["brick", brickName]
Line 986: if nfs:
Line 987: command += ["nfs"]
Line 988: if listCount:
Line 989: command += ["list-cnt", "%s" % listCount]
Above command lines can be moved to separate function. Something like
def _getGlusterVolTopCmd(action, brickName=None, nfs=False, listCount=0, blockSize=0,
count=0):
command = _getGlusterVolCmd() + ["top", volumeName, action]
command += ["brick", brickName] if brickName else []
command += ["nfs"] if nfs else []
command += ["list-cnt", "%s" % listCount] if listCount else []
command += ["bs", "%s" % blockSize] if blockSize else []
command += ["count", "%s" % count] if count else []
return command
This function can be called from different verbs.
Line 990: try:
Line 991: xmltree = _execGlusterXml(command)
Line 992: except ge.GlusterCmdFailedException, e:
Line 993: raise ge.GlusterVolumeTopOpenFailedException(rc=e.rc, err=e.err)
Line 994: try:
Line 995: return _parseVolumeTopOpen(xmltree, nfs)
Line 996: except (etree.ParseError, AttributeError, ValueError):
Line 997: raise ge.GlusterXmlErrorException(err=[etree.tostring(xmltree)])
Line 998:
We can move second exception into _parseVolumeTopGeneric function, so that try catch code
in each verb can be reduced.
Btw, two try-except statement can be reduced to one.
Ex:
try:
xmltree = _execGlusterXml(command)
return _parseVolumeTopOpen(xmltree, nfs)
except ge.GlusterCmdFailedException, e:
raise ge.GlusterVolumeTopOpenFailedException(rc=e.rc, err=e.err)
except (etree.ParseError, AttributeError, ValueError):
raise ge.GlusterXmlErrorException(err=[etree.tostring(xmltree)])
Line 999:
Line 1000: @exportToSuperVdsm
Line 1001: def volumeTopRead(volumeName, brickName=None, nfs=False, listCount=0):
Line 1002: """
--
To view, visit
http://gerrit.ovirt.org/7844
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I96486363a9acb7472014a67fcd2d5185d4f3c428
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: Aravinda VK <avishwan(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server