Nir Soffer has posted comments on this change. Change subject: sp: improve domainStateChange event handling ......................................................................
Patch Set 3: (3 comments) https://gerrit.ovirt.org/#/c/51393/3/vdsm/storage/sp.py File vdsm/storage/sp.py: Line 635 Line 636 Line 637 Line 638 Line 639 This should be reverted if next steps fail, and raise if pool is already connected. Must be handled in different patch before this one. Line 649: self.id = hostID Line 650: # Make sure SDCache doesn't have stale data (it can be in case of FC) Line 651: sdCache.invalidateStorage() Line 652: sdCache.refresh() Line 653: self._startWatchingDomainsState() > what will happen if we are conencted to the pool and issue connect call aga Good catch - since we register a new partial function, this may lead to multiple registrations, and multiple events when domain state changes. However this should be solved in connect - if the pool is connected, it must raise. Callers should call refresh if the pool is already connected. Line 654: try: Line 655: # Rebuild whole Pool Line 656: self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion) Line 657: except Exception: Line 671: @unsecured Line 672: def _stopWatchingDomainsState(self): Line 673: self.log.debug("Stopping to monitor domains state") Line 674: self.domainMonitor.onDomainStateChange.unregister( Line 675: self._domainStateCallback) You should handle KeyError here, see _finalizePoolUpgradeIfNeeded, but log a debug message if we get a KeyError - we don't expect this to fail. Line 676: Line 677: @unsecured Line 678: def stopMonitoringDomains(self): Line 679: self.domainMonitor.stopMonitoring(self.domainMonitor.poolDomains) -- To view, visit https://gerrit.ovirt.org/51393 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If116100d3ae967f6a5490a2d91bf923e953cb4ee Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Idan Shaby <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Freddy Rolland <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
