Mark Wu has posted comments on this change. Change subject: Supervdsm as external service ......................................................................
Patch Set 8: (3 inline comments) .................................................... File vdsm/supervdsm.py 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") Line 35: TIMESTAMP = os.path.join(constants.P_VDSM_RUN, "svdsm.time") we don't need it any more, right? So you should remove it. Line 36: ADDRESS = os.path.join(constants.P_VDSM_RUN, "svdsm.sock") Line 37: Line 38: Line 39: class _SuperVdsmManager(BaseManager): Line 49: def __call__(self, *args, **kwargs): Line 50: callMethod = lambda: \ Line 51: getattr(self._supervdsmProxy._svdsm, self._funcName)(*args, Line 52: **kwargs) Line 53: if not self._supervdsmProxy.isRunning(): even though the super vdsm process can be respawned on crash, but it can't detect the case of non-responsive. So we still need the ping check, right? Line 54: # getting inside only when svdsm is down. its rare case so we Line 55: # don't care that isRunning will run twice Line 56: with self._supervdsmProxy.proxyLock: Line 57: if not self._supervdsmProxy.isRunning(): .................................................... File vdsm/supervdsmServer.py Line 391: while servThread.isAlive(): Line 392: servThread.join(5) Line 393: finally: Line 394: # Verifing that the socket is related to the current instance of Line 395: # supervdsmServer before removing it. I suggest moving the pid file checking into the daemon itself (line 363), the it couldn't see a pid file with different pid because the daemon will fail to start if the pid file exists. Line 396: if file(PIDFILE, 'r').read().strip() == str(os.getpid()): Line 397: if os.path.exists(address): Line 398: utils.rmFile(address) Line 399: -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Mark Wu <[email protected]> Gerrit-Reviewer: Royce Lv <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
