Change in vdsm[master]: gluster: Get size information of a gluster volume.

dnarayan at redhat.com dnarayan at redhat.com
Wed Jun 11 12:48:47 UTC 2014


Darshan N has posted comments on this change.

Change subject: gluster: Get size information of a gluster volume.
......................................................................


Patch Set 3:

(9 comments)

http://gerrit.ovirt.org/#/c/28581/3/vdsm/gluster/api.py
File vdsm/gluster/api.py:

Line 313:         # f_blocks = Total number of blocks
Line 314:         # f_bfree = Total number of blocks free
Line 315:         # f_bavail = Total number of blocks available for non root user
Line 316:         # total blocks available = f_blocks - (f_bfree - f_bavail)
Line 317: 
> remove the above lines
Done
Line 318:         total = (data.f_blocks - (data.f_bfree - data.f_bavail)) * data.f_bsize
Line 319:         free = data.f_bavail * data.f_bsize
Line 320:         used = total - free
Line 321: 


http://gerrit.ovirt.org/#/c/28581/3/vdsm/gluster/exception.py
File vdsm/gluster/exception.py:

Line 504: 
Line 505: 
Line 506: class GlfsInitException(GlusterLibgfapiException):
Line 507:     code = 4573
Line 508:     message = "glfs inint failed"
> please fix the text
Done
Line 509: 
Line 510: 
Line 511: class GlfsFiniException(GlusterLibgfapiException):
Line 512:     code = 4574


Line 509: 
Line 510: 
Line 511: class GlfsFiniException(GlusterLibgfapiException):
Line 512:     code = 4574
Line 513:     message = "glfs finit failed"
> please fix the text
Done


http://gerrit.ovirt.org/#/c/28581/3/vdsm/gluster/gfapi.py
File vdsm/gluster/gfapi.py:

Line 45:         ('f_fsid', ctypes.c_ulong),
Line 46:         ('f_flag', ctypes.c_ulong),
Line 47:         ('f_namemax', ctypes.c_ulong),
Line 48:         ('__f_spare', ctypes.c_int * 6),
Line 49:     ]
> Why dont you give some easy names here to avoid _ in variables.
These names are the names of members of standard statvfs structure.. So wanted to retain them .
Line 50: 
Line 51: 
Line 52: def _lazyInitGfapi():
Line 53:     """


Line 76:                                   protocol,
Line 77:                                   host,
Line 78:                                   port)
Line 79:     if rc != 0:
Line 80:         raise ge.GlfsSetVolfileServerException(rc=rc)
> I feel to use ge.GlfsInitException() than having additional ge.GlfsSetVolfi
Done
Line 81: 
Line 82:     _glfs_init = ctypes.CFUNCTYPE(
Line 83:         ctypes.c_int, ctypes.c_void_p)(('glfs_init', _api))
Line 84:     rc = _glfs_init(fs)


Line 85:     if rc == 0:
Line 86:         return fs
Line 87:     elif rc == 1:
Line 88:         errMsg = "Volume is stopped."
Line 89:         raise ge.GlfsInitException(rc=rc, err=[errMsg])
> How about err=["Volume %s is not running" % volumeName] ?
Done
Line 90:     elif rc == -1:
Line 91:         errMsg = "Volume not found."
Line 92:         raise ge.GlfsInitException(rc=rc, err=[errMsg])
Line 93:     else:


Line 88:         errMsg = "Volume is stopped."
Line 89:         raise ge.GlfsInitException(rc=rc, err=[errMsg])
Line 90:     elif rc == -1:
Line 91:         errMsg = "Volume not found."
Line 92:         raise ge.GlfsInitException(rc=rc, err=[errMsg])
> How about err=["Volume %s is not found" % volumeName] ?
Done
Line 93:     else:
Line 94:         raise ge.GlfsInitException(rc=rc)
Line 95: 
Line 96: 


Line 90:     elif rc == -1:
Line 91:         errMsg = "Volume not found."
Line 92:         raise ge.GlfsInitException(rc=rc, err=[errMsg])
Line 93:     else:
Line 94:         raise ge.GlfsInitException(rc=rc)
> Its required give some err. may be unknown error?
Done
Line 95: 
Line 96: 
Line 97: def glfsFini(fs, volumeId):
Line 98:     _glfs_fini = ctypes.CFUNCTYPE(


Line 114:     _api.glfs_statvfs.argtypes = [ctypes.c_void_p,
Line 115:                                   ctypes.c_char_p,
Line 116:                                   ctypes.POINTER(StatVfsStruct)]
Line 117: 
Line 118:     fs = glfsInit(volumeId, host, port, protocol)
> do we really need to pass the host name (localhost) here and what if volume
yes, _glfs_set_volfile_server() needs hostname as on of its arguments. The default host name is localhost, If some one wants to consume volumeStatvfs() he can provide the hostname that he wants. 

if volume id not found or invalid protocol...etc  the function _glfs_set_volfile_server() or _glfs_init() will return a non zero return value, for which an exception is rised.
Line 119:     _glfs_statvfs = ctypes.CFUNCTYPE(ctypes.c_int,
Line 120:                                      ctypes.c_void_p,
Line 121:                                      ctypes.c_char_p,
Line 122:                                      ctypes.c_void_p)(('glfs_statvfs', _api))


-- 
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: 3
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