Change in vdsm[master]: gluster: Get size information of a gluster volume.
barumuga at redhat.com
barumuga at redhat.com
Wed Jun 11 09:56:07 UTC 2014
Bala.FA has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume.
......................................................................
Patch Set 2: Code-Review-1
(7 comments)
http://gerrit.ovirt.org/#/c/28581/2/vdsm/gluster/api.py
File vdsm/gluster/api.py:
Line 307: return {'tasks': status}
Line 308:
Line 309: @exportAsVerb
Line 310: def volumeStatsInfoGet(self, volumeName, options=None):
Line 311: return self.svdsmProxy.glusterVolumeStatsInfoGet(volumeName)
Any plans to give inode used/free/total too?
Line 312:
Line 313:
Line 314: def getGlusterMethods(gluster):
Line 315: l = []
http://gerrit.ovirt.org/#/c/28581/2/vdsm/gluster/gfapi.py
File vdsm/gluster/gfapi.py:
Line 60: use_errno=True)
Line 61: return
Line 62:
Line 63:
Line 64: def _volumeMount(volumeId):
you could have this function name as glfsInit() as per libfgapi
Line 65: _glfs_new = ctypes.CFUNCTYPE(
Line 66: ctypes.c_void_p, ctypes.c_char_p)(('glfs_new', _api))
Line 67: fs = _glfs_new(volumeId)
Line 68:
Line 88: errMsg += "Volume is stopped."
Line 89: elif rc == -1:
Line 90: errMsg += "Volume not found."
Line 91: raise ge.GlusterVolumeGlfsInitFailedException(rc=rc,
Line 92: err=[errMsg])
1. This nested if can be improved like
if rc == 0:
return fs
elif rc == 1:
raise ge.expection(...)
elif rc == -1:
raise ge.expection(...)
else:
raise ge.expection(...) # this is unknown
2. I feel we could have one exception for this function like
ge.GlfsInitException()
Line 93: return fs
Line 94:
Line 95:
Line 96: def _volumeUmount(fs, volumeId):
Line 92: err=[errMsg])
Line 93: return fs
Line 94:
Line 95:
Line 96: def _volumeUmount(fs, volumeId):
you could have this function name as glfsFini() as per libfgapi
Line 97: _glfs_fini = ctypes.CFUNCTYPE(
Line 98: ctypes.c_int, ctypes.c_void_p)(('glfs_fini', _api))
Line 99: rc = _glfs_fini(fs)
Line 100: if rc != 0:
Line 97: _glfs_fini = ctypes.CFUNCTYPE(
Line 98: ctypes.c_int, ctypes.c_void_p)(('glfs_fini', _api))
Line 99: rc = _glfs_fini(fs)
Line 100: if rc != 0:
Line 101: raise ge.GlusterVolumeGlfsFinitFailedException(rc=rc)
I feel we could have better name of exception like ge.GlfsFiniException()
Line 102:
Line 103:
Line 104: def volumeStatvfs(volumeId):
Line 105: statvfsdata = StatVfsStruct()
Line 100: if rc != 0:
Line 101: raise ge.GlusterVolumeGlfsFinitFailedException(rc=rc)
Line 102:
Line 103:
Line 104: def volumeStatvfs(volumeId):
Please accept optional arg host, port and protocol. This will wide up the usage.
Line 105: statvfsdata = StatVfsStruct()
Line 106: _lazyInitGfapi()
Line 107:
Line 108: # Define return type and argtypes of glfs_statvfs
Line 139: statvfsdata.f_namemax))
Line 140:
Line 141:
Line 142: @makePublic
Line 143: def volumeStatsInfoGet(volumeName):
Please move this function logic to api.volumeStatInfoGet()
Line 144:
Line 145: data = volumeStatvfs(volumeName)
Line 146:
Line 147: # f_blocks = Total number of blocks
--
To view, visit http://gerrit.ovirt.org/28581
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If5d622738ae955eb4002f56fc73adb4f07f0b857
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Aravinda VK <avishwan at redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga at redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Timothy Asir <tjeyasin at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list