Federico Simoncelli has posted comments on this change. Change subject: sp: improve masterMigrate safety ......................................................................
Patch Set 5: (2 comments) http://gerrit.ovirt.org/#/c/22419/5//COMMIT_MSG Commit Message: Line 14: - refactor out all the required metadata changes in switchMasterDomain Line 15: - divde the method in two parts (operations required to switch to the Line 16: new master and previous master cleanup) Line 17: - address the two parts exception handling (hard failure for the first, Line 18: logging only for the second) Beside "divde" being misspelled, according to: http://www.oxforddictionaries.com/words/bullet-points "If the text that follows the bullet point is not a proper sentence, it doesn’t need to begin with a capital letter and it shouldn’t end with a full stop" Line 19: Line 20: Change-Id: I155b36d41df43230f99ba610699bba05bee3f0c0 http://gerrit.ovirt.org/#/c/22419/5/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 872: # or even in this method if we fail to set the old master to Line 873: # regular). That said, for API cleaness switchMasterDomain is Line 874: # the last method to call as "point of no return" after which we Line 875: # only try to cleanup but we cannot rollback. Line 876: newmsd.changeRole(sd.MASTER_DOMAIN) > Lets add an empty line before the critical switchMasterDomain() call, to ma The comment is about both lines, it's explaining why it's fine to have changeRole before switchMasterDomain, and it's describing the switchMasterDomain contract. Line 877: self.switchMasterDomain(curmsd, newmsd, masterVersion) Line 878: except Exception: Line 879: self.log.exception('migration to new master failed') Line 880: try: -- To view, visit http://gerrit.ovirt.org/22419 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I155b36d41df43230f99ba610699bba05bee3f0c0 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
