Change in vdsm[master]: gluster: Temporary fix for supervdsm memory leak.

dnarayan at redhat.com dnarayan at redhat.com
Thu Sep 25 13:01:55 UTC 2014


Darshan N has posted comments on this change.

Change subject: gluster: Temporary fix for supervdsm memory leak.
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.ovirt.org/#/c/33312/4/vdsm/gluster/gfapi.py
File vdsm/gluster/gfapi.py:

Line 21: from ctypes.util import find_library
Line 22: import os
Line 23: 
Line 24: import exception as ge
Line 25: from __init__ import makePublic
> Please add a comment here; it's very annoying.
Done
Line 26: 
Line 27: 
Line 28: GLUSTER_VOL_PROTOCAL = 'tcp'
Line 29: GLUSTER_VOL_HOST = 'localhost'


Line 160:                '-p', str(port), '-H', host, '-t', protocol]
Line 161:     rc, out, err = utils.execCmd(command, raw=True)
Line 162:     if rc != 0:
Line 163:         raise ge.GlfsStatvfsException(rc, out, err)
Line 164:     res = json.loads(out.replace("'", '"'))
> Why is this replace() needed? It is certainly not precise in the general ca
Done
Line 165:     return os.statvfs_result((res['f_bsize'],
Line 166:                               res['f_frsize'],
Line 167:                               res['f_blocks'],
Line 168:                               res['f_bfree'],


Line 196:     try:
Line 197:         res = volumeStatvfsGet(args.volume, args.host,
Line 198:                                int(args.port), args.protocol)
Line 199:     except ge.GlusterLibgfapiException:
Line 200:         sys.stderr.write("Failed to retrive volume stats")
> Why is the special handling of GlusterLibgfapiException ? ANY exception tha
Done
Line 201:         sys.exit(1)
Line 202:     print json.dumps({'f_blocks': res.f_blocks, 'f_bfree': res.f_bfree,
Line 203:                       'f_bsize': res.f_bsize, 'f_frsize': res.f_frsize,
Line 204:                       'f_bavail': res.f_bavail, 'f_files': res.f_files,


Line 198:                                int(args.port), args.protocol)
Line 199:     except ge.GlusterLibgfapiException:
Line 200:         sys.stderr.write("Failed to retrive volume stats")
Line 201:         sys.exit(1)
Line 202:     print json.dumps({'f_blocks': res.f_blocks, 'f_bfree': res.f_bfree,
> It would be more succinct to have
looks like statvfs_result object does not have _fields or any similar attribute to get the field names.
Line 203:                       'f_bsize': res.f_bsize, 'f_frsize': res.f_frsize,
Line 204:                       'f_bavail': res.f_bavail, 'f_files': res.f_files,
Line 205:                       'f_ffree': res.f_ffree, 'f_favail': res.f_favail,
Line 206:                       'f_flag': res.f_flag, 'f_namemax': res.f_namemax},


-- 
To view, visit http://gerrit.ovirt.org/33312
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7079426178ce47008d9a3b83635afce98536ca34
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Bala.FA <barumuga at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Darshan N <dnarayan at redhat.com>
Gerrit-Reviewer: Sahina Bose <sabose 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