vdsm-patches December 2013

vdsm-patches@lists.fedorahosted.org

48 participants
493 discussions

Change in vdsm[master]: sp: [wip] receive domains map in connectStoragePool
by Federico Simoncelli 20 Jan '14

20 Jan '14

Federico Simoncelli has uploaded a new change for review. Change subject: sp: [wip] receive domains map in connectStoragePool ...................................................................... sp: [wip] receive domains map in connectStoragePool Change-Id: I393677f643a62e3711af2a3cfb8b4b9a5ce11c2d Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com> --- M vdsm/API.py M vdsm/BindingXMLRPC.py M vdsm/storage/hsm.py M vdsm/storage/sp.py 4 files changed, 243 insertions(+), 135 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/27/21427/1 diff --git a/vdsm/API.py b/vdsm/API.py index f91910c..90ba002 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -958,9 +958,12 @@ APIBase.__init__(self) self._UUID = UUID - def connect(self, hostID, scsiKey, masterSdUUID, masterVersion): - return self._irs.connectStoragePool(self._UUID, hostID, scsiKey, - masterSdUUID, masterVersion) + # FIXME: WIP patch, requires backward compatibility considerations + def connect(self, hostID, scsiKey, masterSdUUID, masterVersion, + domainsMap): + return self._irs.connectStoragePool( + self._UUID, hostID, scsiKey, masterSdUUID, masterVersion, + domainsMap) def connectStorageServer(self, domainType, connectionParams): return self._irs.connectStorageServer(domainType, self._UUID, diff --git a/vdsm/BindingXMLRPC.py b/vdsm/BindingXMLRPC.py index dd82b05..a1c8817 100644 --- a/vdsm/BindingXMLRPC.py +++ b/vdsm/BindingXMLRPC.py @@ -542,10 +542,12 @@ image = API.Image(imgUUID, spUUID, sdUUID) return image.download(methodArgs, volUUID) + # FIXME: WIP patch, requires backward compatibility considerations def poolConnect(self, spUUID, hostID, scsiKey, msdUUID, masterVersion, - options=None): + domainsMap=None, options=None): pool = API.StoragePool(spUUID) - return pool.connect(hostID, scsiKey, msdUUID, masterVersion) + return pool.connect(hostID, scsiKey, msdUUID, masterVersion, + domainsMap) def poolConnectStorageServer(self, domType, spUUID, conList, options=None): pool = API.StoragePool(spUUID) diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 3707528..56b24b4 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -981,8 +981,8 @@ poolName, masterDom, domList, masterVersion, leaseParams) @public - def connectStoragePool(self, spUUID, hostID, scsiKey, - msdUUID, masterVersion, options=None): + def connectStoragePool(self, spUUID, hostID, scsiKey, msdUUID, + masterVersion, domainsMap=None, options=None): """ Connect a Host to a specific storage pool. @@ -1010,11 +1010,12 @@ hostID, scsiKey))) with rmanager.acquireResource(STORAGE, HSM_DOM_MON_LOCK, rm.LockType.exclusive): - return self._connectStoragePool(spUUID, hostID, scsiKey, msdUUID, - masterVersion, options) + return self._connectStoragePool( + spUUID, hostID, scsiKey, msdUUID, masterVersion, + domainsMap, options) def _connectStoragePool(self, spUUID, hostID, scsiKey, msdUUID, - masterVersion, options=None): + masterVersion, domainsMap=None, options=None): misc.validateUUID(spUUID, 'spUUID') # TBD: To support multiple pool connection on single host, @@ -1055,7 +1056,13 @@ masterVersion=masterVersion) return - pool = sp.StoragePool(spUUID, self.domainMonitor, self.taskMng) + if domainsMap is not None: + pool = sp.StoragePool(spUUID, self.domainMonitor, + self.taskMng, domainsMap, masterVersion) + else: + pool = sp.StoragePoolLegacy( + spUUID, self.domainMonitor, self.taskMng) + res = pool.connect(hostID, scsiKey, msdUUID, masterVersion) if res: diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 46e8224..638891f 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -38,6 +38,7 @@ import fileSD import sd import misc +from misc import deprecated import fileUtils from vdsm.config import config from sdc import sdCache @@ -101,7 +102,7 @@ MAX_DOMAINS /= 48 -class StoragePool(Securable): +class StoragePoolBase(Securable): ''' StoragePool object should be relatively cheap to construct. It should defer any heavy lifting activities until the time it is really needed. @@ -113,7 +114,7 @@ lvExtendPolicy = config.get('irs', 'vol_extend_policy') def __init__(self, spUUID, domainMonitor, taskManager): - Securable.__init__(self) + super(StoragePoolBase, self).__init__() self._formatConverter = DefaultFormatConverter() self._domainsToUpgrade = [] @@ -132,10 +133,6 @@ self.domainMonitor = domainMonitor self._upgradeCallback = partial(StoragePool._upgradePoolDomain, proxy(self)) - - @unsecured - def getSpmLver(self): - return self.getMetaParam(PMDK_LVER) @unsecured def getSpmStatus(self): @@ -279,9 +276,8 @@ self.lver = int(oldlver) + 1 self.invalidateMetadata() - self.setMetaParams({ - PMDK_LVER: self.lver, - PMDK_SPM_ID: self.id}, __securityOverride=True) + self.updateSpmInfo(self.lver, self.id) + self._maxHostID = maxHostID # Upgrade the master domain now if needed @@ -487,10 +483,6 @@ sdUUID, msdUUID, masterVersion, exc_info=True) # Cleanup links to domains under /rhev/datacenter/poolName self.refresh(msdUUID, masterVersion) - - @unsecured - def getMasterVersion(self): - return self.getMetaParam(PMDK_MASTER_VER) # TODO: Remove or rename this function. @unsecured @@ -1361,21 +1353,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 @@ -1391,45 +1368,14 @@ 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): return self.masterDomain.getVersion() @unsecured - def getSpmId(self): - spmid = self.getMetaParam(PMDK_SPM_ID) - if spmid != self.id or self.spmRole != SPM_FREE: - return spmid - - # If we claim to be the SPM we have to be really sure we are - self.invalidateMetadata() - return self.getMetaParam(PMDK_SPM_ID) - - @unsecured def getInfo(self): """ - Get storage pool info. + Get storage group info """ try: msdInfo = self.masterDomain.getInfo() @@ -1438,26 +1384,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) - - poolInfo = { - 'type': msdInfo['type'], - 'name': pmd[PMDK_POOL_DESCRIPTION], - 'domains': domainListEncoder(pmd[PMDK_DOMAINS]), - 'master_uuid': self.masterDomain.sdUUID, - 'master_ver': pmd[PMDK_MASTER_VER], - 'lver': pmd[PMDK_LVER], - 'spm_id': pmd[PMDK_SPM_ID], - 'pool_status': 'uninitialized', - 'version': str(msdInfo['version']), - 'isoprefix': '', - 'pool_status': 'connected', + groupBaseInfo = { + "type": msdInfo["type"], + "name": "", + "domains": "", + "master_uuid": self.masterDomain.sdUUID, + "master_ver": 0, + "lver": LVER_INVALID, + "spm_id": SPM_ID_FREE, + "pool_status": "uninitialized", + "version": str(msdInfo["version"]), + "isoprefix": "", + "pool_status": "connected", } - return poolInfo + + groupBaseInfo.update(self.getGroupInfo()) + return groupBaseInfo @unsecured def getIsoDomain(self): @@ -1476,22 +1418,6 @@ 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 getMasterDomain(self, msdUUID, masterVersion): """ @@ -1506,32 +1432,8 @@ #Manager should start reconstructMaster if SPM. raise se.StoragePoolMasterNotFound(self.spUUID, msdUUID) - if not domain.isMaster(): - self.log.error("Requested master domain %s is not a master domain " - "at all", msdUUID) - raise se.StoragePoolWrongMaster(self.spUUID, msdUUID) - - pools = domain.getPools() - if (self.spUUID not in pools): - self.log.error("Requested master domain %s does not belong to pool" - " %s", msdUUID, self.spUUID) - raise se.StoragePoolWrongMaster(self.spUUID, msdUUID) - - version = self._getPoolMD(domain)[PMDK_MASTER_VER] - if version != int(masterVersion): - self.log.error("Requested master domain %s does not have expected " - "version %s it is version %s", - msdUUID, masterVersion, version) - raise se.StoragePoolWrongMaster(self.spUUID, msdUUID) - - self.log.debug("Master domain %s verified, version %s", msdUUID, - masterVersion) + self.checkMasterDomain(domain, masterVersion) return domain - - @unsecured - def invalidateMetadata(self): - if not self.spmRole == SPM_ACQUIRED: - self._metadata.invalidate() @unsecured @misc.samplingmethod @@ -1565,10 +1467,9 @@ @unsecured def getDomains(self, activeOnly=False): - return dict( - (sdUUID, status) for sdUUID, status in - self.getMetaParam(PMDK_DOMAINS).iteritems() - if not activeOnly or status == sd.DOM_ACTIVE_STATUS) + return dict((sdUUID, status) for sdUUID, status + in self.getDomainsMap().iteritems() + if not activeOnly or status == sd.DOM_ACTIVE_STATUS) def checkBackupDomain(self): domDict = self.getDomains(activeOnly=True) @@ -2012,3 +1913,198 @@ def getAllTasksInfo(self): return self.taskMng.getAllTasksInfo("spm") + + +class StoragePool(StoragePoolBase): + + def __init__(self, spUUID, domainMonitor, taskManager, domainsMap, + masterVersion): + super(StorageGroup, self) \ + .__init__(spUUID, domainMonitor, taskManager) + self.domainsMap = domainsMap + self.masterVersion = masterVersion + + @unsecured + def getSpmId(self): + spmid = self.masterDomain.getClusterLockOwnerId() + return SPM_ID_FREE if spmid is None else spmid + + @unsecured + def getGroupInfo(self): + return { + 'spm_id': self.getSpmId(), + 'master_ver': self.masterVersion, + } + + @unsecured + def getDomainsMap(self): + if self.domainsMap is None: + raise AttributeError("") + return self.domainsMap + + ### Deprecated Methods Section Begin ### + + @deprecated + @unsecured + def getSpmLver(self): + return LVER_INVALID + + @deprecated + @unsecured + def getMasterVersion(self): + return 0 + + @deprecated + @unsecured + def getDescription(self): + return "" + + @deprecated + def setDescription(self, description): + pass + + @deprecated + @unsecured + def checkMasterDomain(self, domain, masterVersion): + pass + + @deprecated + @unsecured + def invalidateMetadata(self): + pass + + @deprecated + @unsecured + def updateSpmInfo(self, lver, id): + pass + + ### Deprecated Methods Section End ### + + +class StoragePoolLegacy(StoragePoolBase): + + def __init__(self, spUUID, domainMonitor, taskManager): + super(StoragePool, self) \ + .__init__(spUUID, domainMonitor, taskManager) + + @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 getSpmLver(self): + return self.getMetaParam(PMDK_LVER) + + @unsecured + def getMasterVersion(self): + return self.getMetaParam(PMDK_MASTER_VER) + + @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 checkMasterDomain(self, domain, masterVersion): + if not domain.isMaster(): + self.log.error("Requested master domain %s is not a master domain " + "at all", domain.sdUUID) + raise se.StoragePoolWrongMaster(self.spUUID, domain.sdUUID) + + pools = domain.getPools() + if (self.spUUID not in pools): + self.log.error("Requested master domain %s does not belong to pool" + " %s", domain.sdUUID, self.spUUID) + raise se.StoragePoolWrongMaster(self.spUUID, domain.sdUUID) + + version = self.getMasterVersion() + if version != int(masterVersion): + self.log.error("Requested master domain %s does not have expected " + "version %s it is version %s", + domain.sdUUID, masterVersion, version) + raise se.StoragePoolWrongMaster(self.spUUID, domain.sdUUID) + + self.log.debug("Master domain %s verified, version %s", domain.sdUUID, + masterVersion) + + @unsecured + def invalidateMetadata(self): + if not self.spmRole == SPM_ACQUIRED: + self._metadata.invalidate() + + @unsecured + def getSpmId(self): + spmid = self.getMetaParam(PMDK_SPM_ID) + if spmid != self.id or self.spmRole != SPM_FREE: + return spmid + + # If we claim to be the SPM we have to be really sure we are + self.invalidateMetadata() + return self.getMetaParam(PMDK_SPM_ID) + + @unsecured + def updateSpmInfo(self, lver, id): + self.setMetaParams({PMDK_LVER: lver, PMDK_SPM_ID: id}) + + @unsecured + def getGroupInfo(self): + try: + pmd = self._getPoolMD(self.masterDomain) + except Exception: + self.log.error("Pool metadata error", exc_info=True) + raise se.StoragePoolActionError(self.spUUID) + + return { + 'name': pmd[PMDK_POOL_DESCRIPTION], + 'domains': domainListEncoder(pmd[PMDK_DOMAINS]), + 'master_ver': pmd[PMDK_MASTER_VER], + 'lver': pmd[PMDK_LVER], + 'spm_id': pmd[PMDK_SPM_ID], + } + + @unsecured + def getDomainsMap(self): + return self.getMetaParam(PMDK_DOMAINS) -- To view, visit http://gerrit.ovirt.org/21427 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I393677f643a62e3711af2a3cfb8b4b9a5ce11c2d Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>

4 14

Change in vdsm[master]: sd: [wip] add getClusterLockOwnerId method
by Federico Simoncelli 20 Jan '14

20 Jan '14

Federico Simoncelli has uploaded a new change for review. Change subject: sd: [wip] add getClusterLockOwnerId method ...................................................................... sd: [wip] add getClusterLockOwnerId method Change-Id: I10e64d74319ea6591a7edf8e17809d367a758386 Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com> --- M vdsm/storage/clusterlock.py M vdsm/storage/sd.py 2 files changed, 27 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/26/21426/1 diff --git a/vdsm/storage/clusterlock.py b/vdsm/storage/clusterlock.py index 6e4d8d6..d0da854 100644 --- a/vdsm/storage/clusterlock.py +++ b/vdsm/storage/clusterlock.py @@ -112,6 +112,9 @@ raise se.AcquireLockFailure(self._sdUUID, rc, out, err) self.log.debug("Clustered lock acquired successfully") + def getOwnerId(self): + pass # FIXME: missing implementation + def getLockUtilFullPath(self): return os.path.join(self.lockUtilPath, self.lockCmd) @@ -167,6 +170,9 @@ def getReservedId(self): return MAX_HOST_ID + + def getLockDisk(self): + return [(self._leasesPath, SDM_LEASE_OFFSET)] def acquireHostId(self, hostId, async): with self._lock: @@ -235,7 +241,7 @@ try: sanlock.acquire(self._sdUUID, SDM_LEASE_NAME, - [(self._leasesPath, SDM_LEASE_OFFSET)], + self.getLockDisk(), slkfd=SANLock._sanlock_fd) except sanlock.SanlockException as e: if e.errno != os.errno.EPIPE: @@ -250,14 +256,26 @@ self.log.debug("Cluster lock for domain %s successfully acquired " "(id: %s)", self._sdUUID, hostId) + def getOwnerId(self): + owners = sanlock.read_resource_owners( + self._sdUUID, SDM_LEASE_NAME, self.getLockDisk()) + + if len(owners) == 1: + return owners[0].get("host_id") + elif len(owners) > 1: + self.log.error("Cluster lock is reported to have more than " + "one owner: %s", owners) + raise RuntimeError("Cluster lock multiple owners error") + + return None + def release(self): with self._lock: self.log.info("Releasing cluster lock for domain %s", self._sdUUID) try: sanlock.release(self._sdUUID, SDM_LEASE_NAME, - [(self._leasesPath, SDM_LEASE_OFFSET)], - slkfd=SANLock._sanlock_fd) + self.getLockDisk(), slkfd=SANLock._sanlock_fd) except sanlock.SanlockException as e: raise se.ReleaseLockFailure(self._sdUUID, e) @@ -345,6 +363,9 @@ self.log.debug("Local lock for domain %s successfully acquired " "(id: %s)", self._sdUUID, hostId) + def getOwnerId(self): + pass # FIXME: missing implementation + def release(self): with self._globalLockMapSync: self.log.info("Releasing local lock for domain %s", self._sdUUID) diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py index 3249343..03d3274 100644 --- a/vdsm/storage/sd.py +++ b/vdsm/storage/sd.py @@ -489,6 +489,9 @@ def releaseClusterLock(self): self._clusterLock.release() + def getClusterLockOwnerId(self): + self._clusterLock.getOwnerId() + def attach(self, spUUID): self.invalidateMetadata() pools = self.getPools() -- To view, visit http://gerrit.ovirt.org/21426 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I10e64d74319ea6591a7edf8e17809d367a758386 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>

5 20

Change in vdsm[master]: mkimage: do not create world-readable image
by fromani＠redhat.com 20 Jan '14

20 Jan '14

Francesco Romani has uploaded a new change for review. Change subject: mkimage: do not create world-readable image ...................................................................... mkimage: do not create world-readable image mkimage generates world-readable images by default. mkimage spawns a genisoimage process through the cpopen package to do the actual work. It is of course possible to fix the permissions once the ISO image is created with a trivial os.chmod() call, but this will leave open a window opportunity to still exploit the bug. A more correct and secure way to fix the permissions of the newly generated image is to setup the umask just before to exec. The current python-cpopen package lacks a way to set the umask of the child before to run it, so this patch depends on an enhanced python-cpopen, temporarily located here: https://github.com/mojaves/python-cpopen With this enhanced cpopen, fixing the permissions is trivially made by passing the correct umask at the ISO image creation. Change-Id: I893a1310d9988c52cec9f48dfd17dfa1647da4dc Bug-Url: https://bugzilla.redhat.com/1034172 Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M lib/vdsm/utils.py M vdsm/mkimage.py 2 files changed, 4 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/21946/1 diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index ebd36b8..8173916 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -501,7 +501,7 @@ def execCmd(command, sudo=False, cwd=None, data=None, raw=False, logErr=True, printable=None, env=None, sync=True, nice=None, ioclass=None, ioclassdata=None, setsid=False, execCmdLogger=logging.root, - deathSignal=0): + deathSignal=0, childUmask=None): """ Executes an external command, optionally via sudo. @@ -531,7 +531,7 @@ execCmdLogger.debug("%s (cwd %s)", cmdline, cwd) p = BetterPopen(command, close_fds=True, cwd=cwd, env=env, - deathSignal=deathSignal) + deathSignal=deathSignal, childUmask=childUmask) p = AsyncProc(p) if not sync: if data is not None: diff --git a/vdsm/mkimage.py b/vdsm/mkimage.py index a4716c7..4daefec 100644 --- a/vdsm/mkimage.py +++ b/vdsm/mkimage.py @@ -117,7 +117,8 @@ if volumeName is not None: command.extend(['-V', volumeName]) command.extend([dirname]) - rc, out, err = storage.misc.execCmd(command, raw=True) + rc, out, err = storage.misc.execCmd(command, raw=True, + childUmask=0o007) if rc: raise OSError(errno.EIO, "could not create iso file: " "code %s, out %s\nerr %s" % (rc, out, err)) -- To view, visit http://gerrit.ovirt.org/21946 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I893a1310d9988c52cec9f48dfd17dfa1647da4dc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

4 29

Change in vdsm[master]: remove redundant libvirt call
by peet＠redhat.com 17 Jan '14

17 Jan '14

Peter V. Saveliev has uploaded a new change for review. Change subject: remove redundant libvirt call ...................................................................... remove redundant libvirt call Checking UUIDString() adresses the bug #603494, which is closed ages ago. While it is nice to support really ancient libvirt versions, it costs us one extra libvirt call. Given global libvirt driver lock, it does matter in mass VM creation. Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=603494 Signed-off-by: Peter V. Saveliev <peet(a)redhat.com> Change-Id: I06a71c2e44332f50b3203a0a8f1bfa799914b684 --- M vdsm/libvirtvm.py 1 file changed, 0 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/54/11054/1 diff --git a/vdsm/libvirtvm.py b/vdsm/libvirtvm.py index d13dbb0..9376e5c 100644 --- a/vdsm/libvirtvm.py +++ b/vdsm/libvirtvm.py @@ -1539,8 +1539,6 @@ self._dom = NotifyingVirDomain( self._connection.createXML(domxml, flags), self._timeoutExperienced) - if self._dom.UUIDString() != self.id: - raise Exception('libvirt bug 603494') hooks.after_vm_start(self._dom.XMLDesc(0), self.conf) if not self._dom: self.setDownStatus(ERROR, 'failed to start libvirt vm') -- To view, visit http://gerrit.ovirt.org/11054 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I06a71c2e44332f50b3203a0a8f1bfa799914b684 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Peter V. Saveliev <peet(a)redhat.com>

6 10

Change in vdsm[master]: storage: refactor nfsSD.py for posix storage
by gpadgett＠redhat.com 16 Jan '14

16 Jan '14

Greg Padgett has uploaded a new change for review. Change subject: storage: refactor nfsSD.py for posix storage ...................................................................... storage: refactor nfsSD.py for posix storage NFS and PosixFs domains are no longer handled the same way, so logic that differs between them should be appropriately separated into classes for each connection type. This removes the need to have a storageType parameter to NfsStorageDomain:_preCreateValidation(). For now, there's no reason to have a PosixFsStorageDomain class for Posix storage (it would be empty), so just use MountableStorageDomain until the need for a subclass arises. Change-Id: Ic5788925f9c11b417aba713c652fc2a92f178830 Signed-off-by: Greg Padgett <gpadgett(a)redhat.com> --- M Makefile.am M vdsm.spec.in M vdsm/storage/Makefile.am M vdsm/storage/hsm.py A vdsm/storage/mountableSD.py M vdsm/storage/nfsSD.py M vdsm/storage/sdc.py 7 files changed, 148 insertions(+), 114 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/9088/1 diff --git a/Makefile.am b/Makefile.am index bd262af..a96d3f7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -83,6 +83,7 @@ vdsm/storage/lvm.py \ vdsm/storage/misc.py \ vdsm/storage/mount.py \ + vdsm/storage/mountableSD.py \ vdsm/storage/multipath.py \ vdsm/storage/nfsSD.py \ vdsm/storage/outOfProcess.py \ diff --git a/vdsm.spec.in b/vdsm.spec.in index c117ee5..d76495c 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -661,6 +661,7 @@ %{_datadir}/%{vdsm_name}/storage/lvm.py* %{_datadir}/%{vdsm_name}/storage/misc.py* %{_datadir}/%{vdsm_name}/storage/mount.py* +%{_datadir}/%{vdsm_name}/storage/mountableSD.py* %{_datadir}/%{vdsm_name}/storage/multipath.py* %{_datadir}/%{vdsm_name}/storage/nfsSD.py* %{_datadir}/%{vdsm_name}/storage/outOfProcess.py* diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am index cff09be..cf8f768 100644 --- a/vdsm/storage/Makefile.am +++ b/vdsm/storage/Makefile.am @@ -41,6 +41,7 @@ lvm.py \ misc.py \ mount.py \ + mountableSD.py \ multipath.py \ nfsSD.py \ outOfProcess.py \ diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 259676b..f56c8b0 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -41,8 +41,9 @@ import sp import sd import blockSD -import nfsSD import localFsSD +import mountableSD +import nfsSD import lvm import fileUtils import multipath @@ -2251,16 +2252,18 @@ #getSharedLock(connectionsResource...) #getExclusiveLock(sdUUID...) - if storageType in sd.BLOCK_DOMAIN_TYPES: - newSD = blockSD.BlockStorageDomain.create(sdUUID, domainName, - domClass, typeSpecificArg, storageType, domVersion) - elif storageType in (sd.NFS_DOMAIN, sd.POSIXFS_DOMAIN): - newSD = nfsSD.NfsStorageDomain.create(sdUUID, domainName, domClass, - typeSpecificArg, storageType, domVersion) - elif storageType == sd.LOCALFS_DOMAIN: - newSD = localFsSD.LocalFsStorageDomain.create(sdUUID, domainName, - domClass, typeSpecificArg, storageType, domVersion) - else: + sdConstructorMap = { + sd.NFS_DOMAIN : nfsSD.NfsStorageDomain, + sd.POSIXFS_DOMAIN : mountableSD.MountableStorageDomain, + sd.LOCALFS_DOMAIN : localFsSD.LocalFsStorageDomain, + } + for domType in sd.BLOCK_DOMAIN_TYPES: + sdConstructorMap[domType] = blockSD.BlockStorageDomain + + try: + newSD = sdConstructorMap[storageType].create(sdUUID, domainName, + domClass, typeSpecificArg, storageType, domVersion) + except KeyError: raise se.StorageDomainTypeError(storageType) sdCache.manuallyAddDomain(newSD) diff --git a/vdsm/storage/mountableSD.py b/vdsm/storage/mountableSD.py new file mode 100644 index 0000000..05284ab --- /dev/null +++ b/vdsm/storage/mountableSD.py @@ -0,0 +1,122 @@ +# +# Copyright 2009-2012 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 os + +import sd +import fileSD +import fileUtils +import storage_exception as se +import outOfProcess as oop +import mount +import misc + + +class MountableStorageDomain(fileSD.FileStorageDomain): + + @classmethod + def _preCreateValidation(cls, sdUUID, domPath, typeSpecificArg, version): + # Some trivial resource validation + sd.validateDomainVersion(version) + + # Make sure the underlying file system is mounted + if not mount.isMounted(domPath): + raise se.StorageDomainFSNotMounted(domPath) + + fileSD.validateDirAccess(domPath) + + # Make sure there are no remnants of other domain + mdpat = os.path.join(domPath, "*", sd.DOMAIN_META_DATA) + if len(oop.getProcessPool(sdUUID).glob.glob(mdpat)) > 0: + raise se.StorageDomainNotEmpty(typeSpecificArg) + + @classmethod + def create(cls, sdUUID, domainName, domClass, remotePath, storageType, + version): + """ + Create new storage domain. + 'sdUUID' - Storage Domain UUID + 'domainName' - storage domain name ("iso" or "data domain name") + 'domClass' - Data/Iso + 'remotePath' - server:/export_path + 'storageType' - NFS_DOMAIN, LOCALFS_DOMAIN, &etc. + 'version' - DOMAIN_VERSIONS + """ + cls.log.info("sdUUID=%s domainName=%s remotePath=%s " + "domClass=%s", sdUUID, domainName, remotePath, domClass) + + if not misc.isAscii(domainName) and not sd.supportsUnicode(version): + raise se.UnicodeArgumentException() + + # Create local path + mntPath = fileUtils.transformPath(remotePath) + + mntPoint = os.path.join(cls.storage_repository, + sd.DOMAIN_MNT_POINT, mntPath) + + cls._preCreateValidation(sdUUID, mntPoint, remotePath, version) + + domainDir = os.path.join(mntPoint, sdUUID) + cls._prepareMetadata(domainDir, sdUUID, domainName, domClass, + remotePath, storageType, version) + + # create domain images folder + imagesDir = os.path.join(domainDir, sd.DOMAIN_IMAGES) + oop.getProcessPool(sdUUID).fileUtils.createdir(imagesDir) + + # create special imageUUID for ISO/Floppy volumes + if domClass is sd.ISO_DOMAIN: + isoDir = os.path.join(imagesDir, sd.ISO_IMAGE_UUID) + oop.getProcessPool(sdUUID).fileUtils.createdir(isoDir) + + fsd = cls(os.path.join(mntPoint, sdUUID)) + fsd.initSPMlease() + + return fsd + + def selftest(self): + """ + Run internal self test + """ + if not mount.isMounted(self.mountpoint): + raise se.StorageDomainFSNotMounted(self.mountpoint) + + # Run general part of selftest + fileSD.FileStorageDomain.selftest(self) + + @staticmethod + def findDomainPath(sdUUID): + for tmpSdUUID, domainPath in fileSD.scanDomains("*"): + if tmpSdUUID == sdUUID and mount.isMounted( + os.path.join(domainPath, "..")): + return domainPath + + raise se.StorageDomainDoesNotExist(sdUUID) + + def getRealPath(self): + try: + return mount.getMountFromTarget(self.mountpoint).fs_spec + except mount.MountError: + return "" + + +def findDomain(sdUUID): + return MountableStorageDomain(MountableStorageDomain + .findDomainPath(sdUUID)) diff --git a/vdsm/storage/nfsSD.py b/vdsm/storage/nfsSD.py index 3a1c90e..15dd137 100644 --- a/vdsm/storage/nfsSD.py +++ b/vdsm/storage/nfsSD.py @@ -1,5 +1,5 @@ # -# Copyright 2009-2011 Red Hat, Inc. +# Copyright 2009-2012 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 @@ -18,110 +18,16 @@ # Refer to the README and COPYING files for full details of the license # -import os - -import sd -import fileSD -import fileUtils +import mountableSD import storage_exception as se -import outOfProcess as oop -import mount -import misc -class NfsStorageDomain(fileSD.FileStorageDomain): +class NfsStorageDomain(mountableSD.MountableStorageDomain): @classmethod - def _preCreateValidation(cls, sdUUID, domPath, typeSpecificArg, - storageType, version): - # Some trivial resource validation - # TODO Checking storageType==nfs in the nfs class is not clean - if storageType == sd.NFS_DOMAIN and ":" not in typeSpecificArg: + def _preCreateValidation(cls, sdUUID, domPath, typeSpecificArg, version): + if ":" not in typeSpecificArg: raise se.StorageDomainIllegalRemotePath(typeSpecificArg) - sd.validateDomainVersion(version) - - # Make sure the underlying file system is mounted - if not mount.isMounted(domPath): - raise se.StorageDomainFSNotMounted(domPath) - - fileSD.validateDirAccess(domPath) - - # Make sure there are no remnants of other domain - mdpat = os.path.join(domPath, "*", sd.DOMAIN_META_DATA) - if len(oop.getProcessPool(sdUUID).glob.glob(mdpat)) > 0: - raise se.StorageDomainNotEmpty(typeSpecificArg) - - @classmethod - def create(cls, sdUUID, domainName, domClass, remotePath, storageType, - version): - """ - Create new storage domain. - 'sdUUID' - Storage Domain UUID - 'domainName' - storage domain name ("iso" or "data domain name") - 'domClass' - Data/Iso - 'remotePath' - server:/export_path - 'storageType' - NFS_DOMAIN, LOCALFS_DOMAIN, &etc. - 'version' - DOMAIN_VERSIONS - """ - cls.log.info("sdUUID=%s domainName=%s remotePath=%s " - "domClass=%s", sdUUID, domainName, remotePath, domClass) - - if not misc.isAscii(domainName) and not sd.supportsUnicode(version): - raise se.UnicodeArgumentException() - - # Create local path - mntPath = fileUtils.transformPath(remotePath) - - mntPoint = os.path.join(cls.storage_repository, - sd.DOMAIN_MNT_POINT, mntPath) - - cls._preCreateValidation(sdUUID, mntPoint, remotePath, storageType, - version) - - domainDir = os.path.join(mntPoint, sdUUID) - cls._prepareMetadata(domainDir, sdUUID, domainName, domClass, - remotePath, storageType, version) - - # create domain images folder - imagesDir = os.path.join(domainDir, sd.DOMAIN_IMAGES) - oop.getProcessPool(sdUUID).fileUtils.createdir(imagesDir) - - # create special imageUUID for ISO/Floppy volumes - if domClass is sd.ISO_DOMAIN: - isoDir = os.path.join(imagesDir, sd.ISO_IMAGE_UUID) - oop.getProcessPool(sdUUID).fileUtils.createdir(isoDir) - - fsd = cls(os.path.join(mntPoint, sdUUID)) - fsd.initSPMlease() - - return fsd - - def selftest(self): - """ - Run internal self test - """ - if not mount.isMounted(self.mountpoint): - raise se.StorageDomainFSNotMounted(self.mountpoint) - - # Run general part of selftest - fileSD.FileStorageDomain.selftest(self) - - @staticmethod - def findDomainPath(sdUUID): - for tmpSdUUID, domainPath in fileSD.scanDomains("*"): - if tmpSdUUID == sdUUID and mount.isMounted( - os.path.join(domainPath, "..")): - return domainPath - - raise se.StorageDomainDoesNotExist(sdUUID) - - def getRealPath(self): - try: - return mount.getMountFromTarget(self.mountpoint).fs_spec - except mount.MountError: - return "" - - -def findDomain(sdUUID): - return NfsStorageDomain(NfsStorageDomain.findDomainPath(sdUUID)) + mountableSD.MountableStorageDomain._preCreateValidation(sdUUID, + domPath, typeSpecificArg, version) diff --git a/vdsm/storage/sdc.py b/vdsm/storage/sdc.py index f2f4534..1064b5e 100644 --- a/vdsm/storage/sdc.py +++ b/vdsm/storage/sdc.py @@ -132,7 +132,7 @@ def _findDomain(self, sdUUID): import blockSD import localFsSD - import nfsSD + import mountableSD # The order is somewhat important, it's ordered # by how quickly get can find the domain. For instance @@ -140,7 +140,7 @@ # until it times out, this should affect fetching # of block\local domains. If for any case in the future # this changes, please update the order. - for mod in (blockSD, localFsSD, nfsSD): + for mod in (blockSD, localFsSD, mountableSD): try: return mod.findDomain(sdUUID) except se.StorageDomainDoesNotExist: -- To view, visit http://gerrit.ovirt.org/9088 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic5788925f9c11b417aba713c652fc2a92f178830 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>

2 3

Change in vdsm[master]: securable: refactor the scurable implementation
by Federico Simoncelli 16 Jan '14

16 Jan '14

Federico Simoncelli has uploaded a new change for review. Change subject: securable: refactor the scurable implementation ...................................................................... securable: refactor the scurable implementation Change-Id: Id5a8be7536748481746795e27701fbecc7c3318c Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com> --- M tests/main.py M vdsm/storage/securable.py M vdsm/storage/sp.py 3 files changed, 28 insertions(+), 68 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/15/22115/1 diff --git a/tests/main.py b/tests/main.py index dfc8b04..1f87092 100644 --- a/tests/main.py +++ b/tests/main.py @@ -23,22 +23,9 @@ from gluster import exception as gluster_exception from storage import storage_exception -from storage import securable from vdsm.utils import GeneralException from testrunner import VdsmTestCase as TestCaseBase - - -class TestSecurable(TestCaseBase): - class MySecureClass(securable.Securable): - pass - - def test_safety(self): - objA = TestSecurable.MySecureClass() - objB = TestSecurable.MySecureClass() - objA._setSafe() - objB._setUnsafe() - self.assertTrue(objA._isSafe()) class TestStorageExceptions(TestCaseBase): diff --git a/vdsm/storage/securable.py b/vdsm/storage/securable.py index cda74ef..9a31bff 100644 --- a/vdsm/storage/securable.py +++ b/vdsm/storage/securable.py @@ -18,35 +18,25 @@ # Refer to the README and COPYING files for full details of the license # -from threading import Event -from functools import wraps +from functools import update_wrapper OVERRIDE_ARG = "__securityOverride" SECURE_FIELD = "__secured__" +SECURE_METHOD = 'isSafe' class SecureError(RuntimeError): pass -class MetaSecurable(type): - def __new__(cls, name, bases, dct): - for fun, val in dct.iteritems(): - if not callable(val): - continue - - if (hasattr(val, SECURE_FIELD) and - not getattr(val, SECURE_FIELD)): - continue - - if fun.startswith("__"): - # Wrapping builtins might cause weird results - continue - - dct[fun] = secured(val) - - dct['__securable__'] = True - return type.__new__(cls, name, bases, dct) +def secured(cls): + for fun, val in cls.__dict__.iteritems(): + if (callable(val) and getattr(val, SECURE_FIELD, True) + and not fun.startswith("__") # skipping builtins + and not fun == SECURE_METHOD): + setattr(cls, fun, secure_method(val)) + cls.__securable__ = True + return cls def unsecured(f): @@ -54,36 +44,16 @@ return f -def secured(f): - @wraps(f) +def secure_method(method): def wrapper(self, *args, **kwargs): if not hasattr(self, "__securable__"): raise RuntimeError("Secured object is not a securable") override = kwargs.pop(OVERRIDE_ARG, False) - if not (self._isSafe() or override): - raise SecureError() + if not (getattr(self, SECURE_METHOD)() or override): + raise SecureError("Secured object is not in safe state") - return f(self, *args, **kwargs) + return method(self, *args, **kwargs) - return wrapper - - -class Securable(object): - __metaclass__ = MetaSecurable - - def __init__(self): - self._safety = Event() - - @unsecured - def _isSafe(self): - return self._safety.isSet() - - @unsecured - def _setSafe(self): - self._safety.set() - - @unsecured - def _setUnsafe(self): - self._safety.clear() + return update_wrapper(wrapper, method) diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index 31e0bcd..6e9d1e5 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -43,7 +43,7 @@ import storage_exception as se from persistentDict import DictValidator, unicodeEncoder, unicodeDecoder from remoteFileHandler import Timeout -from securable import Securable, unsecured +from securable import secured, unsecured import image from resourceFactories import IMAGE_NAMESPACE from storageConstants import STORAGE @@ -100,7 +100,8 @@ MAX_DOMAINS /= 48 -class StoragePool(Securable): +@secured +class StoragePool(object): ''' StoragePool object should be relatively cheap to construct. It should defer any heavy lifting activities until the time it is really needed. @@ -112,12 +113,11 @@ lvExtendPolicy = config.get('irs', 'vol_extend_policy') def __init__(self, spUUID, domainMonitor, taskManager): - Securable.__init__(self) - + self._safety = threading.Event() + self._safety.clear() self._formatConverter = DefaultFormatConverter() self._domainsToUpgrade = [] self.lock = threading.RLock() - self._setUnsafe() self.spUUID = str(spUUID) self.poolPath = os.path.join(self.storage_repository, self.spUUID) self.id = SPM_ID_FREE @@ -131,6 +131,9 @@ self.domainMonitor = domainMonitor self._upgradeCallback = partial(StoragePool._upgradePoolDomain, proxy(self)) + + def isSafe(self): + return self._safety.isSet() @unsecured def getSpmLver(self): @@ -303,11 +306,11 @@ self.spmRole = SPM_ACQUIRED # Once setSafe completes we are running as SPM - self._setSafe() + self._safety.set() # Mailbox issues SPM commands, therefore we start it AFTER spm # commands are allowed to run to prevent a race between the - # mailbox and the "self._setSafe() call" + # mailbox and the "self._safety.set() call" # Create mailbox if SAN pool (currently not needed on nas) # FIXME: Once pool contains mixed type domains (NFS + Block) @@ -395,7 +398,7 @@ return True self._shutDownUpgrade() - self._setUnsafe() + self._safety.clear() stopFailed = False @@ -614,7 +617,7 @@ self._acquireTemporaryClusterLock(msdUUID, leaseParams) try: - self._setSafe() + self._safety.set() # Mark 'master' domain # We should do it before actually attaching this domain to the pool # During 'master' marking we create pool metadata and each attached @@ -644,7 +647,7 @@ exc_info=True) raise finally: - self._setUnsafe() + self._safety.clear() self._releaseTemporaryClusterLock(msdUUID) self.stopMonitoringDomains() -- To view, visit http://gerrit.ovirt.org/22115 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id5a8be7536748481746795e27701fbecc7c3318c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>

8 36

Change in vdsm[master]: Adding monitorPoolDomains replacing connectStoragePool.
by ewarszaw＠redhat.com 16 Jan '14

16 Jan '14

Eduardo has uploaded a new change for review. Change subject: Adding monitorPoolDomains replacing connectStoragePool. ...................................................................... Adding monitorPoolDomains replacing connectStoragePool. Spliting the share storage and host views of the pool in order to remove the pool concept. Change-Id: I1db3aeb5d2c3ab5a4ad90c175bc3e576726f0336 Signed-off-by: Eduardo <ewarszaw(a)redhat.com> --- M vdsm/storage/hsm.py M vdsm/storage/sp.py M vdsm_cli/vdsClient.py 3 files changed, 58 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/37/11937/1 diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 29980f3..3b0d4e3 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -932,6 +932,30 @@ return self._connectStoragePool(spUUID, hostID, scsiKey, msdUUID, masterVersion, options) + @public + def monitorPoolDomains(self, spUUID, hostID, scsiKey, msdUUID, + masterVersion, domains): + with rmanager.acquireResource(STORAGE, spUUID, rm.LockType.exclusive): + try: + pool = self.getPool(spUUID) + except se.StoragePoolUnknown: + pass # pool not connected yet + else: + return True + + pool = sp.StoragePool(spUUID, self.taskMng) + pool.id = hostID + pool.scsiKey = scsiKey + # In memory list of domains + pool.domains = domains + + pool.rebuild(msdUUID=msdUUID, masterVersion=masterVersion) + + pool.createMailboxMonitor() + + self.pools[spUUID] = pool + return True + def _connectStoragePool(self, spUUID, hostID, scsiKey, msdUUID, masterVersion, options=None): misc.validateUUID(spUUID, 'spUUID') diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index fb352e2..4897be8 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -463,7 +463,7 @@ kwargs={"__securityOverride": True}).start() @unsecured - def __createMailboxMonitor(self): + def createMailboxMonitor(self): if self.hsmMailer: return @@ -613,7 +613,7 @@ # During 'master' marking we create pool metadata and each attached # domain should register there self.createMaster(poolName, msd, masterVersion, leaseParams) - self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion) + self.rebuild(msdUUID=msdUUID, masterVersion=masterVersion) # Attach storage domains to the storage pool # Since we are creating the pool then attach is done from the hsm # and not the spm therefore we must manually take the master domain @@ -685,8 +685,8 @@ sdCache.invalidateStorage() sdCache.refresh() # Rebuild whole Pool - self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion) - self.__createMailboxMonitor() + self.rebuild(msdUUID=msdUUID, masterVersion=masterVersion) + self.createMailboxMonitor() return True @@ -1212,7 +1212,7 @@ domPoolMD[PMDK_MASTER_VER] = 0 @unsecured - def __rebuild(self, msdUUID, masterVersion): + def rebuild(self, msdUUID, masterVersion): """ Rebuild storage pool. """ @@ -1291,7 +1291,7 @@ """ sdCache.invalidateStorage() sdCache.refresh() - self.__rebuild(msdUUID=msdUUID, masterVersion=masterVersion) + self.rebuild(msdUUID=msdUUID, masterVersion=masterVersion) def updateVM(self, vmList, sdUUID=None): """ @@ -1631,10 +1631,14 @@ @unsecured def getDomains(self, activeOnly=False): - return dict( - (sdUUID, status) for sdUUID, status in - self.getMetaParam(PMDK_DOMAINS).iteritems() - if not activeOnly or status == sd.DOM_ACTIVE_STATUS) + if hasattr(self, "domains"): + domains = dict.fromkeys(self.domains, sd.DOM_ACTIVE_STATUS) + else: + domains = dict( + (sdUUID, status) for sdUUID, status in + self.getMetaParam(PMDK_DOMAINS).iteritems() + if not activeOnly or status == sd.DOM_ACTIVE_STATUS) + return domains def checkBackupDomain(self): domDict = self.getDomains(activeOnly=True) diff --git a/vdsm_cli/vdsClient.py b/vdsm_cli/vdsClient.py index fefc209..7cad88e 100644 --- a/vdsm_cli/vdsClient.py +++ b/vdsm_cli/vdsClient.py @@ -846,6 +846,19 @@ return pool['status']['code'], pool['status']['message'] return 0, '' + def monitorPoolDomains(self, args): + spUUID = args[0] + hostID = int(args[1]) + scsiKey = args[2] + msdUUID = args[3] + masterVersion = args[4] + domains = args[5] + res = self.s.monitorPoolDomains(spUUID, hostID, scsiKey, msdUUID, + masterVersion, domains) + if res['status']['code']: + return res['status']['code'], res['status']['message'] + return 0, '' + def connectStoragePool(self, args): spUUID = args[0] ID = int(args[1]) @@ -2047,6 +2060,13 @@ ('<spUUID> <id> <scsi-key>', 'Destroy storage pool' )), + 'monitorPoolDomains': (serv.monitorPoolDomains, + ('<spUUID> <id> <scsi-key> <masterUUID> ' + '<masterVer> <domainsList>', + 'Connect a Host to specific storage pool' + 'starting domain monitoring. All the SDs' + 'in domains list are monitored.' + )), 'connectStoragePool': (serv.connectStoragePool, ('<spUUID> <id> <scsi-key> [masterUUID] ' '[masterVer]', -- To view, visit http://gerrit.ovirt.org/11937 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1db3aeb5d2c3ab5a4ad90c175bc3e576726f0336 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>

3 6

Change in vdsm[master]: netinfo: improvements from using Link devices
by asegurap＠redhat.com 15 Jan '14

15 Jan '14

Antoni Segura Puimedon has uploaded a new change for review. Change subject: netinfo: improvements from using Link devices ...................................................................... netinfo: improvements from using Link devices Make use of the new linkwrapper features to simplify netinfo code. Change-Id: I7a51c0d3bcec1358521846238e0cbd4c13ae1d17 Signed-off-by: Antoni S. Puimedon <asegurap(a)redhat.com> --- M lib/vdsm/ipwrapper.py M lib/vdsm/netinfo.py M tests/netinfoTests.py 3 files changed, 79 insertions(+), 115 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/21408/1 diff --git a/lib/vdsm/ipwrapper.py b/lib/vdsm/ipwrapper.py index ebfda41..69be2aa 100644 --- a/lib/vdsm/ipwrapper.py +++ b/lib/vdsm/ipwrapper.py @@ -37,6 +37,7 @@ '/sbin/ethtool', # EL6, ubuntu and Debian '/usr/bin/ethtool', # Arch ) +NET_PATH = '/sys/class/net' def _isValid(ip, verifier): @@ -74,6 +75,7 @@ class Link(object): """Represents link information obtained from iproute2""" _fakeNics = config.get('vars', 'fake_nics').split(',') + _hiddenBonds = config.get('vars', 'hidden_bonds').split(',') _hiddenNics = config.get('vars', 'hidden_nics').split(',') _hiddenVlans = config.get('vars', 'hidden_vlans').split(',') @@ -149,7 +151,7 @@ detectedType = None rc, out, _ = execCmd([_ETHTOOL_BINARY.cmd, '-i', name]) if rc == 71: # Unkown driver, usually dummy or loopback - if not os.path.exists('/sys/class/net/' + name): + if not os.path.exists(os.path.join(NET_PATH, name)): raise ValueError('Device %s does not exist' % name) if name == 'lo': detectedType = LinkType.LOOPBACK @@ -178,6 +180,12 @@ detectedType = LinkType.NIC return detectedType + def isBOND(self): + return self.type == LinkType.BOND + + def isBRIDGE(self): + return self.type == LinkType.BRIDGE + def isDUMMY(self): return self.type == LinkType.DUMMY @@ -204,9 +212,16 @@ if self.isVLAN(): return anyFnmatch(self.name, self._hiddenVlans) elif self.isDUMMY() or self.isVETH() or self.isNIC(): - return anyFnmatch(self.name, self._hiddenNics) - else: - raise NotImplementedError + return (anyFnmatch(self.name, self._hiddenNics) or + (self.master and _bondExists(self.master) and + anyFnmatch(self.master, self._hiddenBonds))) + elif self.isBOND(): + return anyFnmatch(self.name, self._hiddenBonds) + return False + + +def _bondExists(bondName): + return os.path.exists(os.path.join(NET_PATH, bondName, 'bonding')) def getLinks(): diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index dd54a96..39b2d84 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -34,6 +34,7 @@ from .config import config from . import constants from .ipwrapper import getLink, getLinks +from .ipwrapper import NET_PATH from .ipwrapper import Route from .ipwrapper import routeShowAllDefaultGateways from . import libvirtconnection @@ -49,7 +50,6 @@ NET_CONF_PREF = NET_CONF_DIR + 'ifcfg-' PROC_NET_VLAN = '/proc/net/vlan/' -NET_PATH = '/sys/class/net' BONDING_MASTERS = '/sys/class/net/bonding_masters' BONDING_SLAVES = '/sys/class/net/%s/bonding/slaves' BONDING_OPT = '/sys/class/net/%s/bonding/%s' @@ -67,42 +67,11 @@ OPERSTATE_UP = 'up' -def _nic_devices(): - """ - Returns an iterable of nic devices real or fakes, available on the host. - """ - deviceLinks = getLinks() - for device in deviceLinks: - if device.isNIC() or device.isFakeNIC(): - yield device - - def nics(): - """ - Returns a list of nics devices available to be used by vdsm. - The list of nics is built by filtering out nics defined - as hidden, fake or hidden bonds (with related nics'slaves). - """ - def isEnslavedByHiddenBond(device): - """ - Returns boolean stating if a nic device is enslaved to an hidden bond. - """ - hidden_bonds = config.get('vars', 'hidden_bonds').split(',') - valid_bonds_fn = (bond_fn for bond_fn in hidden_bonds if bond_fn) - - for bond_fn in valid_bonds_fn: - for bond_path in iglob(NET_PATH + '/' + bond_fn): - bond = os.path.basename(bond_path) - if device in slaves(bond): - return True - return False - - def isManagedByVdsm(device): - """Returns boolean stating if a device should be managed by vdsm.""" - return (not device.isHidden() and - not isEnslavedByHiddenBond(device.name)) - - res = [dev.name for dev in _nic_devices() if isManagedByVdsm(dev)] + """Returns a list of nics and fake nics devices available (not hidden) to + be used by vdsm.""" + res = [dev.name for dev in getLinks() if + (dev.isNIC() or dev.isFakeNIC) and not dev.isHidden()] return res @@ -514,64 +483,72 @@ def _bridgeinfo(bridge, gateways, ipv6routes): info = _devinfo(bridge) - info.update({'gateway': getgateway(gateways, bridge), - 'ipv6gateway': ipv6routes.get(bridge, '::'), - 'ports': ports(bridge), 'stp': bridge_stp_state(bridge)}) - return (bridge, info) + info.update({'gateway': getgateway(gateways, bridge.name), + 'ipv6gateway': ipv6routes.get(bridge.name, '::'), + 'ports': ports(bridge.name), + 'stp': bridge_stp_state(bridge.name)}) + return info def _nicinfo(nic, paddr): info = _devinfo(nic) - info.update({'hwaddr': gethwaddr(nic), 'speed': speed(nic)}) - if paddr.get(nic): - info['permhwaddr'] = paddr[nic] - return (nic, info) + info.update({'hwaddr': nic.address, 'speed': speed(nic.name)}) + if nic.name in paddr: + info['permhwaddr'] = paddr[nic.name] + return info def _bondinfo(bond): info = _devinfo(bond) - info.update({'hwaddr': gethwaddr(bond), 'slaves': slaves(bond)}) - return (bond, info) + info.update({'hwaddr': bond.address, 'slaves': slaves(bond.name)}) + return info def _vlaninfo(vlan): info = _devinfo(vlan) - info.update({'iface': getVlanDevice(vlan), 'vlanid': getVlanID(vlan)}) - return (vlan, info) + info.update({'iface': vlan.device, 'vlanid': vlan.vlanid}) + return info def _devinfo(dev): - ipv4addr, ipv4netmask, ipv6addrs = getIpInfo(dev) + ipv4addr, ipv4netmask, ipv6addrs = getIpInfo(dev.name) return {'addr': ipv4addr, - 'cfg': getIfaceCfg(dev), + 'cfg': getIfaceCfg(dev.name), 'ipv6addrs': ipv6addrs, - 'mtu': str(getMtu(dev)), + 'mtu': dev.mtu, 'netmask': ipv4netmask} def get(): - d = {} + devInfo = {'networks': {}, 'bridges': {}, 'nics': {}, 'bondings': {}, + 'vlans': {}} gateways = getRoutes() ipv6routes = getIPv6Routes() - paddr = permAddr() - d['networks'] = {} for net, netAttr in networks().iteritems(): try: - d['networks'][net] = _getNetInfo(netAttr.get('iface', net), - netAttr['bridged'], gateways, - ipv6routes, - netAttr.get('qosInbound'), - netAttr.get('qosOutbound')) + devInfo['networks'][net] = _getNetInfo(netAttr.get('iface', net), + netAttr['bridged'], + gateways, ipv6routes, + netAttr.get('qosInbound'), + netAttr.get('qosOutbound')) except KeyError: continue # Do not report missing libvirt networks. - d['bridges'] = dict([_bridgeinfo(bridge, gateways, ipv6routes) - for bridge in bridges()]) - d['nics'] = dict([_nicinfo(nic, paddr) for nic in nics()]) - d['bondings'] = dict([_bondinfo(bond) for bond in bondings()]) - d['vlans'] = dict([_vlaninfo(vlan) for vlan in vlans()]) - return d + paddr = permAddr() + for device in getLinks(): + if device.isHidden(): + continue + if device.isBRIDGE(): + devInfo['bridges'][device.name] = _bridgeinfo(device, gateways, + ipv6routes) + elif device.isBOND(): + devInfo['bondings'][device.name] = _bondinfo(device) + elif device.isVLAN(): + devInfo['vlans'][device.name] = _vlaninfo(device) + elif device.isNIC() or device.isFakeNIC(): + devInfo['nics'][device.name] = _nicinfo(device, paddr) + return devInfo def isVlanned(dev): diff --git a/tests/netinfoTests.py b/tests/netinfoTests.py index a4e5cb8..f5b81de 100644 --- a/tests/netinfoTests.py +++ b/tests/netinfoTests.py @@ -116,7 +116,8 @@ def _testNics(self): """Creates a test fixture so that nics() reports: physical nics: em, me, me0, me1, hid0 and hideous - dummies: fake and fake0""" + dummies: fake and fake0 + bonds: jbond (over me0 and me1)""" lines = ('2: em: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc ' 'pfifo_fast state UP mode DEFAULT group default qlen 1000\\ ' ' link/ether f0:de:f1:da:aa:e7 brd ff:ff:ff:ff:ff:ff ' @@ -134,13 +135,13 @@ ' link/ether ff:de:11:da:aa:e7 brd ff:ff:ff:ff:ff:ff ' 'promiscuity 0 \\ nic ', '6: me0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc ' - 'pfifo_fast state UP mode DEFAULT group default qlen 1000\\ ' - ' link/ether 66:de:f1:da:aa:e7 brd ff:ff:ff:ff:ff:ff ' - 'promiscuity 0 \\ nic ', + 'pfifo_fast master jbond state UP mode DEFAULT group default ' + 'qlen 1000\\ link/ether 66:de:f1:da:aa:e7 brd ' + 'ff:ff:ff:ff:ff:ff promiscuity 0 \\ nic ', '7: me1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc ' - 'pfifo_fast state UP mode DEFAULT group default qlen 1000\\ ' - ' link/ether 77:de:f1:da:aa:e7 brd ff:ff:ff:ff:ff:ff ' - 'promiscuity 0 \\ nic ', + 'pfifo_fast master jbond state UP mode DEFAULT group default ' + 'qlen 1000\\ link/ether 66:de:f1:da:aa:e7 brd ' + 'ff:ff:ff:ff:ff:ff promiscuity 0 \\ nic ', '34: fake0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc ' 'pfifo_fast state UP mode DEFAULT group default qlen 1000\\ ' ' link/ether ff:aa:f1:da:aa:e7 brd ff:ff:ff:ff:ff:ff ' @@ -148,57 +149,28 @@ '35: fake: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc ' 'pfifo_fast state UP mode DEFAULT group default qlen 1000\\ ' ' link/ether ff:aa:f1:da:bb:e7 brd ff:ff:ff:ff:ff:ff ' - 'promiscuity 0 \\ dummy ') + 'promiscuity 0 \\ dummy ', + '419: jbond: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu ' + '1500 qdisc noqueue state UP mode DEFAULT group default \ ' + 'link/ether 66:de:f1:da:aa:e7 brd ff:ff:ff:ff:ff:ff ' + 'promiscuity 1 \ bond') return [ipwrapper.Link.fromText(line) for line in lines] - def _dev_dirs_setup(self, dir_fixture): + def testNics(self): """ - Creates test fixture so that the nics created by _testNics are reported - as: managed by vdsm: em, me, fake0, fake1 not managed due to hidden bond (jbond) enslavement: me0, me1 not managed due to being hidden nics: hid0, hideous - - returns related containing dir. """ - bonding_path = os.path.join(dir_fixture, 'jbond/bonding') - os.makedirs(bonding_path) - with open(os.path.join(bonding_path, 'slaves'), 'w') as f: - f.write('me0 me1') - - return dir_fixture - - def _config_setup(self): - """ - Returns an instance of a config stub. - With patterns: - * hid* for hidden nics. - * fake* for fake nics. - * jb* for hidden bonds. - """ - class Config(object): - def get(self, unused_vars, key): - if key == 'hidden_nics': - return 'hid*' - elif key == 'fake_nics': - return 'fake*' - else: - return 'jb*' - - return Config() - - def testNics(self): temp_dir = tempfile.mkdtemp() - with MonkeyPatchScope([(netinfo, 'BONDING_SLAVES', - temp_dir + '/%s/bonding/slaves'), - (netinfo, 'getLinks', + with MonkeyPatchScope([(netinfo, 'getLinks', self._testNics), - (netinfo, 'NET_PATH', - self._dev_dirs_setup(temp_dir)), + (ipwrapper, '_bondExists', + lambda x: x == 'jbond'), (ipwrapper.Link, '_detectType', partial(_fakeTypeDetection, ipwrapper.Link)), - (netinfo, 'config', self._config_setup()), (ipwrapper.Link, '_fakeNics', ['fake*']), + (ipwrapper.Link, '_hiddenBonds', ['jb*']), (ipwrapper.Link, '_hiddenNics', ['hid*']) ]): try: -- To view, visit http://gerrit.ovirt.org/21408 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7a51c0d3bcec1358521846238e0cbd4c13ae1d17 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon <asegurap(a)redhat.com>

4 16

Change in vdsm[master]: vdsm: Replaced all too generic except: handlers
by Vinzenz Feenstra 14 Jan '14

14 Jan '14

Vinzenz Feenstra has uploaded a new change for review. Change subject: vdsm: Replaced all too generic except: handlers ...................................................................... vdsm: Replaced all too generic except: handlers Change-Id: I00f1c6650b357cebfcaa375a2babb8c12637fd5a Signed-off-by: Vinzenz Feenstra <vfeenstr(a)redhat.com> --- M vdsm/vm.py 1 file changed, 34 insertions(+), 34 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/40/17740/1 diff --git a/vdsm/vm.py b/vdsm/vm.py index 0b71314..79e13a9 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -184,7 +184,7 @@ if not status['status']['code']: self.log.error("Machine already exists on the destination") self.status = errCode['exist'] - except: + except Exception: self.log.error("Error initiating connection", exc_info=True) self.status = errCode['noConPeer'] @@ -243,7 +243,7 @@ if self._mode != 'file': try: self.destServer.destroy(self._vm.id) - except: + except Exception: self.log.error("Failed to destroy remote VM", exc_info=True) # if the guest was stopped before migration, we need to cont it if self._mode == 'file' or self._method != 'online': @@ -332,7 +332,7 @@ self._vm._dom.save(fname) finally: self._vm.cif.teardownVolumePath(self._dst) - except: + except Exception: self._vm._vmStats.cont() raise else: @@ -1393,7 +1393,7 @@ if self._blockDev is None: try: self._blockDev = utils.isBlockDevice(self.path) - except: + except Exception: self.log.debug("Unable to determine if the path '%s' is a " "block device", self.path, exc_info=True) return self._blockDev @@ -2048,7 +2048,7 @@ try: with self._confLock: del self.conf['pauseCode'] - except: + except KeyError: pass if 'recover' in self.conf: @@ -2119,7 +2119,7 @@ self._saveStateInternal() try: self._getUnderlyingVmInfo() - except: + except Exception: # we do not care if _dom suddenly died now pass @@ -2164,7 +2164,7 @@ if self.conf.get('volatileFloppy'): self._ejectFloppy() self.log.debug('ejected volatileFloppy') - except: + except Exception: self.log.error("Reboot event failed", exc_info=True) def onShutdown(self): @@ -2332,7 +2332,7 @@ try: with self._confLock: del self.conf['pauseCode'] - except: + except KeyError: pass return {'status': doneCode, 'output': ['']} finally: @@ -2379,7 +2379,7 @@ 'code': errCode['exist']['status']['code'], 'message': 'VM without ACPI or active SolidICE tools. ' 'Try Forced Shutdown.'}} - except: + except Exception: self.log.error("Shutdown failed", exc_info=True) return {'status': {'code': errCode['exist']['status']['code'], 'message': 'Failed to shutdown VM. Try Forced Shutdown.'}} @@ -2395,7 +2395,7 @@ self.destroy() else: self._acpiShutdown() - except: + except Exception: self.log.error("_timedShutdown failed", exc_info=True) def _cleanupDrives(self, *drives): @@ -2413,7 +2413,7 @@ for drive in drives: try: self.cif.teardownVolumePath(drive) - except: + except Exception: self.log.error("Drive teardown failure for %s", drive, exc_info=True) @@ -2425,7 +2425,7 @@ try: self.log.debug("Floppy %s cleanup" % self.conf['floppy']) utils.rmFile(self.conf['floppy']) - except: + except Exception: pass def _cleanupGuestAgent(self): @@ -2434,7 +2434,7 @@ """ try: self.guestAgent.stop() - except: + except Exception: pass utils.rmFile(self._guestSocketFile) @@ -2454,11 +2454,11 @@ pass try: self.guestAgent.stop() - except: + except Exception: pass try: self._vmStats.stop() - except: + except Exception: pass self.saveState() @@ -2523,7 +2523,7 @@ and decStats['statsAge'] > config.getint('vars', 'vm_command_timeout')): stats['monitorResponse'] = '-1' - except: + except Exception: self.log.error("Error fetching vm stats", exc_info=True) for var in decStats: if type(decStats[var]) is not dict: @@ -2536,7 +2536,7 @@ for value in decStats[var]: stats['disks'][var][value] = \ utils.convertToStr(decStats[var][value]) - except: + except Exception: self.log.error("Error setting vm disk stats", exc_info=True) @@ -2560,7 +2560,7 @@ stats['pauseCode'] = self.conf['pauseCode'] try: stats.update(self.guestAgent.getGuestInfo()) - except: + except Exception: return stats memUsage = 0 realMemUsage = int(stats['memUsage']) @@ -2776,7 +2776,7 @@ # handle it. We must handle it now try: self._dom.destroy() - except: + except Exception: pass raise Exception('destroy() called before Vm started') @@ -2819,7 +2819,7 @@ try: self._dom.setSchedulerParameters({'cpu_shares': cpuShares}) - except: + except Exception: self.log.warning('failed to set Vm niceness', exc_info=True) def _run(self): @@ -3524,7 +3524,7 @@ for vmDevName, drive in newDrives.iteritems(): try: self.cif.teardownVolumePath(drive) - except: + except Exception: self.log.error("Unable to teardown drive: %s", vmDevName, exc_info=True) @@ -3679,7 +3679,7 @@ for drive in newDrives.values(): # Update the drive information try: self.updateDriveParameters(drive) - except: + except Exception: # Here it's too late to fail, the switch already happened # and there's nothing we can do, we must to proceed anyway # to report the live snapshot success. @@ -3702,7 +3702,7 @@ try: self._dom.blockRebase(mergeStatus['path'], mergeStatus['basePath'], 0, 0) - except: + except Exception: mergeStatus['status'] = MERGESTATUS.FAILED self.log.error("Live merge failed for %s", mergeStatus['path'], exc_info=True) @@ -3718,7 +3718,7 @@ try: jobInfo = self._dom.blockJobInfo(mergeStatus['path'], 0) - except: + except Exception: jobInfo = None if not jobInfo: @@ -3845,7 +3845,7 @@ try: self._setDiskReplica(srcDrive, dstDisk) - except: + except Exception: self.log.error("Unable to set the replication for disk '%s' with " "destination '%s'" % srcDrive.name, dstDisk) return errCode['replicaErr'] @@ -3865,12 +3865,12 @@ libvirt.VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT | libvirt.VIR_DOMAIN_BLOCK_REBASE_SHALLOW )) - except: + except Exception: self.log.error("Unable to start the replication for %s to %s", srcDrive.name, dstDiskCopy, exc_info=True) self.cif.teardownVolumePath(dstDiskCopy) raise - except: + except Exception: self.log.error("Cannot complete the disk replication process", exc_info=True) self._delDiskReplica(srcDrive) @@ -3878,7 +3878,7 @@ try: self.extendDriveVolume(srcDrive) - except: + except Exception: self.log.error("Initial extension request failed for %s", srcDrive.name, exc_info=True) @@ -3938,12 +3938,12 @@ try: # Stopping the replication self._dom.blockJobAbort(srcDrive.name, blockJobFlags) - except: + except Exception: self.log.error("Unable to stop the replication for the drive: %s", srcDrive.name, exc_info=True) try: self.cif.teardownVolumePath(srcDrive.diskReplicate) - except: + except Exception: # There is nothing we can do at this point other than logging self.log.error("Unable to teardown the replication " "destination disk", exc_info=True) @@ -3951,7 +3951,7 @@ else: try: self.cif.teardownVolumePath(diskToTeardown) - except: + except Exception: # There is nothing we can do at this point other than logging self.log.error("Unable to teardown the previous chain: %s", diskToTeardown, exc_info=True) @@ -4044,7 +4044,7 @@ return self._diskSizeExtendCow(drive, newSizeBytes) else: return self._diskSizeExtendRaw(drive, newSizeBytes) - except: + except Exception: self.log.error("Unable to extend disk %s to size %s", drive.name, newSizeBytes, exc_info=True) @@ -4094,7 +4094,7 @@ try: self._dom.updateDeviceFlags( diskelem.toxml(), libvirt.VIR_DOMAIN_DEVICE_MODIFY_FORCE) - except: + except Exception: self.log.debug("updateDeviceFlags failed", exc_info=True) self.cif.teardownVolumePath(drivespec) return {'status': {'code': errCode['changeDisk']['status']['code'], @@ -4178,7 +4178,7 @@ try: vmName = self.conf['vmName'].encode('utf-8') pid = supervdsm.getProxy().getVmPid(vmName) - except: + except Exception: pass return pid -- To view, visit http://gerrit.ovirt.org/17740 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I00f1c6650b357cebfcaa375a2babb8c12637fd5a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <vfeenstr(a)redhat.com>

5 12

Change in vdsm[master]: utils: cleanup - typos, grammar and comments refinment
by vvolansk＠redhat.com 14 Jan '14

14 Jan '14

Vered Volansky has uploaded a new change for review. Change subject: utils: cleanup - typos, grammar and comments refinment ...................................................................... utils: cleanup - typos, grammar and comments refinment Change-Id: I04a1c4d444f2604b66b44bb9deac7d780db04aaf Signed-off-by: Vered Volansky <vvolansk(a)redhat.com> --- M lib/vdsm/utils.py 1 file changed, 24 insertions(+), 17 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/62/22862/1 diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index d6c65b2..122da3e 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -19,7 +19,7 @@ # """ -A module containing miscellaneous functions and classes that are user +A module containing miscellaneous functions and classes that are used plentifuly around vdsm. .. attribute:: utils.symbolerror @@ -56,7 +56,7 @@ from cpopen import CPopen from . import constants -# Buffsize is 1K because I tested it on some use cases and 1k was fastets. If +# Buffsize is 1K because it was tested on some use cases and 1K was fastest. If # you find this number to be a bottleneck in any way you are welcome to change # it BUFFSIZE = 1024 @@ -118,7 +118,7 @@ """ Try to remove a file. - If the file doesn't exist is assumed that it was already removed. + If the file doesn't exist it's assumed that it was already removed. """ try: os.unlink(fileToRemove) @@ -135,7 +135,7 @@ """ Try to remove a directory and all it's contents. - If the directory doesn't exist is assumed that it was already removed. + If the directory doesn't exist it's assumed that it was already removed. """ try: shutil.rmtree(directoryToRemove) @@ -180,7 +180,7 @@ """ Parse ``/proc/meminfo`` and return its content as a dictionary. - For a reason unknown to me, ``/proc/meminfo`` is is sometime + For a reason unknown to me, ``/proc/meminfo`` is sometimes empty when opened. If that happens, the function retries to open it 3 times. @@ -297,7 +297,7 @@ class AsyncProc(object): """ - AsyncProc is a funky class. It warps a standard subprocess.Popen + AsyncProc is a funky class. It wraps a standard subprocess.Popen Object and gives it super powers. Like the power to read from a stream without the fear of deadlock. It does this by always sampling all stream while waiting for data. By doing this the other process can freely @@ -440,7 +440,7 @@ try: if self._stdin.len > 0 and self._stdin.pos == 0: # Polling stdin is redundant if there is nothing to write - # trun on only if data is waiting to be pushed + # turn on only if data is waiting to be pushed self._poller.modify(self._fdin, select.EPOLLOUT) pollres = NoIntrPoll(self._poller.poll, 1) @@ -801,12 +801,12 @@ class memoized(object): - """Decorator that caches a function's return value each time it is called. + """ + Decorator that caches a function's return value each time it is called. If called later with the same arguments, the cached value is returned, and not re-evaluated. There is no support for uncachable arguments. Adaptation from http://wiki.python.org/moin/PythonDecoratorLibrary#Memoize - """ def __init__(self, func): self.func = func @@ -914,7 +914,7 @@ :param func: The callable to run. :param expectedException: The exception you expect to receive when the function fails. - :param tries: The number of time to try. None\0,-1 means infinite. + :param tries: The number of times to try. None\0,-1 means infinite. :param timeout: The time you want to spend waiting. This **WILL NOT** stop the method. It will just not run it if it ended after the timeout. @@ -950,10 +950,12 @@ class AsyncProcessOperation(object): def __init__(self, proc, resultParser=None): - """Wraps a running process operation. + """ + Wraps a running process operation. resultParser should be of type callback(rc, out, err) and can return - anything or throw exceptions.""" + anything or throw exceptions. + """ self._lock = threading.Lock() self._result = None @@ -962,17 +964,22 @@ self._proc = proc def wait(self, timeout=None, cond=None): - """Waits until the process has exited, the timeout has been reached or - the condition has been met""" + """ + Waits until the process has exited, the timeout has been reached or + the condition has been met + """ return self._proc.wait(timeout, cond) def stop(self): - """Stops the running operation, effectively sending a kill signal to - the process""" + """ + Stops the running operation, effectively sending a kill signal to + the process + """ self._proc.kill() def result(self): - """Returns the result in the as a tuple of (result, error). + """ + Returns the result as a tuple of (result, error). If the operation is still running it will block until it returns. If no resultParser has been set the default result -- To view, visit http://gerrit.ovirt.org/22862 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I04a1c4d444f2604b66b44bb9deac7d780db04aaf Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Vered Volansky <vvolansk(a)redhat.com>

7 32

← Newer
1
...
20
21
22
23
24
25
26
...
50
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches December 2013