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)