Federico Simoncelli has posted comments on this change. Change subject: domainMonitor: Extract domain monitoring methods ......................................................................
Patch Set 4: Code-Review+1 (3 comments) +1 but I'd like to see my comment being addressed. http://gerrit.ovirt.org/#/c/27714/4/vdsm/storage/domainMonitor.py File vdsm/storage/domainMonitor.py: Line 209: Line 210: def _monitorDomain(self): Line 211: self.nextStatus.clear() Line 212: Line 213: if self._shouldRefreshDomain(): The descriptive name is not covering "why" we need to refresh. I think the comment about the specific upgrade case should be left. Line 214: self._refreshDomain() Line 215: Line 216: try: Line 217: if self.domain is None: Line 270: # it might take some time and we don't want to slow down the thread Line 271: # start (and anything else that relies on that as for example Line 272: # updateMonitoringThreads). It also needs to be inside the loop Line 273: # since it might fail and we want keep trying until we succeed or Line 274: # the domain is deactivated. This comment doesn't belong here. It should be where you call _produceDomain. Line 275: self.domain = sdCache.produce(self.sdUUID) Line 276: Line 277: def _setIsoDomainInfo(self): Line 278: # The isIsoDomain assignment is delayed because the isoPrefix discovery Line 276: Line 277: def _setIsoDomainInfo(self): Line 278: # The isIsoDomain assignment is delayed because the isoPrefix discovery Line 279: # might fail (if the domain suddenly disappears) and we could risk to Line 280: # never try to set it again. Also this comment belongs to where you call _setIsoDomainInfo. Line 281: isIsoDomain = self.domain.isISO() Line 282: if isIsoDomain: Line 283: self.isoPrefix = self.domain.getIsoDomainImagesDir() Line 284: self.isIsoDomain = isIsoDomain -- To view, visit http://gerrit.ovirt.org/27714 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87ac6a82e560bc4360a3bc3f6f6fd94623678cc2 Gerrit-PatchSet: 4 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: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Xavi Francisco <[email protected]> Gerrit-Reviewer: Yoav Kleinberger <[email protected]> Gerrit-Reviewer: [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
