Royce Lv has posted comments on this change.
Change subject: [WIP] creating supervdsm service
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(5 inline comments)
Generally I like this design.I noticed supervdsmd does not restart when vdsm
restart, I assume we're going to handle this after the main framework is stable.
problems need fix as I mentioned in comments.
Also I'll help re-examine the edge case after these problems settled.
....................................................
File vdsm/supervdsmd.init.in
Line 18: # Default-Start: 2 3 4 5
Line 19: # Default-Stop: 0 1 6
Line 20: # Description: init script for the Super VDS management server
Line 21: # Short-Description: init script for the Super VDS management server
Line 22: ### END INIT INFO
pls add supervdsm.init in .gitignore and clean files, or build will fail
Line 23:
Line 24: SUPERVDSM_BIN=@VDSMDIR@/supervdsmServer.py
Line 25: PIDFILE=@VDSMRUNDIR@/supervdsmd.pid
Line 26: RESPAWNPIDFILE=@VDSMRUNDIR@/supervdsm_respawn.pid
Line 33: log_failure_msg() { echo -n "$@"; failure "$@"; echo; }
Line 34: log_success_msg() { echo -n "$@"; success "$@"; echo; }
Line 35:
Line 36: start(){
Line 37: LC_ALL=C @VDSMDIR@/respawn --minlifetime 10 --daemon
--masterpid $RESPAWNPIDFILE $SUPERVDSM_BIN $AUTH_KEY $SOCKFILE $VDSM_USER
problem(may not related to this one) why should we keep using respawn script
when we have respawn for upstart and systemd?
Line 38: RETVAL=$?
Line 39: [ "$RETVAL" -eq 0 ] && log_success_msg $"$prog start" ||
log_failure_msg $"$prog start"
Line 40: }
Line 41:
....................................................
File vdsm/supervdsm.py
Line 70: # don't care that isRunning will run twice
Line 71: with self._supervdsmProxy.proxyLock:
Line 72: if not self._supervdsmProxy.isRunning():
Line 73: self._supervdsmProxy.launch()
Line 74:
I assume we are planning to fix this problem I've mentioned, I suggest we don't
mix following two way:
1. make sure svdsm is OK through isRunning and call,if error , raise directly
2.directly call svdsm see it is OK, if not, retry.
Just choose one will make it right, mix things make it complex.
Line 75: try:
Line 76: return callMethod()
Line 77: # handling internal exception that we raise to identify
supervdsm
Line 78: # validation.
Line 116: try:
Line 117: self._manager.connect()
Line 118: except Exception, ex:
Line 119: self._log.warn("Connect to svdsm failed %s", ex)
Line 120: raise
We should define this behaviour:
*if we can't connect svdsm, is it fatal or not?
If it seriously effect vdsm service, we start vdsm and svdsm all over;
if we can downgrade vdsm service without supervdsm,
we should wrap a specific error to indicate supervdsm service error.
Line 121: self._svdsm = self._manager.instance()
Line 122:
Line 123: def launch(self):
Line 124: utils.retry(self._connect, Exception, timeout=60)
....................................................
File vdsm/supervdsmServer.py
Line 79: raise
Line 80: return wrapper
Line 81:
Line 82: KB = 2 ** 10
Line 83: TEST_BUFF_LEN = 4 * KB
We should use another log file for supervdsm, as supervdsm use the same
logger.conf as vdsm, if it start first, it will generate "vdsm.log" with root
priviledge.
Line 84: LOG_CONF_PATH = "/etc/vdsm/logger.conf"
Line 85:
Line 86:
Line 87: class _SuperVdsm(object):
--
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Royce Lv <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
