Alon Bar-Lev has posted comments on this change.

Change subject: engine: application touches a file to detect un orderly shutdown
......................................................................


Patch Set 3:

(5 comments)

....................................................
File packaging/services/ovirt-engine/ovirt-engine.conf.in
Line 34
Line 35
Line 36
Line 37
Line 38
please move file to here.


Line 197: #
Line 198: # A file created on init and removed on orderly shutdown
Line 199: # Used to diagnose unexpected engine stop by the notification service.
Line 200: #
Line 201: ENGINE_UP_MARK=${ENGINE_VAR}/engine.up
quotes please

please move above


....................................................
File packaging/services/ovirt-engine/ovirt-engine.py
Line 447:         return (consoleLog, consoleLog)
Line 448: 
Line 449:     def daemonContext(self):
Line 450:         # create a application pid file, not to be confused with 
service.py pid file
Line 451:         # used by sysv
hmm...?

 #
 # create mark file to be used by notifier service
 #
 with open(self._config.get('ENGINE_UP_MARK'), 'w') as f:
     f.write('%s\n' % os.getpid())
Line 452:         self.logger.info('creating pid file')
Line 453:         with file(self._config.get('ENGINE_UP_MARK'), 'w') as f:
Line 454:             f.write('%s\n' % os.getpid())
Line 455:         self.daemonAsExternalProcess(


Line 450:         # create a application pid file, not to be confused with 
service.py pid file
Line 451:         # used by sysv
Line 452:         self.logger.info('creating pid file')
Line 453:         with file(self._config.get('ENGINE_UP_MARK'), 'w') as f:
Line 454:             f.write('%s\n' % os.getpid())
space line
Line 455:         self.daemonAsExternalProcess(
Line 456:             executable=self._executable,
Line 457:             args=self._engineArgs,
Line 458:             env=self._engineEnv,


Line 466: 
Line 467:     def daemonCleanup(self):
Line 468:         self._tempDir.destroy()
Line 469:         self.logger.info('removing pid file')
Line 470:         os.remove(self._config.get('ENGINE_UP_MARK'))
this happening at finally so it is not good for the usage, as if aborted by any 
reason we want to know, right? so move this up to daemonContext and do this in 
normal exit flow or if self.TerminateException is received.

Also, no need for the excessive log... it is obvious that you remove the file...

Also I recommend to remove it only if exists.
Line 471: 
Line 472: 
Line 473: if __name__ == '__main__':
Line 474:     service.setupLogger()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic8de64637299e7932a5a2761c7bb495373c418aa
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to