Hello Dan Kenigsberg, Eduardo,
I'd like you to do a code review. Please visit
to review the following change.
Change subject: Make getRepoStats() a hsm method. ......................................................................
Make getRepoStats() a hsm method.
Making repoStats pool independent.
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1019273 Change-Id: I0273611a23f29b5c6be0354a4c6b2d6526a9b574 Signed-off-by: Eduardo ewarszaw@redhat.com Reviewed-on: http://gerrit.ovirt.org/14673 Reviewed-by: Dan Kenigsberg danken@redhat.com --- M vdsm/storage/hsm.py M vdsm/storage/sp.py 2 files changed, 108 insertions(+), 98 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/92/20192/1
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index ace6634..01edb44 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -2540,7 +2540,18 @@ vars.task.setDefaultException( se.StoragePoolActionError("spUUID=%s" % spUUID)) vars.task.getSharedLock(STORAGE, spUUID) - return self.getPool(spUUID).getInfo() + pool = self.getPool(spUUID) + poolInfo = pool.getInfo() + doms = pool.getDomains() + domInfo = self._getDomsStats(pool.domainMonitor, doms) + for sdUUID in doms.iterkeys(): + if domInfo[sdUUID]['isoprefix']: + poolInfo['isoprefix'] = domInfo[sdUUID]['isoprefix'] + break + else: + poolInfo['isoprefix'] = '' # No ISO domain found + + return dict(info=poolInfo, dominfo=domInfo)
@public def createStorageDomain(self, storageType, sdUUID, domainName, @@ -3547,6 +3558,97 @@ pool._upgradePool(targetDomVersion) return {"upgradeStatus": "started"}
+ def _getDomsStats(self, domainMonitor, doms): + domInfo = {} + repoStats = self._getRepoStats(domainMonitor) + + for sdUUID, sdStatus in doms.iteritems(): + # Return statistics for active domains only + domInfo[sdUUID] = {'status': sdStatus, 'alerts': []} + + if sdStatus != sd.DOM_ACTIVE_STATUS or sdUUID not in repoStats: + continue + + domInfo[sdUUID]['version'] = repoStats[sdUUID]['result']['version'] + + # For unreachable domains repoStats will return disktotal and + # diskfree as None. + if (repoStats[sdUUID]['disktotal'] is not None + and repoStats[sdUUID]['diskfree'] is not None): + domInfo[sdUUID]['disktotal'] = repoStats[sdUUID]['disktotal'] + domInfo[sdUUID]['diskfree'] = repoStats[sdUUID]['diskfree'] + + if not repoStats[sdUUID]['mdavalid']: + domInfo[sdUUID]['alerts'].append({ + 'code': se.SmallVgMetadata.code, + 'message': se.SmallVgMetadata.message, + }) + self.log.warning("VG %s's metadata size too small %s", + sdUUID, repoStats[sdUUID]['mdasize']) + + if not repoStats[sdUUID]['mdathreshold']: + domInfo[sdUUID]['alerts'].append({ + 'code': se.VgMetadataCriticallyFull.code, + 'message': se.VgMetadataCriticallyFull.message, + }) + self.log.warning("VG %s's metadata size exceeded critical " + "size: mdasize=%s mdafree=%s", sdUUID, + repoStats[sdUUID]['mdasize'], + repoStats[sdUUID]['mdafree']) + + isoprefix = repoStats[sdUUID]['isoprefix'] + domInfo[sdUUID]['isoprefix'] = \ + isoprefix if isoprefix is not None else '' + + return domInfo + + def _getRepoStats(self, domainMonitor): + repoStats = {} + statsGenTime = time.time() + + for sdUUID in domainMonitor.monitoredDomains: + domStatus = domainMonitor.getStatus(sdUUID) + + if domStatus.error is None: + code = 0 + elif isinstance(domStatus.error, se.StorageException): + code = domStatus.error.code + else: + code = se.StorageException.code + + disktotal, diskfree = domStatus.diskUtilization + vgmdtotal, vgmdfree = domStatus.vgMdUtilization + lastcheck = '%.1f' % (statsGenTime - domStatus.lastCheck) + + repoStats[sdUUID] = { + 'finish': domStatus.lastCheck, + + 'result': { + 'code': code, + 'lastCheck': lastcheck, + 'delay': str(domStatus.readDelay), + 'valid': (domStatus.error is None), + 'version': domStatus.version, + }, + + 'disktotal': disktotal, + 'diskfree': diskfree, + + 'mdavalid': domStatus.vgMdHasEnoughFreeSpace, + 'mdathreshold': domStatus.vgMdFreeBelowThreashold, + 'mdasize': vgmdtotal, + 'mdafree': vgmdfree, + + 'masterValidate': { + 'mount': domStatus.masterMounted, + 'valid': domStatus.masterValid + }, + + 'isoprefix': domStatus.isoPrefix, + } + + return repoStats + @public def repoStats(self, options=None): """ @@ -3559,7 +3661,7 @@ result = {}
for p in self.pools.values(): - repo_stats = p.getRepoStats() + repo_stats = self._getRepoStats(p.domainMonitor)
for d in repo_stats: result[d] = repo_stats[d]['result'] diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 7dab69d..e0559bd 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -19,7 +19,6 @@ #
import os -import time from glob import iglob, glob import logging import threading @@ -1434,66 +1433,16 @@ return self.getMetaParam(PMDK_SPM_ID)
@unsecured - def getRepoStats(self): - #FIXME : this should actually be built in HSM - repoStats = {} - statsGenTime = time.time() - - for sdUUID in self.domainMonitor.monitoredDomains: - domStatus = self.domainMonitor.getStatus(sdUUID) - - if domStatus.error is None: - code = 0 - elif isinstance(domStatus.error, se.StorageException): - code = domStatus.error.code - else: - code = se.StorageException.code - - disktotal, diskfree = domStatus.diskUtilization - vgmdtotal, vgmdfree = domStatus.vgMdUtilization - lastcheck = '%.1f' % (statsGenTime - domStatus.lastCheck) - - repoStats[sdUUID] = { - 'finish': domStatus.lastCheck, - - 'result': { - 'code': code, - 'lastCheck': lastcheck, - 'delay': str(domStatus.readDelay), - 'valid': (domStatus.error is None), - 'version': domStatus.version, - }, - - 'disktotal': disktotal, - 'diskfree': diskfree, - - 'mdavalid': domStatus.vgMdHasEnoughFreeSpace, - 'mdathreshold': domStatus.vgMdFreeBelowThreashold, - 'mdasize': vgmdtotal, - 'mdafree': vgmdfree, - - 'masterValidate': { - 'mount': domStatus.masterMounted, - 'valid': domStatus.masterValid - }, - - 'isoprefix': domStatus.isoPrefix, - } - - return repoStats - - @unsecured def getInfo(self): """ Get storage pool info. """ - msdUUID = self.masterDomain.sdUUID - try: msdInfo = self.masterDomain.getInfo() except Exception: self.log.error("Couldn't read from master domain", exc_info=True) - raise se.StoragePoolMasterNotFound(self.spUUID, msdUUID) + raise se.StoragePoolMasterNotFound(self.spUUID, + self.masterDomain.sdUUID)
try: pmd = self._getPoolMD(self.masterDomain) @@ -1512,50 +1461,9 @@ 'pool_status': 'uninitialized', 'version': str(msdInfo['version']), 'isoprefix': '', + 'pool_status': 'connected', } - - domInfo = {} - repoStats = self.getRepoStats() - - for sdUUID, sdStatus in pmd[PMDK_DOMAINS].iteritems(): - # Return statistics for active domains only - domInfo[sdUUID] = {'status': sdStatus, 'alerts': []} - - if sdStatus != sd.DOM_ACTIVE_STATUS or sdUUID not in repoStats: - continue - - if repoStats[sdUUID]['isoprefix']: - poolInfo['isoprefix'] = repoStats[sdUUID]['isoprefix'] - - domInfo[sdUUID]['version'] = repoStats[sdUUID]['result']['version'] - - # For unreachable domains repoStats will return disktotal and - # diskfree as None. - if (repoStats[sdUUID]['disktotal'] is not None - and repoStats[sdUUID]['diskfree'] is not None): - domInfo[sdUUID]['disktotal'] = repoStats[sdUUID]['disktotal'] - domInfo[sdUUID]['diskfree'] = repoStats[sdUUID]['diskfree'] - - if not repoStats[sdUUID]['mdavalid']: - domInfo[sdUUID]['alerts'].append({ - 'code': se.SmallVgMetadata.code, - 'message': se.SmallVgMetadata.message, - }) - self.log.warning("VG %s's metadata size too small %s", - sdUUID, repoStats[sdUUID]['mdasize']) - - if not repoStats[sdUUID]['mdathreshold']: - domInfo[sdUUID]['alerts'].append({ - 'code': se.VgMetadataCriticallyFull.code, - 'message': se.VgMetadataCriticallyFull.message, - }) - self.log.warning("VG %s's metadata size exceeded critical " - "size: mdasize=%s mdafree=%s", sdUUID, - repoStats[sdUUID]['mdasize'], - repoStats[sdUUID]['mdafree']) - - poolInfo["pool_status"] = "connected" - return dict(info=poolInfo, dominfo=domInfo) + return poolInfo
@unsecured def getIsoDomain(self):
Federico Simoncelli has abandoned this change.
Change subject: Make getRepoStats() a hsm method. ......................................................................
Abandoned
vdsm-patches@lists.fedorahosted.org
-
Federico Simoncelli