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