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

tjeyasin at redhat.com tjeyasin at redhat.com
Thu Feb 28 10:21:54 UTC 2013


Timothy Asir has posted comments on this change.

Change subject: Gluster: Add gluster volume top functionality
......................................................................


Patch Set 10: (3 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}, ...]
done
Line 29: 
Line 30: * glusterVolumeTopRead
Line 31:   output structure:
Line 32:     When nfs is False:


....................................................
File vdsm/gluster/cli.py
Line 985:         command += ["brick", brickName]
Line 986:     if nfs:
Line 987:         command += ["nfs"]
Line 988:     if listCount:
Line 989:         command += ["list-cnt", "%s" % listCount]
done
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: 
Yes I agree, this is valid when we have some common behaviour (under same process) in every statements. But here, the function _execGlusterXml internally calls an another (process) script (gluster cli) and combined the error/output with the usual python exception details. For this reason it's been placed inside the function itself. 

I will move the second exception into _parseVolumeTopGeneric function and send a new patch.
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