Yaniv Bronhaim has posted comments on this change.
Change subject: Move fencing logic out of API.py
......................................................................
Patch Set 7: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File vdsm/clientIF.py
Line 94: vm.MigrationSourceThread.setMaxOutgoingMigrations(mog)
Line 95:
Line 96: self.lastRemoteAccess = 0
Line 97: self._memLock = threading.Lock()
Line 98: self._enabled = True
Now you don't need this _enabled variable anymore. If you think event is better here,
you should replace all the _enabled use with your new event.
Line 99: self.shutdownEvent = threading.Event()
Line 100: self._netConfigDirty = False
Line 101: self._prepareMOM()
Line 102: threading.Thread(target=self._recoverExistingVms,
....................................................
File vdsm/fenceAgent.py
Line 23: try:
Line 24: p.stdin.write(inp)
Line 25: p.stdin.close()
Line 26: while p.poll() is None:
Line 27: cancelEvent.wait(1)
if cancelEvent = None ??
Line 28: if cancelEvent.isSet():
Line 29: log.debug('killing fence script pid %s', p.pid)
Line 30: os.kill(p.pid, signal.SIGTERM)
Line 31: time.sleep(1)
Line 27: cancelEvent.wait(1)
Line 28: if cancelEvent.isSet():
Line 29: log.debug('killing fence script pid %s', p.pid)
Line 30: os.kill(p.pid, signal.SIGTERM)
Line 31: time.sleep(1)
just raising a thought, is 1 second enough? vdsm works 40seconds before exit after
SIGTERM. sleeping 1sec before SIGKILL sounds fast. You can check if pid still exists and
than send the SIGKILL..
Line 32: try:
Line 33: os.kill(p.pid, signal.SIGKILL)
Line 34: except:
Line 35: pass
Line 29: log.debug('killing fence script pid %s', p.pid)
Line 30: os.kill(p.pid, signal.SIGTERM)
Line 31: time.sleep(1)
Line 32: try:
Line 33: os.kill(p.pid, signal.SIGKILL)
smells like you need timestemp here.. the pid might be used by another process
Line 34: except:
Line 35: pass
Line 36: return
Line 37:
--
To view, visit
http://gerrit.ovirt.org/7190
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I944c6548a42612f705a410fb4290215451bca035
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server