Nir Soffer has posted comments on this change. Change subject: sp: Nest try-finally for temporary secure change ......................................................................
Patch Set 1: (4 comments) See the comments explaining the issues of the previous code. http://gerrit.ovirt.org/#/c/23955/1/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 597 Line 598 Line 599 Line 600 Line 601 If this raise, the temporary lock will not be released, and monitors would not be stopped. Line 701 Line 702 Line 703 Line 704 Line 705 Note - if this raise, the lock will not released, and monitor would not be stopped. Line 598: self.log.error("Cleanup failed due to an unexpected error", Line 599: exc_info=True) Line 600: raise Line 601: finally: Line 602: self._setUnsecure() If this raise, everything is cleaned up properly. Line 603: finally: Line 604: self._releaseTemporaryClusterLock(msdUUID) Line 605: self.stopMonitoringDomains() Line 606: Line 702: # As in the create method we need to temporarily set the object Line 703: # secure in order to change the domains map. Line 704: # TODO: it is clear that reconstructMaster and create (StoragePool) Line 705: # are extremely similar and they should be unified. Line 706: self._setSecure() If this raise, the lock and monitors are cleaned up properly. Line 707: try: Line 708: self.createMaster(poolName, futureMaster, masterVersion, Line 709: leaseParams) Line 710: self.setMasterDomain(msdUUID, masterVersion) -- To view, visit http://gerrit.ovirt.org/23955 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia9054063db0eeacd156a8e586681496515f80e2b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
