Change in vdsm[master]: Gluster: Add gluster volume top functionality

avishwan at redhat.com avishwan at redhat.com
Mon Feb 25 11:55:04 UTC 2013


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 at redhat.com>
Gerrit-Reviewer: Aravinda VK <avishwan at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list