Change in vdsm[master]: isRunning didn't check local variable before reading saved d...

ybronhei at redhat.com ybronhei at redhat.com
Mon Dec 31 09:00:16 UTC 2012


Yaniv Bronhaim has posted comments on this change.

Change subject: isRunning didn't check local variable before reading saved data
......................................................................


Patch Set 2: (1 inline comment)

Dan, I don't understand your comment. please explain.. 
Thanks.

....................................................
File vdsm/supervdsm.py
Line 143:         self._svdsm = None
Line 144:         self._firstLaunch = True
Line 145: 
Line 146:     def isRunning(self):
Line 147:         if self._firstLaunch or self._svdsm is None:
this is exactly the reason why i can use firstLaunch,
we don't need to check self.pidfile if we know it is the first run ...
I don't see how can it be raceful?? it's really unrelated to self.pidfile at all... 


i just say, first isRunning must return false if we still haven't changed firstLaunch boolean or we haven't set _svdsm var, both means we haven't call lanuch() yet (or we called kill() and cleaned to local vars).. after calling launch, isRunning should check the internal files to distinguish if svdsm is up or not.
Line 148:             return False
Line 149: 
Line 150:         try:
Line 151:             with open(self.pidfile, "r") as f:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9dec0c6955dadcd959cc1c8df4e9745322fb0ce3
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce at linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list