Change in vdsm[master]: Avoiding automatically restart of sanlock service when start...

Alon Bar-Lev alonbl at redhat.com
Wed Oct 9 17:19:51 UTC 2013


Alon Bar-Lev has posted comments on this change.

Change subject: Avoiding automatically restart of sanlock service when starting vdsmd
......................................................................


Patch Set 8:

(6 comments)

....................................................
File lib/vdsm/tool/configurator.py
Line 93:     """
Line 94:     sanlock_pid_file = "/var/run/sanlock/sanlock.pid"
Line 95:     proc_status_path = "/proc/%s/status"
Line 96:     proc_status_group_prefix = "Groups:\t"
Line 97:     group_prefix_len = len(proc_status_group_prefix)
why do you need constant for string len?
Line 98: 
Line 99:     try:
Line 100:         sanlock_pid = open(sanlock_pid_file, "r").readline().strip()
Line 101:         sanlock_status = open(proc_status_path % sanlock_pid, "r")


Line 96:     proc_status_group_prefix = "Groups:\t"
Line 97:     group_prefix_len = len(proc_status_group_prefix)
Line 98: 
Line 99:     try:
Line 100:         sanlock_pid = open(sanlock_pid_file, "r").readline().strip()
this leaks
Line 101:         sanlock_status = open(proc_status_path % sanlock_pid, "r")
Line 102:     except IOError as e:
Line 103:         if e.errno == os.errno.ENOENT:
Line 104:             return 0  # service is not running, returning


Line 97:     group_prefix_len = len(proc_status_group_prefix)
Line 98: 
Line 99:     try:
Line 100:         sanlock_pid = open(sanlock_pid_file, "r").readline().strip()
Line 101:         sanlock_status = open(proc_status_path % sanlock_pid, "r")
this leaks
Line 102:     except IOError as e:
Line 103:         if e.errno == os.errno.ENOENT:
Line 104:             return 0  # service is not running, returning
Line 105:         raise


Line 114: 
Line 115:     diskimage_gid = grp.getgrnam(DISKIMAGE_GROUP)[2]
Line 116:     if diskimage_gid in groups:
Line 117:         sys.stdout.write("sanlock service requires restart\n")
Line 118:         return 0
again... I do not like return in middle of functions...
Line 119: 
Line 120:     sys.stdout.write("sanlock service is already configured\n")
Line 121:     return 1
Line 122: 


Line 120:     sys.stdout.write("sanlock service is already configured\n")
Line 121:     return 1
Line 122: 
Line 123: 
Line 124: @vdsm.tool.expose("reconfigure-all")
configure-all ?
Line 125: def configure_all(*args):
Line 126:     """
Line 127:     Configure related services for vdsm and restart them. --force for
Line 128:     forcing restart of related services after configure


Line 130:     if '--force' in args:
Line 131:         service.service_stop("supervdsmd")
Line 132:         service.service_stop("libvirtd")
Line 133: 
Line 134:     configure_libvirt(*args)
don't you need to check rc?
Line 135:     if sanlock_check_service(*args):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I48658066f707632719df8d65799d26c7b239dcd5
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list