Nir Soffer has posted comments on this change. Change subject: sp: allow executing upgradePool even if there is a pending update ......................................................................
Patch Set 1: Code-Review-1 (3 comments) Looks like wrong approach, we are making this complex code even more complex. The correct solution is make sure the old upgrade cannot be stuck forever, and wait until it times out before doing new upgrades, or redesign things so domain upgrades do not depend on each other. There is no chance to get such changes in stable version. https://gerrit.ovirt.org/#/c/45774/1/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 427: rm.LockType.exclusive): Line 428: acquiredDomainResources = [] Line 429: Line 430: sd.validateDomainVersion(targetDomVersion) Line 431: try: Do not replace with with try finally. Line 432: self.log.info("Trying to lock domains that are currently pending for an update") Line 433: for sdUUID in self._domainsToUpgrade: Line 434: try: Line 435: res = rmanager.acquireResource(STORAGE, "upgrade_" + sdUUID, Line 432: self.log.info("Trying to lock domains that are currently pending for an update") Line 433: for sdUUID in self._domainsToUpgrade: Line 434: try: Line 435: res = rmanager.acquireResource(STORAGE, "upgrade_" + sdUUID, Line 436: rm.LockType.exclusive) Please do not change locking in this delicate parts. Line 437: except (rm.RequestTimedOutError, se.ResourceAcqusitionFailed): Line 438: self.log.info("Cannot acquire domain upgrade resource '%s'", str(sdUUID)) Line 439: raise se.PoolUpgradeInProgress(self.spUUID, self.sdUUID) Line 440: else: Line 453: except ValueError: Line 454: pass Line 455: finally: Line 456: for res in acquiredDomainResources: Line 457: res.release() This is not safe, if res.release() raises, you are left with locks that nobody will ever release. Line 458: Line 459: self.log.debug("Registering with state change event") Line 460: self.domainMonitor.onDomainStateChange.register( Line 461: self._upgradeCallback) -- To view, visit https://gerrit.ovirt.org/45774 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c30439b5b84f64fd2ad69d82f33dfdfc4c9ef68 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Liron Aravot <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
