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

2014-09-29 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 7:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged_test_debug/234/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5867/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/4027/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el7-x86_64_merged/37/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc20-x86_64_merged/33/ : 
FAILURE

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el6-x86_64_merged/39/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc21-x86_64_merged/13/ : 
FAILURE

-- 
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: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-29 Thread danken
Dan Kenigsberg has submitted this change and it was merged.

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


gluster: Temporary fix for supervdsm memory leak.

 Supervdsm memory leak is caused by memory leak in libgfapi
(BZ:1093594) which is used by verb glusterVolumeStatsInfoGet to
retrive the volume statistics. The libgfapi method glfs_fini()
is not cleaning up the memory, so everytime the verb is invoked
memory gets added up(~20-30MiB). This verb is invoked by engine
every 5 minutes for volume capacity feature. So supervdsm memory
usage shoots up when gluster is enabled.

This patch provides a temporary fix until libgfapi issue is
resolved. In this patch libgfapi method calls are executed as
script using execCmd(), by this libgfapi invocation is not part
of supervdsm process.

Change-Id: I7079426178ce47008d9a3b83635afce98536ca34
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1142647
Signed-off-by: ndarshan 
Reviewed-on: http://gerrit.ovirt.org/33312
Reviewed-by: Antoni Segura Puimedon 
Reviewed-by: Dan Kenigsberg 
---
M vdsm/gluster/gfapi.py
1 file changed, 78 insertions(+), 3 deletions(-)

Approvals:
  Antoni Segura Puimedon: Looks good to me, but someone else must approve
  Darshan N: Verified
  Dan Kenigsberg: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I7079426178ce47008d9a3b83635afce98536ca34
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-29 Thread danken
Dan Kenigsberg has posted comments on this change.

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


Patch Set 6: Code-Review+2

-- 
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-29 Thread asegurap
Antoni Segura Puimedon has posted comments on this change.

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


Patch Set 6: Code-Review+1

The imports look good now

-- 
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-26 Thread dnarayan
Darshan N has posted comments on this change.

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


Patch Set 3:

(4 comments)

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

Line 26: 
Line 27: import exception as ge
Line 28: from vdsm import constants
Line 29: from vdsm import utils
Line 30: from __init__ import makePublic
> Please move extra imports before workaround function
Done
Line 31: 
Line 32: 
Line 33: GLUSTER_VOL_PROTOCAL = 'tcp'
Line 34: GLUSTER_VOL_HOST = 'localhost'


Line 147: # This can be reverted back once the memory leak issue
Line 148: # is fixed in libgfapi.
Line 149: @makePublic
Line 150: def volumeStatvfs(volumeName):
Line 151: script = constants.P_VDSM_GLUSTER + "gfapi.pyc"
> instead of script have:
Done
Line 152: command = [constants.EXT_PYTHON, script, "-v", volumeName]
Line 153: rc, out, err = utils.execCmd(command)
Line 154: if rc != 0:
Line 155: raise ge.GlfsStatvfsException(rc, out, err)


Line 148: # is fixed in libgfapi.
Line 149: @makePublic
Line 150: def volumeStatvfs(volumeName):
Line 151: script = constants.P_VDSM_GLUSTER + "gfapi.pyc"
Line 152: command = [constants.EXT_PYTHON, script, "-v", volumeName]
> Here:
Done
Line 153: rc, out, err = utils.execCmd(command)
Line 154: if rc != 0:
Line 155: raise ge.GlfsStatvfsException(rc, out, err)
Line 156: json_acceptable_string = out[0].replace("'", "\"")


Line 183: args = parser.parse_args()
Line 184: return args
Line 185: 
Line 186: 
Line 187: if __name__ == '__main__':
> This will get executed by the python interpreter when putting -m and pointi
Done
Line 188: args = parse_cmdargs()
Line 189: try:
Line 190: res = volumeStatvfsGet(args.volume, args.host,
Line 191:args.port, args.protocol)


-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-26 Thread dnarayan
Darshan N has posted comments on this change.

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


Patch Set 6: Verified+1

-- 
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-26 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 6:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11660/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12604/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12449/ : SUCCESS

-- 
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 5:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11642/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12586/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12431/ : SUCCESS

-- 
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread asegurap
Antoni Segura Puimedon has posted comments on this change.

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


Patch Set 3:

(4 comments)

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

Line 26: 
Line 27: import exception as ge
Line 28: from vdsm import constants
Line 29: from vdsm import utils
Line 30: from __init__ import makePublic
> It's sad that we have to use this
This is not really necessary. here continue to have 

from . import makePublic
Line 31: 
Line 32: 
Line 33: GLUSTER_VOL_PROTOCAL = 'tcp'
Line 34: GLUSTER_VOL_HOST = 'localhost'


Line 147: # This can be reverted back once the memory leak issue
Line 148: # is fixed in libgfapi.
Line 149: @makePublic
Line 150: def volumeStatvfs(volumeName):
Line 151: script = constants.P_VDSM_GLUSTER + "gfapi.pyc"
instead of script have:

module = gluster.gfapi
Line 152: command = [constants.EXT_PYTHON, script, "-v", volumeName]
Line 153: rc, out, err = utils.execCmd(command)
Line 154: if rc != 0:
Line 155: raise ge.GlfsStatvfsException(rc, out, err)


Line 148: # is fixed in libgfapi.
Line 149: @makePublic
Line 150: def volumeStatvfs(volumeName):
Line 151: script = constants.P_VDSM_GLUSTER + "gfapi.pyc"
Line 152: command = [constants.EXT_PYTHON, script, "-v", volumeName]
Here:

command = [constants.EXT_PYTHON, '-m', module, '-v', volumeName]
Line 153: rc, out, err = utils.execCmd(command)
Line 154: if rc != 0:
Line 155: raise ge.GlfsStatvfsException(rc, out, err)
Line 156: json_acceptable_string = out[0].replace("'", "\"")


Line 183: args = parser.parse_args()
Line 184: return args
Line 185: 
Line 186: 
Line 187: if __name__ == '__main__':
This will get executed by the python interpreter when putting -m and pointing 
it to this module of the gluster package (the package must be in the path, of 
course)
Line 188: args = parse_cmdargs()
Line 189: try:
Line 190: res = volumeStatvfsGet(args.volume, args.host,
Line 191:args.port, args.protocol)


-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread asegurap
Antoni Segura Puimedon has posted comments on this change.

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


Patch Set 5: Code-Review-1

See my comments to patch set 3

-- 
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Antoni Segura Puimedon 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread dnarayan
Darshan N has posted comments on this change.

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


Patch Set 5: Verified+1

-- 
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread dnarayan
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 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread danken
Dan Kenigsberg has posted comments on this change.

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


Patch Set 4: Code-Review-1

(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.
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 case.
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 that 
happens during volumeStatvfsGet means just that. I think we can throw away the 
whole try-block.
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

  dict(zip(res._fields, res))

And there's no need to use "print". json.dump() is enough.
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 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread barumuga
Bala.FA has posted comments on this change.

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


Patch Set 4: Code-Review+1

-- 
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 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 4:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11636/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12580/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12425/ : SUCCESS

-- 
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 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread danken
Dan Kenigsberg has posted comments on this change.

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


Patch Set 3: Code-Review-1

(4 comments)

http://gerrit.ovirt.org/#/c/33312/3/lib/vdsm/constants.py.in
File lib/vdsm/constants.py.in:

Line 72: # Path definitions
Line 73: #
Line 74: P_LIBVIRT_VMCHANNELS = '/var/lib/libvirt/qemu/channels/'
Line 75: P_VDSM = '@VDSMDIR@/'
Line 76: P_VDSM_GLUSTER = '@VDSMDIR@/gluster/'
> I feel its better to be moved to gfapi.py
right. actually, os.path.realpath(__file__) may be used there.
Line 77: P_VDSM_RPC = '@VDSMDIR@/rpc/'
Line 78: P_VDSM_HOOKS = '@HOOKSDIR@/'
Line 79: P_VDSM_LIB = '@VDSMLIBDIR@/'
Line 80: P_VDSM_RUN = '@VDSMRUNDIR@/'


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

Line 26: 
Line 27: import exception as ge
Line 28: from vdsm import constants
Line 29: from vdsm import utils
Line 30: from __init__ import makePublic
> Please move extra imports before workaround function
It's sad that we have to use this

 from __init__ import makePublic

trick to allow importing from a package. Please add a comment about it.
Line 31: 
Line 32: 
Line 33: GLUSTER_VOL_PROTOCAL = 'tcp'
Line 34: GLUSTER_VOL_HOST = 'localhost'


Line 189: try:
Line 190: res = volumeStatvfsGet(args.volume, args.host,
Line 191:args.port, args.protocol)
Line 192: except ge.GlusterLibgfapiException:
Line 193: print "Failed to retrive volume stats"
> Please print to stderr
or use sys.stderr.write() to make Python 3 geeks happy.

But more importantly, why is the special handling of GlusterLibgfapiException ? 
ANY exception that happens during volumeStatvfsGet means just that. I think we 
can throw away the whole try-block.
Line 194: sys.exit(1)
Line 195: print json.dumps({'f_blocks': res.f_blocks, 'f_bfree': 
res.f_bfree,
Line 196:   'f_bsize': res.f_bsize, 'f_frsize': 
res.f_frsize,
Line 197:   'f_bavail': res.f_bavail, 'f_files': 
res.f_files,


Line 191:args.port, args.protocol)
Line 192: except ge.GlusterLibgfapiException:
Line 193: print "Failed to retrive volume stats"
Line 194: sys.exit(1)
Line 195: print json.dumps({'f_blocks': res.f_blocks, 'f_bfree': 
res.f_bfree,
It would be more succinct to have

  dict(zip(res._fields, res))

And there's no need to use "print". json.dump() is enough.
Line 196:   'f_bsize': res.f_bsize, 'f_frsize': 
res.f_frsize,
Line 197:   'f_bavail': res.f_bavail, 'f_files': 
res.f_files,
Line 198:   'f_ffree': res.f_ffree, 'f_favail': 
res.f_favail,
Line 199:   '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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread barumuga
Bala.FA has posted comments on this change.

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


Patch Set 3: Code-Review-1

(6 comments)

http://gerrit.ovirt.org/#/c/33312/3/lib/vdsm/constants.py.in
File lib/vdsm/constants.py.in:

Line 72: # Path definitions
Line 73: #
Line 74: P_LIBVIRT_VMCHANNELS = '/var/lib/libvirt/qemu/channels/'
Line 75: P_VDSM = '@VDSMDIR@/'
Line 76: P_VDSM_GLUSTER = '@VDSMDIR@/gluster/'
I feel its better to be moved to gfapi.py
Line 77: P_VDSM_RPC = '@VDSMDIR@/rpc/'
Line 78: P_VDSM_HOOKS = '@HOOKSDIR@/'
Line 79: P_VDSM_LIB = '@VDSMLIBDIR@/'
Line 80: P_VDSM_RUN = '@VDSMRUNDIR@/'


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

Line 26: 
Line 27: import exception as ge
Line 28: from vdsm import constants
Line 29: from vdsm import utils
Line 30: from __init__ import makePublic
Please move extra imports before workaround function
Line 31: 
Line 32: 
Line 33: GLUSTER_VOL_PROTOCAL = 'tcp'
Line 34: GLUSTER_VOL_HOST = 'localhost'


Line 146: # avoid that, this file is executed as script.
Line 147: # This can be reverted back once the memory leak issue
Line 148: # is fixed in libgfapi.
Line 149: @makePublic
Line 150: def volumeStatvfs(volumeName):
1. This function should accept as similar as volumeStatVfsGet()

2. For simplicity you could have two function and conditionally call based on 
_WORKAROUND constant.  This way function prototype will not change.
Line 151: script = constants.P_VDSM_GLUSTER + "gfapi.pyc"
Line 152: command = [constants.EXT_PYTHON, script, "-v", volumeName]
Line 153: rc, out, err = utils.execCmd(command)
Line 154: if rc != 0:


Line 149: @makePublic
Line 150: def volumeStatvfs(volumeName):
Line 151: script = constants.P_VDSM_GLUSTER + "gfapi.pyc"
Line 152: command = [constants.EXT_PYTHON, script, "-v", volumeName]
Line 153: rc, out, err = utils.execCmd(command)
You could use raw=True to get out/err as single string
Line 154: if rc != 0:
Line 155: raise ge.GlfsStatvfsException(rc, out, err)
Line 156: json_acceptable_string = out[0].replace("'", "\"")
Line 157: res = json.loads(json_acceptable_string)


Line 173: def parse_cmdargs():
Line 174: parser = argparse.ArgumentParser()
Line 175: parser.add_argument("-v", "--volume", action="store", type=str,
Line 176: help="volumeName")
Line 177: parser.add_argument("-ht", "--host", action="store", type=str,
Please have -h or -H
Line 178: default="localhost", help="host name")
Line 179: parser.add_argument("-p", "--port", action="store", type=int,
Line 180: default=24007, help="port number")
Line 181: parser.add_argument("-t", "--protocol", action="store", type=str,


Line 189: try:
Line 190: res = volumeStatvfsGet(args.volume, args.host,
Line 191:args.port, args.protocol)
Line 192: except ge.GlusterLibgfapiException:
Line 193: print "Failed to retrive volume stats"
Please print to stderr
Line 194: sys.exit(1)
Line 195: print json.dumps({'f_blocks': res.f_blocks, 'f_bfree': 
res.f_bfree,
Line 196:   'f_bsize': res.f_bsize, 'f_frsize': 
res.f_frsize,
Line 197:   'f_bavail': res.f_bavail, 'f_files': 
res.f_files,


-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 3:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11634/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12578/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12423/ : SUCCESS

-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread dnarayan
Darshan N has posted comments on this change.

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


Patch Set 3: Verified+1

-- 
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: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-25 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 2: Code-Review-1 Verified-1

Build Unstable 

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11633/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12577/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12422/ : UNSTABLE

-- 
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-24 Thread danken
Dan Kenigsberg has posted comments on this change.

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


Patch Set 1: Code-Review-1

(4 comments)

http://gerrit.ovirt.org/#/c/33312/1/vdsm/gluster/api.py
File vdsm/gluster/api.py:

Line 204: self.svdsmProxy.glusterVolumeRemoveBrickForce(volumeName, 
brickList,
Line 205:   replicaCount)
Line 206: 
Line 207: def _computeVolumeStats(self, data):
Line 208: total = data['f_blocks'] * data['f_bsize']
keep the new function returning the same namedtuple result, so that the hack is 
localized.
Line 209: free = data['f_bfree'] * data['f_bsize']
Line 210: used = total - free
Line 211: return {'sizeTotal': str(total),
Line 212: 'sizeFree': str(free),


http://gerrit.ovirt.org/#/c/33312/1/vdsm/gluster/cli.py
File vdsm/gluster/cli.py:

Line 1058: raise 
ge.GlusterXmlErrorException(err=[etree.tostring(xmltree)])
Line 1059: 
Line 1060: 
Line 1061: # This is a workaround for memory leak caused by the
Line 1062: # libgfapi used to get volume statistics. Memory accumulates
please add a BZ# - we should know when this can be safely removed.
Line 1063: # every time the api is invoked to avoid that, it is invoked
Line 1064: # through the script gfapi.py. This can be reverted back once
Line 1065: # the memory leak issue is fixed in libgfapi.
Line 1066: @makePublic


Line 1064: # through the script gfapi.py. This can be reverted back once
Line 1065: # the memory leak issue is fixed in libgfapi.
Line 1066: @makePublic
Line 1067: def volumeStatvfs(volumeName):
Line 1068: script = constants.P_GFAPI_SCRIPT + "gfapi"
There's no need to move gfapi arround.

Executing

 [EXT_PYTHON, previous_location.pyc,"-v", volumeName]

should be fine. This can even be done within gfapi.py itself, so the hack is 
contained within one file.
Line 1069: command = [script, "-v", volumeName]
Line 1070: rc, out, err = utils.execCmd(command)
Line 1071: if rc != 0:
Line 1072: raise ge.GlfsStatvfsException(rc, out, err)


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

Line 161: args.port, args.protocol)
Line 162: except GlusterLibgfapiException:
Line 163: print "Failed to retrive volume stats"
Line 164: sys.exit(1)
Line 165: print json.dumps({'f_blocks': res.f_blocks, 'f_bfree': 
res.f_bfree,
please use

 json.dumps({...}, sys.stdout)

to face the Python 3 future


-- 
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-24 Thread dnarayan
Darshan N has posted comments on this change.

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


Patch Set 1: Verified+1

-- 
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Darshan N 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-24 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

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


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-fc20_created/374/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_install-rpm-sanity-el6_created/391/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11624/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12568/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12413/ : SUCCESS

-- 
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Darshan N 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


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

2014-09-24 Thread dnarayan
Darshan N has uploaded a new change for review.

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

gluster: Temporary fix for supervdsm memory leak.

Supervdsm memory leak is caused by memory leak in libgfapi
which is used by verb glusterVolumeStatsInfoGet to retrive
the volume statistics. This patch provides a temporary fix
until libgfapi issue is resolved.
In this patch libgfapi call is moved to the script gfapi
which provides volume statistics and this script is executed
using execCmd(), by this libgfapi invocation is not part
of supervdsm process.

Change-Id: I7079426178ce47008d9a3b83635afce98536ca34
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1142647
Signed-off-by: ndarshan 
---
M Makefile.am
M lib/vdsm/constants.py.in
M vdsm.spec.in
M vdsm/gluster/Makefile.am
M vdsm/gluster/__init__.py
M vdsm/gluster/api.py
M vdsm/gluster/cli.py
R vdsm/gluster/gfapi
8 files changed, 62 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/33312/1

diff --git a/Makefile.am b/Makefile.am
index 88a99c8..cbfbd53 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -70,6 +70,7 @@
contrib/profile-stats \
init/daemonAdapter \
vdsm/get-conf-item \
+   vdsm/gluster/gfapi \
vdsm/set-conf-item \
vdsm/supervdsmServer \
vdsm/vdsm \
diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index ee379d6..daa05f8 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -71,6 +71,7 @@
 #
 # Path definitions
 #
+P_GFAPI_SCRIPT = '@VDSMDIR@/gluster/'
 P_LIBVIRT_VMCHANNELS = '/var/lib/libvirt/qemu/channels/'
 P_VDSM = '@VDSMDIR@/'
 P_VDSM_RPC = '@VDSMDIR@/rpc/'
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 8438df7..28a85e2 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -1518,7 +1518,7 @@
 %{_datadir}/%{vdsm_name}/gluster/api.py*
 %{_datadir}/%{vdsm_name}/gluster/apiwrapper.py*
 %{_datadir}/%{vdsm_name}/rpc/vdsmapi-gluster-schema.json
-%{_datadir}/%{vdsm_name}/gluster/gfapi.py*
+%{_datadir}/%{vdsm_name}/gluster/gfapi
 %{_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 d1d3ebf..e6e4ca8 100644
--- a/vdsm/gluster/Makefile.am
+++ b/vdsm/gluster/Makefile.am
@@ -32,12 +32,15 @@
apiwrapper.py \
cli.py \
exception.py \
-   gfapi.py \
hooks.py \
services.py \
tasks.py \
$(NULL)
 
+dist_vdsmgluster_SCRIPTS = \
+   gfapi \
+   $(NULL)
+
 EXTRA_DIST = \
hostname.py.in
$(NULL)
diff --git a/vdsm/gluster/__init__.py b/vdsm/gluster/__init__.py
index b83efbb..0aefb8d 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', 'gfapi')
+MODULE_LIST = ('cli', 'hooks', 'services', 'tasks')
 
 
 def makePublic(func):
diff --git a/vdsm/gluster/api.py b/vdsm/gluster/api.py
index b9a08e5..067427b 100644
--- a/vdsm/gluster/api.py
+++ b/vdsm/gluster/api.py
@@ -205,8 +205,8 @@
   replicaCount)
 
 def _computeVolumeStats(self, data):
-total = data.f_blocks * data.f_bsize
-free = data.f_bfree * data.f_bsize
+total = data['f_blocks'] * data['f_bsize']
+free = data['f_bfree'] * data['f_bsize']
 used = total - free
 return {'sizeTotal': str(total),
 'sizeFree': str(free),
diff --git a/vdsm/gluster/cli.py b/vdsm/gluster/cli.py
index 2e1c9a9..aab7145 100644
--- a/vdsm/gluster/cli.py
+++ b/vdsm/gluster/cli.py
@@ -19,12 +19,14 @@
 #
 
 import xml.etree.cElementTree as etree
+import json
 
 from vdsm import utils
 from vdsm import netinfo
 import exception as ge
 from hostname import getHostNameFqdn, HostNameException
 from . import makePublic
+from vdsm import constants
 
 _glusterCommandPath = utils.CommandPath("gluster",
 "/usr/sbin/gluster",
@@ -1054,3 +1056,20 @@
 return _parseVolumeTasks(xmltree)
 except _etreeExceptions:
 raise ge.GlusterXmlErrorException(err=[etree.tostring(xmltree)])
+
+
+# This is a workaround for memory leak caused by the
+# libgfapi used to get volume statistics. Memory accumulates
+# every time the api is invoked to avoid that, it is invoked
+# through the script gfapi.py. This can be reverted back once
+# the memory leak issue is fixed in libgfapi.
+@makePublic
+def volumeStatvfs(volumeName):
+script = constants.P_GFAPI_SCRIPT + "gfapi"
+command = [script, "-v", volumeName]
+rc, out, err = utils.execCmd(command)
+if rc != 0:
+raise ge.GlfsStatvfsException(rc, out, err)
+json_acceptable_string = out[0].replace("'", "\"")
+res = json.loads(json_acceptable_string)
+retu