[node-patches] Change in ovirt-node[master]: ovirt-node-upgrade: add lock file check to prevent parallel ...

[Alon Bar-Lev]

Alon Bar-Lev has posted comments on this change.

Change subject: ovirt-node-upgrade: add lock file check to prevent parallel runs
......................................................................


Patch Set 6: (4 inline comments)

....................................................
File scripts/ovirt-node-upgrade.py
Line 82:         os.close(lock_fd)
Line 83: 
Line 84:     def __enter__(self):
Line 85:         self._acquire()
Line 86:         return
return?
Line 87: 
Line 88:     def __exit__(self, exc_type, exc_value, traceback):
Line 89:         self._remove()
Line 90:         return


Line 86:         return
Line 87: 
Line 88:     def __exit__(self, exc_type, exc_value, traceback):
Line 89:         self._remove()
Line 90:         return
return?
Line 91: 
Line 92:     def _acquire(self):
Line 93:         self._logger.info("Acquiring Lock")
Line 94:         if os.path.exists(OVIRT_UPGRADE_LOCK):


Line 109:             raise RuntimeError("Unable to write lockfile")
Line 110: 
Line 111:     def _remove(self):
Line 112:         try:
Line 113:             os.remove(OVIRT_UPGRADE_LOCK)
if we are the owner... I think you reach remove in any case, casing non owner to remove the lock.
Line 114:             if os.path.exists(self._lock_file):
Line 115:                 os.remove(self._lock_file)
Line 116:         except Exception as e:
Line 117:             self._logger.exception('Error: Upgrade Failed: %s', e)


Line 355:                 self._logger.info("Upgrade Failed, Rollback Completed")
Line 356:                 sys.exit(1)
Line 357:             finally:
Line 358:                 self._cleanup()
Line 359:                 sys.exit(0)
are you sure the sys.exit(1) will not go into the finally and exit with sys.exit(0)?

better to have a ret variable and exit at one place.
Line 360: 
Line 361: if __name__ == "__main__":
Line 362:     initLogger()
Line 363:     u = UpgradeTool()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdd6553046d99cf9c9db29c1c899dc93e8ad733a
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Joey Boggs <jboggs at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: Joey Boggs <jboggs at redhat.com>
Gerrit-Reviewer: Michael Burns <mburns at redhat.com>