Darshan N has uploaded a new change for review.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
gluster: Get size information of a gluster volume.
New vdsm gluster verb to get free, used and total size of gluster volume. This verb uses libgfapi to get the statistics related to volume. This patch makes use of ctypes to utilize the libgfapi.
verb: glusterVolumeStatsInfoGet
Output format: {"sizeTotal": LONG as STR, "sizeFree": LONG as STR, "sizeUsed": LONG as STR}
Change-Id: If5d622738ae955eb4002f56fc73adb4f07f0b857 Signed-off-by: darshan n dnarayan@redhat.com --- M client/vdsClientGluster.py M vdsm.spec.in M vdsm/gluster/Makefile.am M vdsm/gluster/__init__.py M vdsm/gluster/api.py M vdsm/gluster/exception.py A vdsm/gluster/gfapi.py M vdsm/gluster/vdsmapi-gluster-schema.json 8 files changed, 236 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/81/28581/1
diff --git a/client/vdsClientGluster.py b/client/vdsClientGluster.py index 9fa58b2..421c853 100644 --- a/client/vdsClientGluster.py +++ b/client/vdsClientGluster.py @@ -422,6 +422,14 @@ pp.pprint(status) return status['status']['code'], status['status']['message']
+ def do_glusterVolumeStatsInfoGet(self, args): + params = self._eqSplit(args) + volumeName = params.get('volumeName', '') + + status = self.s.glusterVolumeStatsInfoGet(volumeName) + pp.pprint(status) + return status['status']['code'], status['status']['message'] +
def getGlusterCmdDict(serv): return \ @@ -718,4 +726,9 @@ ('[taskIds=<task_id1,task_id2,..>]', 'list all or given gluster tasks' )), + 'glusterVolumeStatsInfoGet': ( + serv.do_glusterVolumeStatsInfoGet, + ('volumeName=<volume name>', + 'Returns total, free and used space(bytes) of gluster volume' + )), } diff --git a/vdsm.spec.in b/vdsm.spec.in index df027d0..b93847b 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -1484,6 +1484,7 @@ %doc COPYING %{_datadir}/%{vdsm_name}/gluster/api.py* %{_datadir}/%{vdsm_name}/gluster/vdsmapi-gluster-schema.json +%{_datadir}/%{vdsm_name}/gluster/gfapi.py* %{_datadir}/%{vdsm_name}/gluster/hooks.py* %{_datadir}/%{vdsm_name}/gluster/services.py* %{_datadir}/%{vdsm_name}/gluster/tasks.py* diff --git a/vdsm/gluster/Makefile.am b/vdsm/gluster/Makefile.am index b397e95..3c307aa 100644 --- a/vdsm/gluster/Makefile.am +++ b/vdsm/gluster/Makefile.am @@ -31,6 +31,7 @@ api.py \ cli.py \ exception.py \ + gfapi.py \ hooks.py \ services.py \ tasks.py \ diff --git a/vdsm/gluster/__init__.py b/vdsm/gluster/__init__.py index 0aefb8d..b83efbb 100644 --- a/vdsm/gluster/__init__.py +++ b/vdsm/gluster/__init__.py @@ -22,7 +22,7 @@ import tempfile from functools import wraps
-MODULE_LIST = ('cli', 'hooks', 'services', 'tasks') +MODULE_LIST = ('cli', 'hooks', 'services', 'tasks', 'gfapi')
def makePublic(func): diff --git a/vdsm/gluster/api.py b/vdsm/gluster/api.py index 128ff7d..5cdd599 100644 --- a/vdsm/gluster/api.py +++ b/vdsm/gluster/api.py @@ -306,6 +306,10 @@ status = self.svdsmProxy.glusterTasksList(taskIds) return {'tasks': status}
+ @exportAsVerb + def volumeStatsInfoGet(self, volumeName, options=None): + return self.svdsmProxy.glusterVolumeStatsInfoGet(volumeName) +
def getGlusterMethods(gluster): l = [] diff --git a/vdsm/gluster/exception.py b/vdsm/gluster/exception.py index c7a4dae..26f7443 100644 --- a/vdsm/gluster/exception.py +++ b/vdsm/gluster/exception.py @@ -486,3 +486,28 @@ prefix = "%s: " % (action) self.message = prefix + "Service action is not supported" self.err = [self.message] + + +class GlusterLibgfapiException(GlusterException): + code = 4570 + message = "Gluster Libgfapi Exception" + + +class GlusterVolumeStatsInfoGetFailedException(GlusterLibgfapiException): + code = 4571 + message = "Failed to get Gluster volume Size info" + + +class GlusterVolumeSetVolfileServerFailedException(GlusterLibgfapiException): + code = 4572 + message = "setting volfile server failed" + + +class GlusterVolumeGlfsInitFailedException(GlusterLibgfapiException): + code = 4573 + message = "glfs inint failed" + + +class GlusterVolumeGlfsFinitFailedException(GlusterLibgfapiException): + code = 4574 + message = "glfs finit failed" diff --git a/vdsm/gluster/gfapi.py b/vdsm/gluster/gfapi.py new file mode 100644 index 0000000..5d566cc --- /dev/null +++ b/vdsm/gluster/gfapi.py @@ -0,0 +1,158 @@ +# +# Copyright 2013 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# +import ctypes +from ctypes.util import find_library +import os + +import exception as ge +from . import makePublic + + +GLUSTER_VOL_PROTOCAL = 'tcp' +GLUSTER_VOL_HOST = 'localhost' +GLUSTER_VOL_PORT = 24007 +GLUSTER_VOL_PATH = "/" +_api = None + + +class StatVfsStruct(ctypes.Structure): + _fields_ = [ + ('f_bsize', ctypes.c_ulong), + ('f_frsize', ctypes.c_ulong), + ('f_blocks', ctypes.c_ulong), + ('f_bfree', ctypes.c_ulong), + ('f_bavail', ctypes.c_ulong), + ('f_files', ctypes.c_ulong), + ('f_ffree', ctypes.c_ulong), + ('f_favail', ctypes.c_ulong), + ('f_fsid', ctypes.c_ulong), + ('f_flag', ctypes.c_ulong), + ('f_namemax', ctypes.c_ulong), + ('__f_spare', ctypes.c_int * 6), + ] + + +def _lazyInitGfapi(): + """ + Lazy loading libgfapi + """ + global _api + if not _api: + _api = ctypes.CDLL(find_library("gfapi"), + ctypes.RTLD_GLOBAL, + use_errno=True) + return + + +def _volumeMount(volumeId): + _glfs_new = ctypes.CFUNCTYPE( + ctypes.c_void_p, ctypes.c_char_p)(('glfs_new', _api)) + fs = _glfs_new(volumeId) + + _glfs_set_volfile_server = ctypes.CFUNCTYPE( + ctypes.c_int, + ctypes.c_void_p, + ctypes.c_char_p, + ctypes.c_char_p, + ctypes.c_int)(('glfs_set_volfile_server', _api)) + rc = _glfs_set_volfile_server(fs, + GLUSTER_VOL_PROTOCAL, + GLUSTER_VOL_HOST, + GLUSTER_VOL_PORT) + if rc != 0: + raise ge.GlusterVolumeSetVolfileServerFailedException(rc=rc) + + _glfs_init = ctypes.CFUNCTYPE( + ctypes.c_int, ctypes.c_void_p)(('glfs_init', _api)) + rc = _glfs_init(fs) + if rc != 0: + errMsg = "" + if rc == 1: + errMsg += "Volume is stopped." + elif rc == -1: + errMsg += "Volume not found." + raise ge.GlusterVolumeGlfsInitFailedException(rc=rc, + err=[errMsg]) + return fs + + +def _volumeUmount(fs, volumeId): + _glfs_fini = ctypes.CFUNCTYPE( + ctypes.c_int, ctypes.c_void_p)(('glfs_fini', _api)) + rc = _glfs_fini(fs) + if rc != 0: + raise ge.GlusterVolumeGlfsFinitFailedException(rc=rc) + + +def volumeStatvfs(volumeId): + statvfsdata = StatVfsStruct() + _lazyInitGfapi() + + # Define return type and argtypes of glfs_statvfs + _api.glfs_statvfs.restype = ctypes.c_int + _api.glfs_statvfs.argtypes = [ctypes.c_void_p, + ctypes.c_char_p, + ctypes.POINTER(StatVfsStruct)] + + fs = _volumeMount(volumeId) + _glfs_statvfs = ctypes.CFUNCTYPE(ctypes.c_int, + ctypes.c_void_p, + ctypes.c_char_p, + ctypes.c_void_p)(('glfs_statvfs', _api)) + + rc = _glfs_statvfs(fs, GLUSTER_VOL_PATH, ctypes.byref(statvfsdata)) + if rc != 0: + raise ge.GlusterVolumeStatsInfoGetFailedException(rc=rc, + volId=volumeId) + + _volumeUmount(fs, volumeId) + + # To convert to os.statvfs_result we need to pass tuple/list in + # following order: bsize, frsize, blocks, bfree, bavail, files, + # ffree, favail, flag, namemax + return os.statvfs_result((statvfsdata.f_bsize, + statvfsdata.f_frsize, + statvfsdata.f_blocks, + statvfsdata.f_bfree, + statvfsdata.f_bavail, + statvfsdata.f_files, + statvfsdata.f_ffree, + statvfsdata.f_favail, + statvfsdata.f_flag, + statvfsdata.f_namemax)) + + +@makePublic +def volumeStatsInfoGet(volumeName): + + data = volumeStatvfs(volumeName) + + # f_blocks = Total number of blocks + # f_bfree = Total number of blocks free + # f_bavail = Total number of blocks available for non root user + # total blocks available = f_blocks - (f_bfree - f_bavail) + + total = (data.f_blocks - (data.f_bfree - data.f_bavail)) * data.f_bsize + free = data.f_bavail * data.f_bsize + used = total - free + + return {'sizeTotal': str(total), + 'sizeFree': str(free), + 'sizeUsed': str(used)} diff --git a/vdsm/gluster/vdsmapi-gluster-schema.json b/vdsm/gluster/vdsmapi-gluster-schema.json index 4e80679..4ddd182 100644 --- a/vdsm/gluster/vdsmapi-gluster-schema.json +++ b/vdsm/gluster/vdsmapi-gluster-schema.json @@ -1205,3 +1205,36 @@ ## {'command': {'class': 'GlusterHost', 'name': 'list'}, 'returns': ['HostList']} + +## +# @GlusterVolumeStatsInfo: +# +# Gluster Volumes disk usage statistics +# +# @sizeTotal: Total space available in bytes +# +# @sizeFree: Free space available in bytes +# +# @sizeUsed: Used space in bytes +# +# Since: 4.15.0 +## +{'type': 'GlusterVolumeStatsInfo', + 'data': {'sizeTotal': 'str', 'sizeFree': 'str', 'sizeUsed': 'str'}} + + +## +# @GlusterVolume.statsInfoGet: +# +# Get the size info of gluster volume +# +# @volumeName: Gluster volume name +# +# Returns: +# Stats info of GlusterFS volume +# +# Since: 4.15.0 +## +{'command': {'class': 'GlusterVolume', 'name': 'statsInfoGet'}, + 'data': {'volumeName': 'str'}, + 'returns': 'GlusterVolumeStatsInfo'}
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 1: Code-Review-1 Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9022/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/717/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9806/ : UNSTABLE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9962/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9023/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/718/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9807/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9963/ : SUCCESS
Darshan N has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 2: Verified+1
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
Darshan N has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 2:
(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?
Not as of now, can be enhanced in future. 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
Done 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])
- This nested if can be improved like
Done 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
Done 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()
Done 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
Done 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()
Done Line 144: Line 145: data = volumeStatvfs(volumeName) Line 146: Line 147: # f_blocks = Total number of blocks
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9051/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/723/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9835/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/9991/ : SUCCESS
Darshan N has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 3: Verified+1
Timothy Asir has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 3:
(3 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 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/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. Line 50: Line 51: Line 52: def _lazyInitGfapi(): Line 53: """
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 volumeId not found or invalid protocol? are we catching those exceptions? 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))
Bala.FA has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 3:
(6 comments)
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 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
http://gerrit.ovirt.org/#/c/28581/3/vdsm/gluster/gfapi.py File vdsm/gluster/gfapi.py:
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.GlfsSetVolfileServerException() ie its too verbose.
Its up to you to decide. 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] ? 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] ? 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? Line 95: Line 96: Line 97: def glfsFini(fs, volumeId): Line 98: _glfs_fini = ctypes.CFUNCTYPE(
Bala.FA has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 3: Code-Review-1
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))
Darshan N has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 4: Verified+1
Timothy Asir has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 4: Code-Review+1
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 4:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9063/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/726/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9847/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10003/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/4929/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3086/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 4: Code-Review-1 Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9078/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/727/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9862/ : UNSTABLE
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10018/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/4944/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3101/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 4: -Code-Review -Verified
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9133/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/732/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9918/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10073/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5000/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3157/ : SUCCESS
Bala.FA has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 4: Code-Review+1
Bala.FA has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 5:
(1 comment)
http://gerrit.ovirt.org/#/c/28581/5/vdsm/gluster/api.py File vdsm/gluster/api.py:
Line 203: replicaCount=0, options=None): Line 204: self.svdsmProxy.glusterVolumeRemoveBrickForce(volumeName, brickList, Line 205: replicaCount) Line 206: Line 207: def computeVolumeStats(self, data): May be a private method by _computeVolumeStats()? Line 208: total = (data.f_blocks - (data.f_bfree - data.f_bavail)) * data.f_bsize Line 209: free = data.f_bavail * data.f_bsize Line 210: used = total - free Line 211: return {'sizeTotal': str(total),
Darshan N has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 5:
(1 comment)
http://gerrit.ovirt.org/#/c/28581/5/vdsm/gluster/api.py File vdsm/gluster/api.py:
Line 203: replicaCount=0, options=None): Line 204: self.svdsmProxy.glusterVolumeRemoveBrickForce(volumeName, brickList, Line 205: replicaCount) Line 206: Line 207: def computeVolumeStats(self, data):
May be a private method by _computeVolumeStats()?
Done Line 208: total = (data.f_blocks - (data.f_bfree - data.f_bavail)) * data.f_bsize Line 209: free = data.f_bavail * data.f_bsize Line 210: used = total - free Line 211: return {'sizeTotal': str(total),
Darshan N has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 6: Verified+1
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 6:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9215/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/743/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/9999/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10154/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5081/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3238/ : SUCCESS
Antoni Segura Puimedon has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 5:
(6 comments)
http://gerrit.ovirt.org/#/c/28581/5/vdsm/gluster/gfapi.py File vdsm/gluster/gfapi.py:
Line 56: global _api Line 57: if not _api: Line 58: _api = ctypes.CDLL(find_library("gfapi"), Line 59: ctypes.RTLD_GLOBAL, Line 60: use_errno=True) I think _lib would be a better name for it. I would either: * do what I wrote in the comments below, of just putting the ctypes at the end of the file and not lazy (happening on module loading time). * do as above, but lazily loading them into the module namespace. Line 61: return Line 62: Line 63: Line 64: def glfsInit(volumeId, host, port, protocol):
Line 63: Line 64: def glfsInit(volumeId, host, port, protocol): 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) You'll be wrapping the C method on each volumeStatvfs. Please, consider moving this definition to the module level. Line 68: Line 69: _glfs_set_volfile_server = ctypes.CFUNCTYPE( Line 70: ctypes.c_int, Line 71: ctypes.c_void_p,
Line 70: ctypes.c_int, Line 71: ctypes.c_void_p, Line 72: ctypes.c_char_p, Line 73: ctypes.c_char_p, Line 74: ctypes.c_int)(('glfs_set_volfile_server', _api)) You'll be wrapping the C method on each volumeStatvfs. Please, consider moving this definition to the module level. Line 75: rc = _glfs_set_volfile_server(fs, Line 76: protocol, Line 77: host, Line 78: port)
Line 81: rc=rc, err=["setting volfile server failed"] Line 82: ) Line 83: Line 84: _glfs_init = ctypes.CFUNCTYPE( Line 85: ctypes.c_int, ctypes.c_void_p)(('glfs_init', _api)) You'll be wrapping the C method on each volumeStatvfs. Please, consider moving this definition to the module level. Line 86: rc = _glfs_init(fs) Line 87: if rc == 0: Line 88: return fs Line 89: elif rc == 1:
Line 99: Line 100: Line 101: def glfsFini(fs, volumeId): Line 102: _glfs_fini = ctypes.CFUNCTYPE( Line 103: ctypes.c_int, ctypes.c_void_p)(('glfs_fini', _api)) You'll be wrapping the C method on each volumeStatvfs. Please, consider moving this definition to the module level. Line 104: rc = _glfs_fini(fs) Line 105: if rc != 0: Line 106: raise ge.GlfsFiniException(rc=rc) Line 107:
Line 118: # Define return type and argtypes of glfs_statvfs 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)) You'll be wrapping the C method on each volumeStatvfs. Please, consider moving this definition to the module level. Line 123: Line 124: rc = _glfs_statvfs(fs, GLUSTER_VOL_PATH, ctypes.byref(statvfsdata)) Line 125: if rc != 0: Line 126: raise ge.GlfsStatvfsException(rc=rc)
Antoni Segura Puimedon has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 6: Code-Review-1
Please, see my comments to patchset 5.
Darshan N has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 5:
(6 comments)
http://gerrit.ovirt.org/#/c/28581/5/vdsm/gluster/gfapi.py File vdsm/gluster/gfapi.py:
Line 56: global _api Line 57: if not _api: Line 58: _api = ctypes.CDLL(find_library("gfapi"), Line 59: ctypes.RTLD_GLOBAL, Line 60: use_errno=True)
I think _lib would be a better name for it. I would either:
Done Line 61: return Line 62: Line 63: Line 64: def glfsInit(volumeId, host, port, protocol):
Line 63: Line 64: def glfsInit(volumeId, host, port, protocol): 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)
You'll be wrapping the C method on each volumeStatvfs. Please, consider mov
Done Line 68: Line 69: _glfs_set_volfile_server = ctypes.CFUNCTYPE( Line 70: ctypes.c_int, Line 71: ctypes.c_void_p,
Line 70: ctypes.c_int, Line 71: ctypes.c_void_p, Line 72: ctypes.c_char_p, Line 73: ctypes.c_char_p, Line 74: ctypes.c_int)(('glfs_set_volfile_server', _api))
You'll be wrapping the C method on each volumeStatvfs. Please, consider mov
Done Line 75: rc = _glfs_set_volfile_server(fs, Line 76: protocol, Line 77: host, Line 78: port)
Line 81: rc=rc, err=["setting volfile server failed"] Line 82: ) Line 83: Line 84: _glfs_init = ctypes.CFUNCTYPE( Line 85: ctypes.c_int, ctypes.c_void_p)(('glfs_init', _api))
You'll be wrapping the C method on each volumeStatvfs. Please, consider mov
Done Line 86: rc = _glfs_init(fs) Line 87: if rc == 0: Line 88: return fs Line 89: elif rc == 1:
Line 99: Line 100: Line 101: def glfsFini(fs, volumeId): Line 102: _glfs_fini = ctypes.CFUNCTYPE( Line 103: ctypes.c_int, ctypes.c_void_p)(('glfs_fini', _api))
You'll be wrapping the C method on each volumeStatvfs. Please, consider mov
Done Line 104: rc = _glfs_fini(fs) Line 105: if rc != 0: Line 106: raise ge.GlfsFiniException(rc=rc) Line 107:
Line 118: # Define return type and argtypes of glfs_statvfs 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))
You'll be wrapping the C method on each volumeStatvfs. Please, consider mov
Done Line 123: Line 124: rc = _glfs_statvfs(fs, GLUSTER_VOL_PATH, ctypes.byref(statvfsdata)) Line 125: if rc != 0: Line 126: raise ge.GlfsStatvfsException(rc=rc)
Darshan N has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 7: Verified+1
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 7:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/9256/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/745/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10040/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit/10195/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5122/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/3279/ : SUCCESS
Antoni Segura Puimedon has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 7: Code-Review+1
I've concentrated mostly in the ctypes interface and it looks good.
Bala.FA has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 7: Code-Review+1
Dan Kenigsberg has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 7: Code-Review+2
Raising score
Dan Kenigsberg has submitted this change and it was merged.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
gluster: Get size information of a gluster volume.
New vdsm gluster verb to get free, used and total size of gluster volume. This verb uses libgfapi to get the statistics related to volume. This patch makes use of ctypes to utilize the libgfapi. This patch also enhances glusterVolumeStatus verb to return volume size info as well when status option is set to detail.
verb: glusterVolumeStatsInfoGet
Output format: {"sizeTotal": LONG as STR, "sizeFree": LONG as STR, "sizeUsed": LONG as STR}
Change-Id: If5d622738ae955eb4002f56fc73adb4f07f0b857 Signed-off-by: darshan n dnarayan@redhat.com Reviewed-on: http://gerrit.ovirt.org/28581 Reviewed-by: Antoni Segura Puimedon asegurap@redhat.com Reviewed-by: Bala.FA barumuga@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com --- M client/vdsClientGluster.py M vdsm.spec.in M vdsm/gluster/Makefile.am M vdsm/gluster/__init__.py M vdsm/gluster/api.py M vdsm/gluster/exception.py A vdsm/gluster/gfapi.py M vdsm/gluster/vdsmapi-gluster-schema.json 8 files changed, 221 insertions(+), 1 deletion(-)
Approvals: Bala.FA: Looks good to me, but someone else must approve Antoni Segura Puimedon: Looks good to me, but someone else must approve Darshan N: Verified Dan Kenigsberg: Looks good to me, approved
oVirt Jenkins CI Server has posted comments on this change.
Change subject: gluster: Get size information of a gluster volume. ......................................................................
Patch Set 8:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/751/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged/1451/ : SUCCESS
vdsm-patches@lists.fedorahosted.org