Change in vdsm[master]: Supervdsm as external service

danken at redhat.com danken at redhat.com
Tue May 21 05:55:30 UTC 2013


Dan Kenigsberg has posted comments on this change.

Change subject: Supervdsm as external service
......................................................................


Patch Set 9: I would prefer that you didn't submit this

(6 inline comments)

Incomplete review, sorry.

....................................................
File configure.ac
Line 189: AC_PATH_PROG([UNPERSIST_PATH], [unpersist], [/usr/sbin/unpersist])
Line 190: AC_PATH_PROG([WGET_PATH], [wget], [/usr/bin/wget])
Line 191: AC_PATH_PROG([YUM_PATH], [yum], [/usr/bin/yum])
Line 192: AC_PATH_PROG([VDSM_TOOL], [vdsm-tool], [/usr/bin/vdsm-tool])
Line 193: 
Why is this needed? Vdsm-tool is not to be found. Also the naming of the macro is inconsistent.
Line 194: # Keep sorted
Line 195: AC_OUTPUT([
Line 196: 	Makefile
Line 197: 	client/Makefile


....................................................
File lib/vdsm/constants.py.in
Line 135: 
Line 136: EXT_VDSM_STORE_NET_CONFIG = '@VDSMDIR@/vdsm-store-net-config'
Line 137: 
Line 138: EXT_WGET = '@WGET_PATH@'
Line 139: EXT_WRITE_NET_CONFIG = '@VDSMDIR@/write-net-config'
Bad rebase?
Line 140: 


....................................................
File vdsm/sudoers.vdsm.in
Line 30
Line 31
Line 32
Line 33
Line 34
Does vdsm need to restart supervdsm? Why?


....................................................
File vdsm/supervdsm.py
Line 30: _g_singletonSupervdsmInstance_lock = threading.Lock()
Line 31: 
Line 32: 
Line 33: PIDFILE = os.path.join(constants.P_VDSM_RUN, "supervdsmd.pid")
Line 34: AUTHFILE = os.path.join(os.path.dirname(__file__), "supervdsm_auth")
I see no point in a password file. It add no further security over limiting socket access to vdsm user.
Line 35: ADDRESS = os.path.join(constants.P_VDSM_RUN, "svdsm.sock")
Line 36: 
Line 37: 
Line 38: class _SuperVdsmManager(BaseManager):


Line 53:             # getting inside only when svdsm is down. its rare case so we
Line 54:             # don't care that isRunning will run twice
Line 55:             with self._supervdsmProxy.proxyLock:
Line 56:                 if not self._supervdsmProxy.isRunning():
Line 57:                     self._supervdsmProxy.relaunch()
Would you remind me why do we need to keep the relaunch logic? Supervdsm should just run. Always. And that's that.
Line 58: 
Line 59:         try:
Line 60:             return callMethod()
Line 61:         # handling internal exception that we raise to identify supervdsm


....................................................
File vdsm/vdsmd.init.in
Line 33: DOM_METADATA_BACKUP_DIR=/var/log/vdsm/backup
Line 34: CORE_PATTERN=/proc/sys/kernel/core_pattern
Line 35: QEMU_DUMP_PATH="/var/log/core"
Line 36: NEEDED_SERVICES="iscsid multipathd ntpd wdmd sanlock network libvirtd
Line 37: 		 supervdsmd"
Need new tabs please.
Line 38: CONFLICTING_SERVICES="libvirt-guests"
Line 39: 
Line 40: # trigger for reconfiguration
Line 41: FORCE_RECONFIGURE=@VDSMLIBDIR@/reconfigure


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I290a584f38129406cd390fdd1d3d1aad9f829a60
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Mei Liu <liumbj at linux.vnet.ibm.com>
Gerrit-Reviewer: Ohad Basan <obasan at redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce at linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list