Change in vdsm[master]: supervdsmServer is down after failed operation

danken at redhat.com danken at redhat.com
Tue Oct 2 11:23:46 UTC 2012


Dan Kenigsberg has posted comments on this change.

Change subject: supervdsmServer is down after failed operation
......................................................................


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

(4 inline comments)

....................................................
File vdsm/supervdsm.py
Line 43: 
Line 44:     raise RuntimeError("SuperVDSM Server not found")
Line 45: 
Line 46: PIDFILE = os.path.join(constants.P_VDSM_RUN, "svdsm.pid")
Line 47: TIMESTEMP = os.path.join(constants.P_VDSM_RUN, "svdsm.time")
TIMESTAMP
Line 48: ADDRESS = os.path.join(constants.P_VDSM_RUN, "svdsm.sock")
Line 49: SUPERVDSM = __supervdsmServerPath()
Line 50: 
Line 51: 


Line 62:     def __call__(self, *args, **kwargs):
Line 63:         callMethod = lambda: \
Line 64:             getattr(self._supervdsmProxy._svdsm, self._funcName)(*args,
Line 65:                                                                  **kwargs)
Line 66:         if not self._supervdsmProxy.isRunning():
test-and-later-launch is racy.
Line 67:             self._supervdsmProxy.launch()
Line 68: 
Line 69:         try:
Line 70:             return callMethod()


Line 79:     """
Line 80:     A wrapper around all the supervdsm init stuff
Line 81:     """
Line 82:     _log = logging.getLogger("SuperVdsmProxy")
Line 83:     pidfile = PIDFILE
I like making the code more testable, but this merits its own patch imo.

If possible, use the same name for the module var and the class var.
Line 84:     tsfile = TIMESTEMP
Line 85: 
Line 86:     def open(self, *args, **kwargs):
Line 87:         return self._manager.open(*args, **kwargs)


Line 107:         try:
Line 108:             with open(self.pidfile, "r") as f:
Line 109:                 spid = int(f.read().strip())
Line 110:             with open(self.tsfile, "r") as f:
Line 111:                 createdTime = f.read().strip()
I think that os.stat(pidfile) would give you the same info about creation time, with no need for another file.
Line 112:                 pTime = str(misc.getProcCtime(spid))
Line 113: 
Line 114:             if pTime == createdTime:
Line 115:                 return True


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idad4a622b82259b777851d1b0c1b37ec8da2b01e
Gerrit-PatchSet: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei 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: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce at linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>


More information about the vdsm-patches mailing list