Federico Simoncelli has uploaded a new change for review.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
sp: refactor out the metadata access from StoragePool
Change-Id: I75493d1db60e51cccd5231b516f963c970d24c99 Signed-off-by: Federico Simoncelli fsimonce@redhat.com --- M debian/vdsm.install M vdsm.spec.in M vdsm/storage/Makefile.am M vdsm/storage/hsm.py M vdsm/storage/sp.py A vdsm/storage/spbackends.py 6 files changed, 375 insertions(+), 280 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/32/22132/1
diff --git a/debian/vdsm.install b/debian/vdsm.install index ef46ed3..43ffd5a 100644 --- a/debian/vdsm.install +++ b/debian/vdsm.install @@ -113,6 +113,7 @@ ./usr/share/vdsm/storage/sdc.py ./usr/share/vdsm/storage/securable.py ./usr/share/vdsm/storage/sp.py +./usr/share/vdsm/storage/spbackends.py ./usr/share/vdsm/storage/storageConstants.py ./usr/share/vdsm/storage/storageServer.py ./usr/share/vdsm/storage/storage_exception.py diff --git a/vdsm.spec.in b/vdsm.spec.in index f248b74..2a70661 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -926,6 +926,7 @@ %{_datadir}/%{vdsm_name}/storage/sd.py* %{_datadir}/%{vdsm_name}/storage/securable.py* %{_datadir}/%{vdsm_name}/storage/sp.py* +%{_datadir}/%{vdsm_name}/storage/spbackends.py* %{_datadir}/%{vdsm_name}/storage/storageConstants.py* %{_datadir}/%{vdsm_name}/storage/storage_exception.py* %{_datadir}/%{vdsm_name}/storage/storage_mailbox.py* diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am index 2d3e9a6..09b40ad 100644 --- a/vdsm/storage/Makefile.am +++ b/vdsm/storage/Makefile.am @@ -57,6 +57,7 @@ sd.py \ securable.py \ sp.py \ + spbackends.py \ storageConstants.py \ storage_exception.py \ storage_mailbox.py \ diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 2ae508e..7ec4345 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -40,6 +40,7 @@
from vdsm.config import config import sp +import spbackends import domainMonitor import sd import blockSD @@ -614,7 +615,7 @@ def _getSpmStatusInfo(pool): return dict( zip(('spmStatus', 'spmLver', 'spmId'), - (pool.spmRole,) + pool.getSpmStatus())) + (pool.spmRole,) + pool.backend.getSpmStatus()))
@public def getSpmStatus(self, spUUID, options=None): @@ -1035,6 +1036,7 @@ return True
pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) + pool.backend = spbackends.StoragePoolMetaBackend(pool)
# Must register domain state change callbacks *before* connecting # the pool, which starts domain monitor threads. Otherwise we will @@ -1820,8 +1822,8 @@ except: domDict[d] = sd.validateSDDeprecatedStatus(status)
- return pool.reconstructMaster(hostId, poolName, masterDom, domDict, - masterVersion, leaseParams) + return pool.backend.reconstructMaster( + hostId, poolName, masterDom, domDict, masterVersion, leaseParams)
def _logResp_getDeviceList(self, response): logableDevs = deepcopy(response) diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index d550652..ce60b55 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -41,7 +41,6 @@ from vdsm.config import config from sdc import sdCache import storage_exception as se -from persistentDict import DictValidator, unicodeEncoder, unicodeDecoder from remoteFileHandler import Timeout from securable import secured, unsecured import image @@ -52,14 +51,6 @@ import mount
POOL_MASTER_DOMAIN = 'mastersd' - -MAX_POOL_DESCRIPTION_SIZE = 50 - -PMDK_DOMAINS = "POOL_DOMAINS" -PMDK_POOL_DESCRIPTION = "POOL_DESCRIPTION" -PMDK_LVER = "POOL_SPM_LVER" -PMDK_SPM_ID = "POOL_SPM_ID" -PMDK_MASTER_VER = "MASTER_VERSION"
rmanager = rm.ResourceManager.getInstance()
@@ -83,21 +74,6 @@ k, v = domDecl.split(':') domList[k.strip("'")] = v.strip("'").capitalize() return domList - -SP_MD_FIELDS = { - # Key dec, enc - PMDK_DOMAINS: (domainListDecoder, domainListEncoder), - PMDK_POOL_DESCRIPTION: (unicodeDecoder, unicodeEncoder), - PMDK_LVER: (int, str), - PMDK_SPM_ID: (int, str), - PMDK_MASTER_VER: (int, str) -} - -# Calculate how many domains can be in the pool before overflowing the Metadata -MAX_DOMAINS = blockSD.SD_METADATA_SIZE - blockSD.METADATA_BASE_SIZE -MAX_DOMAINS -= MAX_POOL_DESCRIPTION_SIZE + sd.MAX_DOMAIN_DESCRIPTION_SIZE -MAX_DOMAINS -= blockSD.PVS_METADATA_SIZE -MAX_DOMAINS /= 48
@secured @@ -130,65 +106,14 @@ self.domainMonitor = domainMonitor self._upgradeCallback = partial(StoragePool._upgradePoolDomain, proxy(self)) + self.backend = None
def isSafe(self): return self._safety.isSet()
- @unsecured - def getSpmStatus(self): - try: - # XXX: in case the host id is not acquired yet we won't be - # able to get the spm id (I should verify this) and the code - # below would return SPM_ID_FREE. If that's the case and this - # new behavior introduces a problem then we should prepend: - # self.masterDomain.acquireHostId(self.id) - # that could take a long time. - lVer, spmId = self.masterDomain.inquireClusterLock() - lVer, spmId = lVer or LVER_INVALID, spmId or SPM_ID_FREE - except NotImplementedError: - poolMeta = self._getPoolMD(self.masterDomain) - - # if we claim that we were the SPM (but we're currently not) we - # have to make sure that we're not returning stale data - if (poolMeta[PMDK_SPM_ID] == self.id - and not self.spmRole == SPM_ACQUIRED): - self.invalidateMetadata() - poolMeta = self._getPoolMD(self.masterDomain) - - lVer, spmId = poolMeta[PMDK_LVER], poolMeta[PMDK_SPM_ID] - - return lVer, spmId - - def setSpmStatus(self, lVer=None, spmId=None): - self.invalidateMetadata() - metaParams = dict(filter(lambda (k, v): v is not None, - ((PMDK_LVER, lVer), (PMDK_SPM_ID, spmId)))) - # this method must be secured (as it changes the pool metadata), - # but since it is also used during the SPM status transition by - # default we override the security for setMetaParams. - # NOTE: this introduces a race when the method is used in the - # secured mode, but generally you shouldn't need to call this at - # any time. - self.setMetaParams(metaParams, __securityOverride=True) - - @unsecured - def getDomainsMap(self): - self.invalidateMetadata() - return self.getMetaParam(PMDK_DOMAINS) - - def setDomainsMap(self, domains): - self.setMetaParam(PMDK_DOMAINS, domains) - def __del__(self): if len(self.domainMonitor.poolMonitoredDomains) > 0: threading.Thread(target=self.stopMonitoringDomains).start() - - @unsecured - def forceFreeSpm(self): - # DO NOT USE, STUPID, HERE ONLY FOR BC - # TODO: SCSI Fence the 'lastOwner' - self.setSpmStatus(LVER_INVALID, SPM_ID_FREE) - self.spmRole = SPM_FREE
def _upgradePoolDomain(self, sdUUID, isValid): # This method is called everytime the onDomainStateChange @@ -260,7 +185,7 @@ continue
try: - self.setDomainMasterRole(domain, sd.REGULAR_DOMAIN, 0) + self.backend.setDomainMasterRole(domain, sd.REGULAR_DOMAIN, 0) except: self.log.exception('Unable to set role for domain %s', sdUUID)
@@ -292,7 +217,7 @@ raise se.OperationInProgress("spm start %s" % self.spUUID)
self.updateMonitoringThreads() - oldlver, oldid = self.getSpmStatus() + oldlver, oldid = self.backend.getSpmStatus() masterDomVersion = self.getVersion() # If no specific domain version was specified use current master # domain version @@ -324,7 +249,8 @@ try: self.lver = int(oldlver) + 1
- self.setSpmStatus(self.lver, self.id, __securityOverride=True) + self.backend.setSpmStatus(self.lver, self.id, + __securityOverride=True) self._maxHostID = maxHostID
# Upgrade the master domain now if needed @@ -460,8 +386,8 @@
if not stopFailed: try: - self.setSpmStatus(spmId=SPM_ID_FREE, - __securityOverride=True) + self.backend.setSpmStatus(spmId=SPM_ID_FREE, + __securityOverride=True) except: pass # The system can handle this inconsistency
@@ -532,22 +458,6 @@ # Cleanup links to domains under /rhev/datacenter/poolName self.refresh(msdUUID, masterVersion)
- @unsecured - def getMasterVersion(self, useMasterDomain=None): - domain = (self.masterDomain - if useMasterDomain is None else useMasterDomain) - return self._getPoolMD(domain)[PMDK_MASTER_VER] - - def setDomainMasterRole(self, domain, role, masterVersion): - poolMeta = self._getPoolMD(domain) - # NOTE: the transaction here does not ensure the consistency between - # the domain and pool metadata. For example if the role on the domain - # has been changed and the pool metadata transaction fails then the - # domain role is not reverted to the previous value. - with poolMeta.transaction(): - poolMeta[PMDK_MASTER_VER] = masterVersion - domain.changeRole(role) - # TODO: Remove or rename this function. def validatePoolSD(self, sdUUID): if sdUUID not in self.getDomains(): @@ -564,17 +474,6 @@ if self.spUUID not in dom.getPools(): raise se.StorageDomainNotInPool(self.spUUID, dom.sdUUID) return True - - @unsecured - def getMaximumSupportedDomains(self): - msdInfo = self.masterDomain.getInfo() - msdType = sd.name2type(msdInfo["type"]) - msdVersion = int(msdInfo["version"]) - if msdType in sd.BLOCK_DOMAIN_TYPES and \ - msdVersion in blockSD.VERS_METADATA_LV: - return MAX_DOMAINS - else: - return config.getint("irs", "maximum_domains_in_pool")
@unsecured def _acquireTemporaryClusterLock(self, msdUUID, leaseParams): @@ -805,80 +704,13 @@ if not misc.isAscii(poolName) and not domain.supportsUnicode(): raise se.UnicodeArgumentException()
- futurePoolMD.update({ - PMDK_SPM_ID: SPM_ID_FREE, - PMDK_LVER: LVER_INVALID, - PMDK_MASTER_VER: masterVersion, - PMDK_POOL_DESCRIPTION: poolName, - PMDK_DOMAINS: {domain.sdUUID: sd.DOM_ACTIVE_STATUS}}) - - @unsecured - def reconstructMaster(self, hostId, poolName, msdUUID, domDict, - masterVersion, leaseParams): - self.log.info("spUUID=%s hostId=%s poolName=%s msdUUID=%s domDict=%s " - "masterVersion=%s leaseparams=(%s)", self.spUUID, hostId, - poolName, msdUUID, domDict, masterVersion, leaseParams) - - if msdUUID not in domDict: - raise se.InvalidParameterException("masterDomain", msdUUID) - - futureMaster = sdCache.produce(msdUUID) - - # @deprecated, domain version < 3 - # For backward compatibility we must support a reconstructMaster - # that doesn't specify an hostId. - if not hostId: - self._acquireTemporaryClusterLock(msdUUID, leaseParams) - temporaryLock = True - else: - # Forcing to acquire the host id (if it's not acquired already). - futureMaster.acquireHostId(hostId) - futureMaster.acquireClusterLock(hostId) - - # The host id must be set for createMaster(...). - self.id = hostId - temporaryLock = False - - try: - self.createMaster(poolName, futureMaster, masterVersion, - leaseParams) - - for sdUUID in domDict: - domDict[sdUUID] = domDict[sdUUID].capitalize() - - # Add domain to domain list in pool metadata. - self.log.info("Set storage pool domains: %s", domDict) - self._getPoolMD(futureMaster).update({PMDK_DOMAINS: domDict}) - - self.refresh(msdUUID=msdUUID, masterVersion=masterVersion) - finally: - if temporaryLock: - self._releaseTemporaryClusterLock(msdUUID) - self.stopMonitoringDomains() - else: - futureMaster.releaseClusterLock() - - @unsecured - def copyPoolMD(self, prevMd, newMD): - prevPoolMD = self._getPoolMD(prevMd) - domains = prevPoolMD[PMDK_DOMAINS] - pool_descr = prevPoolMD[PMDK_POOL_DESCRIPTION] - lver = prevPoolMD[PMDK_LVER] - spmId = prevPoolMD[PMDK_SPM_ID] - # This is actually domain metadata, But I can't change this because of - # backward compatibility - leaseParams = prevMd.getLeaseParams() - - # Now insert pool metadata into new mastersd metadata - - newPoolMD = self._getPoolMD(newMD) - with newPoolMD.transaction(): - newPoolMD.update({ - PMDK_DOMAINS: domains, - PMDK_POOL_DESCRIPTION: pool_descr, - PMDK_LVER: lver, - PMDK_SPM_ID: spmId}) - newMD.changeLeaseParams(leaseParams) +# FIXME !!!!!!!!!! +# futurePoolMD.update({ +# PMDK_SPM_ID: SPM_ID_FREE, +# PMDK_LVER: LVER_INVALID, +# PMDK_MASTER_VER: masterVersion, +# PMDK_POOL_DESCRIPTION: poolName, +# PMDK_DOMAINS: {domain.sdUUID: sd.DOM_ACTIVE_STATUS}})
@unsecured def _copyLeaseParameters(self, srcDomain, dstDomain): @@ -893,7 +725,7 @@ msdUUID)
# TODO: is this check still relevant? - if not masterVersion > self.getMasterVersion(): + if not masterVersion > self.backend.getMasterVersion(): raise se.StoragePoolWrongMaster(self.spUUID, self.masterDomain.sdUUID)
@@ -938,7 +770,7 @@ self.log.error("Unexpected error", exc_info=True) raise se.StorageDomainMasterCopyError(msdUUID)
- self.copyPoolMD(curmsd, newmsd) + self.backend.prepareNewMasterDomain(curmsd, newmsd)
path = newmsd.getMDPath() if not path: @@ -968,7 +800,8 @@ self.log.debug("masterMigrate - lease acquired successfully")
try: - self.setDomainMasterRole(newmsd, sd.MASTER_DOMAIN, masterVersion) + self.backend.setDomainMasterRole( + newmsd, sd.MASTER_DOMAIN, masterVersion) self.savePoolParams(self.id, newmsd.sdUUID, masterVersion) except Exception: self.log.error("Unexpected error", exc_info=True) @@ -1014,7 +847,7 @@ if sdUUID in domains: return True
- if len(domains) >= self.getMaximumSupportedDomains(): + if len(domains) >= self.backend.getMaximumSupportedDomains(): raise se.TooManyDomainsInStoragePoolError()
try: @@ -1040,7 +873,7 @@
dom.attach(self.spUUID) domains[sdUUID] = sd.DOM_ATTACHED_STATUS - self.setDomainsMap(domains) + self.backend.setDomainsMap(domains) self._refreshDomainLinks(dom)
finally: @@ -1060,7 +893,7 @@
del domains[sdUUID]
- self.setDomainsMap(domains) + self.backend.setDomainsMap(domains) self._cleanupDomainLinks(sdUUID)
# If the domain that we are detaching is the master domain @@ -1110,6 +943,23 @@ # Remove domain from pool metadata self.forcedDetachSD(sdUUID)
+ def detachAllDomains(self): + """ + Detach all domains from pool before destroying pool + + Assumed cluster lock and that SPM is already stopped. + """ + # Find regular (i.e. not master) domains from the pool metadata + regularDoms = tuple(sdUUID for sdUUID in self.getDomains() + if sdUUID != self.masterDomain.sdUUID) + # The Master domain should be detached last + for sdUUID in regularDoms: + self.detachSD(sdUUID) + + # Forced detach master domain + self.forcedDetachSD(self.masterDomain.sdUUID) + self.masterDomain.detach(self.spUUID) + @unsecured def _convertDomain(self, domain, targetFormat=None): # Remember to get the sdUUID before upgrading because the object is @@ -1149,7 +999,7 @@
# Domain conversion requires the links to be present self._refreshDomainLinks(dom) - self.setDomainMasterRole(dom, sd.REGULAR_DOMAIN, 0) + self.backend.setDomainMasterRole(dom, sd.REGULAR_DOMAIN, 0)
if dom.getDomainClass() == sd.DATA_DOMAIN: self._convertDomain(dom) @@ -1157,7 +1007,7 @@ dom.activate() # set domains also do rebuild domainStatuses[sdUUID] = sd.DOM_ACTIVE_STATUS - self.setDomainsMap(domainStatuses) + self.backend.setDomainsMap(domainStatuses) self.updateMonitoringThreads() return True
@@ -1220,7 +1070,7 @@ "%s", masterDir, dom)
domList[sdUUID] = sd.DOM_ATTACHED_STATUS - self.setDomainsMap(domList) + self.backend.setDomainsMap(domList) self.updateMonitoringThreads()
@unsecured @@ -1393,21 +1243,6 @@ if os.path.exists(os.path.join(vms, vmUUID)): fileUtils.cleanupdir(os.path.join(vms, vmUUID))
- def setDescription(self, descr): - """ - Set storage pool description. - 'descr' - pool description - """ - if len(descr) > MAX_POOL_DESCRIPTION_SIZE: - raise se.StoragePoolDescriptionTooLongError() - - self.log.info("spUUID=%s descr=%s", self.spUUID, descr) - - if not misc.isAscii(descr) and not self.masterDomain.supportsUnicode(): - raise se.UnicodeArgumentException() - - self.setMetaParam(PMDK_POOL_DESCRIPTION, descr) - def extendVolume(self, sdUUID, volumeUUID, size, isShuttingDown=None): # This method is not exposed through the remote API but it's called # directly from the mailbox to implement the thin provisioning on @@ -1423,27 +1258,6 @@ rm.LockType.exclusive): return sdCache.produce(sdUUID) \ .produceVolume(imgUUID, volUUID).extendSize(int(newSize)) - - @classmethod - def _getPoolMD(cls, domain): - # This might look disgusting but this makes it so that - # This is the only intrusion needed to satisfy the - # unholy union between pool and SD metadata - return DictValidator(domain._metadata._dict, SP_MD_FIELDS) - - @property - def _metadata(self): - return self._getPoolMD(self.masterDomain) - - @unsecured - def getDescription(self): - try: - return self.getMetaParam(PMDK_POOL_DESCRIPTION) - # There was a bug that cause pool description to - # disappear. Returning "" might be ugly but it keeps - # everyone happy. - except KeyError: - return ""
@unsecured def getVersion(self): @@ -1461,25 +1275,22 @@ raise se.StoragePoolMasterNotFound(self.spUUID, self.masterDomain.sdUUID)
- try: - pmd = self._getPoolMD(self.masterDomain) - except Exception: - self.log.error("Pool metadata error", exc_info=True) - raise se.StoragePoolActionError(self.spUUID) + lVer, spmId = self.backend.getSpmStatus()
poolInfo = { 'type': msdInfo['type'], - 'name': pmd[PMDK_POOL_DESCRIPTION], - 'domains': domainListEncoder(pmd[PMDK_DOMAINS]), + 'name': '', + 'domains': domainListEncoder(self.backend.getDomainsMap()), 'master_uuid': self.masterDomain.sdUUID, - 'master_ver': pmd[PMDK_MASTER_VER], - 'lver': pmd[PMDK_LVER], - 'spm_id': pmd[PMDK_SPM_ID], + 'master_ver': self.backend.getMasterVersion(), + 'lver': lVer, + 'spm_id': spmId, 'pool_status': 'uninitialized', 'version': str(msdInfo['version']), 'isoprefix': '', 'pool_status': 'connected', } + return poolInfo
@unsecured @@ -1498,22 +1309,6 @@ if dom.isISO(): return dom return None - - def setMetaParams(self, params): - self._metadata.update(params) - - def setMetaParam(self, key, value): - """ - Set key:value in pool metadata file - """ - self._metadata[key] = value - - @unsecured - def getMetaParam(self, key): - """ - Get parameter from pool metadata file - """ - return self._metadata[key]
@unsecured def setMasterDomain(self, msdUUID, masterVersion): @@ -1540,7 +1335,7 @@ " %s", msdUUID, self.spUUID) raise se.StoragePoolWrongMaster(self.spUUID, msdUUID)
- version = self.getMasterVersion(useMasterDomain=domain) + version = self.backend.getMasterVersion(useMasterDomain=domain) if version != int(masterVersion): self.log.error("Requested master domain %s does not have expected " "version %s it is version %s", @@ -1551,11 +1346,6 @@ masterVersion) self.masterDomain = domain self.updateMonitoringThreads() - - @unsecured - def invalidateMetadata(self): - if not self.spmRole == SPM_ACQUIRED: - self._metadata.invalidate()
@unsecured @misc.samplingmethod @@ -1589,7 +1379,7 @@ @unsecured def getDomains(self, activeOnly=False): return dict((sdUUID, status) for sdUUID, status - in self.getDomainsMap().iteritems() + in self.backend.getDomainsMap().iteritems() if not activeOnly or status == sd.DOM_ACTIVE_STATUS)
def checkBackupDomain(self): @@ -1975,23 +1765,6 @@ self._maxHostID self.spmMailer.setMaxHostID(maxID) raise se.NotImplementedException - - def detachAllDomains(self): - """ - Detach all domains from pool before destroying pool - - Assumed cluster lock and that SPM is already stopped. - """ - # Find regular (i.e. not master) domains from the pool metadata - regularDoms = tuple(sdUUID for sdUUID in self.getDomains() - if sdUUID != self.masterDomain.sdUUID) - # The Master domain should be detached last - for sdUUID in regularDoms: - self.detachSD(sdUUID) - - # Forced detach master domain - self.forcedDetachSD(self.masterDomain.sdUUID) - self.masterDomain.detach(self.spUUID)
def setVolumeDescription(self, sdUUID, imgUUID, volUUID, description): self.validatePoolSD(sdUUID) diff --git a/vdsm/storage/spbackends.py b/vdsm/storage/spbackends.py new file mode 100644 index 0000000..e0f8013 --- /dev/null +++ b/vdsm/storage/spbackends.py @@ -0,0 +1,317 @@ +# +# Copyright 2013 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# + +import logging +import weakref + +import blockSD +import misc +import sd +import storage_exception as se + +from persistentDict import DictValidator +from persistentDict import unicodeDecoder +from persistentDict import unicodeEncoder +from sdc import sdCache +from securable import secured +from securable import unsecured +from sp import LVER_INVALID +from sp import SPM_ACQUIRED +from sp import SPM_FREE +from sp import SPM_ID_FREE +from sp import domainListDecoder +from sp import domainListEncoder +from vdsm.config import config + + +MAX_POOL_DESCRIPTION_SIZE = 50 + +PMDK_DOMAINS = "POOL_DOMAINS" +PMDK_POOL_DESCRIPTION = "POOL_DESCRIPTION" +PMDK_LVER = "POOL_SPM_LVER" +PMDK_SPM_ID = "POOL_SPM_ID" +PMDK_MASTER_VER = "MASTER_VERSION" + + +# Calculate how many domains can be in the pool before overflowing the Metadata +MAX_DOMAINS = blockSD.SD_METADATA_SIZE - blockSD.METADATA_BASE_SIZE +MAX_DOMAINS -= MAX_POOL_DESCRIPTION_SIZE + sd.MAX_DOMAIN_DESCRIPTION_SIZE +MAX_DOMAINS -= blockSD.PVS_METADATA_SIZE +MAX_DOMAINS /= 48 + + +SP_MD_FIELDS = { + # Key dec, enc + PMDK_DOMAINS: (domainListDecoder, domainListEncoder), + PMDK_POOL_DESCRIPTION: (unicodeDecoder, unicodeEncoder), + PMDK_LVER: (int, str), + PMDK_SPM_ID: (int, str), + PMDK_MASTER_VER: (int, str) +} + + +@secured +class StoragePoolMetaBackend(object): + + __slots__ = ('pool',) + + log = logging.getLogger('Storage.StoragePoolMetaBackend') + + def __init__(self, pool): + self.pool = weakref.proxy(pool) + + ### Read-Only StoragePool Object Accessors ### + + def isSafe(self): + return self.pool.isSafe() + + @property + def id(self): + return self.pool.id + + @property + def spmRole(self): + return self.pool.spmRole + + @property + def spUUID(self): + return self.pool.spUUID + + @property + def masterDomain(self): + return self.pool.masterDomain + + ### StoragePool Abstract Methods Implementation ### + + @unsecured + def getSpmStatus(self): + try: + # XXX: in case the host id is not acquired yet we won't be + # able to get the spm id (I should verify this) and the code + # below would return SPM_ID_FREE. If that's the case and this + # new behavior introduces a problem then we should prepend: + # self.masterDomain.acquireHostId(self.id) + # that could take a long time. + lVer, spmId = self.masterDomain.inquireClusterLock() + lVer, spmId = lVer or LVER_INVALID, spmId or SPM_ID_FREE + except NotImplementedError: + poolMeta = self._getPoolMD(self.masterDomain) + + # if we claim that we were the SPM (but we're currently not) we + # have to make sure that we're not returning stale data + if (poolMeta[PMDK_SPM_ID] == self.id + and not self.spmRole == SPM_ACQUIRED): + self.invalidateMetadata() + poolMeta = self._getPoolMD(self.masterDomain) + + lVer, spmId = poolMeta[PMDK_LVER], poolMeta[PMDK_SPM_ID] + + return lVer, spmId + + def setSpmStatus(self, lVer=None, spmId=None): + self.invalidateMetadata() + metaParams = dict(filter(lambda (k, v): v is not None, + ((PMDK_LVER, lVer), (PMDK_SPM_ID, spmId)))) + # this method must be secured (as it changes the pool metadata), + # but since it is also used during the SPM status transition by + # default we override the security for setMetaParams. + # NOTE: this introduces a race when the method is used in the + # secured mode, but generally you shouldn't need to call this at + # any time. + self.setMetaParams(metaParams, __securityOverride=True) + + @unsecured + def getDomainsMap(self): + self.invalidateMetadata() + return self.getMetaParam(PMDK_DOMAINS) + + def setDomainsMap(self, domains): + self.setMetaParam(PMDK_DOMAINS, domains) + + @unsecured + def getMaximumSupportedDomains(self): + msdInfo = self.masterDomain.getInfo() + msdType = sd.name2type(msdInfo["type"]) + msdVersion = int(msdInfo["version"]) + if msdType in sd.BLOCK_DOMAIN_TYPES and \ + msdVersion in blockSD.VERS_METADATA_LV: + return MAX_DOMAINS + else: + return config.getint("irs", "maximum_domains_in_pool") + + @unsecured + def getMasterVersion(self, useMasterDomain=None): + domain = (self.masterDomain + if useMasterDomain is None else useMasterDomain) + return self._getPoolMD(domain)[PMDK_MASTER_VER] + + def setDomainMasterRole(self, domain, role, masterVersion): + poolMeta = self._getPoolMD(domain) + # NOTE: the transaction here does not ensure the consistency between + # the domain and pool metadata. For example if the role on the domain + # has been changed and the pool metadata transaction fails then the + # domain role is not reverted to the previous value. + with poolMeta.transaction(): + poolMeta[PMDK_MASTER_VER] = masterVersion + domain.changeRole(role) + + @unsecured + def prepareNewMasterDomain(self, prevMd, newMD): + prevPoolMD = self._getPoolMD(prevMd) + domains = prevPoolMD[PMDK_DOMAINS] + pool_descr = prevPoolMD[PMDK_POOL_DESCRIPTION] + lver = prevPoolMD[PMDK_LVER] + spmId = prevPoolMD[PMDK_SPM_ID] + # This is actually domain metadata, But I can't change this because of + # backward compatibility + leaseParams = prevMd.getLeaseParams() + + # Now insert pool metadata into new mastersd metadata + + newPoolMD = self._getPoolMD(newMD) + with newPoolMD.transaction(): + newPoolMD.update({ + PMDK_DOMAINS: domains, + PMDK_POOL_DESCRIPTION: pool_descr, + PMDK_LVER: lver, + PMDK_SPM_ID: spmId}) + newMD.changeLeaseParams(leaseParams) + + ### StoragePool Overridden Methods ### + + @unsecured + def getInfo(self): + poolInfo = super(StoragePoolMetaBackend, self).getInfo() + + # XXX: in the previous implementation the + poolInfo.update({'name': self.getDescription()}) + return poolInfo + + ### Backend Specific Methods ### + + @unsecured + def forceFreeSpm(self): + # DO NOT USE, STUPID, HERE ONLY FOR BC + # TODO: SCSI Fence the 'lastOwner' + self.setSpmStatus(LVER_INVALID, SPM_ID_FREE) + self.spmRole = SPM_FREE + + @classmethod + def _getPoolMD(cls, domain): + # This might look disgusting but this makes it so that + # This is the only intrusion needed to satisfy the + # unholy union between pool and SD metadata + return DictValidator(domain._metadata._dict, SP_MD_FIELDS) + + @property + def _metadata(self): + return self._getPoolMD(self.masterDomain) + + @unsecured + def getMetaParam(self, key): + """ + Get parameter from pool metadata file + """ + return self._metadata[key] + + def setMetaParams(self, params): + self._metadata.update(params) + + def setMetaParam(self, key, value): + """ + Set key:value in pool metadata file + """ + self._metadata[key] = value + + @unsecured + def getDescription(self): + try: + return self.getMetaParam(PMDK_POOL_DESCRIPTION) + # There was a bug that cause pool description to + # disappear. Returning "" might be ugly but it keeps + # everyone happy. + except KeyError: + return "" + + def setDescription(self, descr): + """ + Set storage pool description. + 'descr' - pool description + """ + if len(descr) > MAX_POOL_DESCRIPTION_SIZE: + raise se.StoragePoolDescriptionTooLongError() + + self.log.info("spUUID=%s descr=%s", self.spUUID, descr) + + if not misc.isAscii(descr) and not self.masterDomain.supportsUnicode(): + raise se.UnicodeArgumentException() + + self.setMetaParam(PMDK_POOL_DESCRIPTION, descr) + + @unsecured + def invalidateMetadata(self): + if not self.spmRole == SPM_ACQUIRED: + self._metadata.invalidate() + + @unsecured + def reconstructMaster(self, hostId, poolName, msdUUID, domDict, + masterVersion, leaseParams): + self.log.info("spUUID=%s hostId=%s poolName=%s msdUUID=%s domDict=%s " + "masterVersion=%s leaseparams=(%s)", self.spUUID, hostId, + poolName, msdUUID, domDict, masterVersion, leaseParams) + + if msdUUID not in domDict: + raise se.InvalidParameterException("masterDomain", msdUUID) + + futureMaster = sdCache.produce(msdUUID) + + # @deprecated, domain version < 3 + # For backward compatibility we must support a reconstructMaster + # that doesn't specify an hostId. + if not hostId: + self.pool._acquireTemporaryClusterLock(msdUUID, leaseParams) + temporaryLock = True + else: + # Forcing to acquire the host id (if it's not acquired already). + futureMaster.acquireHostId(hostId) + futureMaster.acquireClusterLock(hostId) + + # The host id must be set for createMaster(...). + self.id = hostId + temporaryLock = False + + try: + self.pool.createMaster(poolName, futureMaster, masterVersion, + leaseParams) + + for sdUUID in domDict: + domDict[sdUUID] = domDict[sdUUID].capitalize() + + # Add domain to domain list in pool metadata. + self.log.info("Set storage pool domains: %s", domDict) + self._getPoolMD(futureMaster).update({PMDK_DOMAINS: domDict}) + + self.pool.refresh(msdUUID=msdUUID, masterVersion=masterVersion) + finally: + if temporaryLock: + self.pool._releaseTemporaryClusterLock(msdUUID) + self.pool.stopMonitoringDomains() + else: + futureMaster.releaseClusterLock()
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5960/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5164/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/93/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6052/ : SUCCESS
Federico Simoncelli has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 1: Code-Review-1
Outdated, don't review.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 2: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6642/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5749/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6555/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/219/ : FAILURE
Ayal Baron has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 2:
(3 comments)
http://gerrit.ovirt.org/#/c/22132/2/vdsm/storage/hsm.py File vdsm/storage/hsm.py:
Line 600: @staticmethod Line 601: def _getSpmStatusInfo(pool): Line 602: return dict( Line 603: zip(('spmStatus', 'spmLver', 'spmId'), Line 604: (pool.spmRole,) + pool.getBackend().getSpmStatus())) why is getSpmStatus not wrapped by the pool? (i.e. instead of calling getBackend here, leave as is and expose getSpmStatus in pool object Line 605: Line 606: @public Line 607: def getSpmStatus(self, spUUID, options=None): Line 608: pool = self.getPool(spUUID)
Line 1035: self._updateStoragePool(pool, hostID, msdUUID, masterVersion) Line 1036: return True Line 1037: Line 1038: pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) Line 1039: pool.backend = spbackends.StoragePoolMetaBackend(pool) why is this not set in __init__? (see same comment in next patch since once domainsMap is passed then it is easy to pass that to pool object ctor and determine backend type accordingly Line 1040: Line 1041: # Must register domain state change callbacks *before* connecting Line 1042: # the pool, which starts domain monitor threads. Otherwise we will Line 1043: # miss the first event from the monitor thread.
Line 1821: sd.validateSDStatus(status) Line 1822: except: Line 1823: domDict[d] = sd.validateSDDeprecatedStatus(status) Line 1824: Line 1825: return pool.getBackend().reconstructMaster( again, expose reconstructMaster in pool object? Line 1826: hostId, poolName, masterDom, domDict, masterVersion, leaseParams) Line 1827: Line 1828: def _logResp_getDeviceList(self, response): Line 1829: logableDevs = deepcopy(response)
Nir Soffer has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 2:
(4 comments)
This patch should move some methods to the backend, keeping the pool api without change, except private methods. But is looks like this patch do change pool public api.
If some pool apis are public now, but should be private, the change should be separate from the backend patch.
http://gerrit.ovirt.org/#/c/22132/2/vdsm/storage/hsm.py File vdsm/storage/hsm.py:
Line 600: @staticmethod Line 601: def _getSpmStatusInfo(pool): Line 602: return dict( Line 603: zip(('spmStatus', 'spmLver', 'spmId'), Line 604: (pool.spmRole,) + pool.getBackend().getSpmStatus()))
why is getSpmStatus not wrapped by the pool? (i.e. instead of calling getBa
+1 for hiding the backend behind the pool interface. Line 605: Line 606: @public Line 607: def getSpmStatus(self, spUUID, options=None): Line 608: pool = self.getPool(spUUID)
Line 1035: self._updateStoragePool(pool, hostID, msdUUID, masterVersion) Line 1036: return True Line 1037: Line 1038: pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) Line 1039: pool.backend = spbackends.StoragePoolMetaBackend(pool)
why is this not set in __init__? (see same comment in next patch since once
We have circular reference here, which makes it hard to create these objects.
It would be nice if the public object (pool), is created with the internal object (backend), and in pool.__init__ we can attach them togehter using:
self.backend.pool = self Line 1040: Line 1041: # Must register domain state change callbacks *before* connecting Line 1042: # the pool, which starts domain monitor threads. Otherwise we will Line 1043: # miss the first event from the monitor thread.
Line 1821: sd.validateSDStatus(status) Line 1822: except: Line 1823: domDict[d] = sd.validateSDDeprecatedStatus(status) Line 1824: Line 1825: return pool.getBackend().reconstructMaster(
again, expose reconstructMaster in pool object?
+1 Line 1826: hostId, poolName, masterDom, domDict, masterVersion, leaseParams) Line 1827: Line 1828: def _logResp_getDeviceList(self, response): Line 1829: logableDevs = deepcopy(response)
http://gerrit.ovirt.org/#/c/22132/2/vdsm/storage/spbackends.py File vdsm/storage/spbackends.py:
Line 74: Line 75: log = logging.getLogger('Storage.StoragePoolMetaBackend') Line 76: Line 77: def __init__(self, pool): Line 78: self.pool = weakref.proxy(pool) If we set the pool after the object is created, we remove the dependency on the pool and make object creation easier.
We can set the pool by:
@property def pool(self): return self._pool
@pool.setter def pooll(sefl, value): self._pool = weakref.proxy(value) Line 79: Line 80: ### Read-Only StoragePool Object Accessors ### Line 81: Line 82: def isSafe(self):
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 3: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5946/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/272/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6839/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6734/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests/22/ : FAILURE
Ayal Baron has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 3: Code-Review+2
Federico Simoncelli has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 4: Verified+1
Verified: createStoragePool, several attach/detachStorageDomain, destroyStoragePool, reconstructMaster and masterMigrate.
Dan Kenigsberg has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 4: Code-Review+2
Copying Ayal's score - only change is taking MAX_POOL_DESCRIPTION_SIZE from spbackend.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
sp: refactor out the metadata access from StoragePool
All the storage pool metadata operations have been relocated to a specific backend: StoragePoolDiskBackend. It is now possible, implementing the required API, to use different backends for the pool metadata.
A backend may require other attributes and methods in addition to the regular API used by StoragePool, these specific features should be handled outside of the StoragePool implementation, typically in the HSM class where the backward compatibility layer is maintained.
Change-Id: I75493d1db60e51cccd5231b516f963c970d24c99 Signed-off-by: Federico Simoncelli fsimonce@redhat.com Reviewed-on: http://gerrit.ovirt.org/22132 Reviewed-by: Dan Kenigsberg danken@redhat.com --- M debian/vdsm.install M vdsm.spec.in M vdsm/storage/Makefile.am M vdsm/storage/hsm.py M vdsm/storage/sp.py A vdsm/storage/spbackends.py 6 files changed, 458 insertions(+), 223 deletions(-)
Approvals: Federico Simoncelli: Verified Dan Kenigsberg: Looks good to me, approved
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 4:
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/5970/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/280/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/6758/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/6864/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_storage_functional_tests/33/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sp: refactor out the metadata access from StoragePool ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_install_rpm_sanity_gerrit/281/ : SUCCESS
vdsm-patches@lists.fedorahosted.org