Hello Xu He Jie,
I'd like you to do a code review. Please visit
to review the following change.
Change subject: Make vdsm/storage/fileSD.py pep8 clean ......................................................................
Make vdsm/storage/fileSD.py pep8 clean
Change-Id: I79e4e8bf82213c31269a6fe1fefddf619a34b82f Signed-off-by: Xu He Jie xuhj@linux.vnet.ibm.com --- M Makefile.am M vdsm/storage/fileSD.py 2 files changed, 70 insertions(+), 44 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/20/7520/1
diff --git a/Makefile.am b/Makefile.am index 17b5a85..29e84b1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -69,6 +69,7 @@ vdsm/storage/blockVolume.py \ vdsm/storage/devicemapper.py \ vdsm/storage/domainMonitor.py \ + vdsm/storage/fileSD.py \ vdsm/storage/fileUtils.py \ vdsm/storage/fileVolume.py \ vdsm/storage/fuser.py \ diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py index e47ddde..c2d6962 100644 --- a/vdsm/storage/fileSD.py +++ b/vdsm/storage/fileSD.py @@ -1,5 +1,5 @@ # -# Copyright 2009-2011 Red Hat, Inc. +# Copyright 2011-2011 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 @@ -47,6 +47,7 @@
getProcPool = oop.getGlobalProcPool
+ def validateDirAccess(dirPath): try: getProcPool().fileUtils.validateAccess(dirPath) @@ -68,8 +69,10 @@ sdUUIDPos = 3
metaList = metafile.split('/') - sdUUID = len(os.path.normpath(config.get('irs', 'repository')).split('/')) + sdUUIDPos + sdUUID = len(os.path.normpath(config.get('irs', + 'repository')).split('/')) + sdUUIDPos return metaList[sdUUID] +
class FileMetadataRW(object): """ @@ -103,7 +106,10 @@ self._oop.writeLines(tmpFilePath, metadata) self._oop.os.rename(tmpFilePath, self._metafile)
-FileSDMetadata = lambda metafile: DictValidator(PersistentDict(FileMetadataRW(metafile)), FILE_SD_MD_FIELDS) + +FileSDMetadata = lambda metafile: DictValidator( + PersistentDict(FileMetadataRW(metafile)), FILE_SD_MD_FIELDS) +
class FileStorageDomain(sd.StorageDomain): def __init__(self, domainPath): @@ -113,7 +119,8 @@ self.log.debug("Reading domain in path %s", domainPath) self.mountpoint = os.path.dirname(domainPath) self.remotePath = os.path.basename(self.mountpoint) - self.metafile = os.path.join(domainPath, sd.DOMAIN_META_DATA, sd.METADATA) + self.metafile = os.path.join(domainPath, sd.DOMAIN_META_DATA, + sd.METADATA)
metadata = FileSDMetadata(self.metafile) sdUUID = metadata[sd.DMDK_SDUUID] @@ -126,7 +133,8 @@ self._registerResourceNamespaces()
@classmethod - def _prepareMetadata(cls, domPath, sdUUID, domainName, domClass, remotePath, storageType, version): + def _prepareMetadata(cls, domPath, sdUUID, domainName, domClass, + remotePath, storageType, version): """ Prepare all domain's special volumes and metadata """ @@ -152,19 +160,23 @@ # Do we really need to keep the EXPORT_PATH? # no one uses it md.update({ - sd.DMDK_VERSION : version, - sd.DMDK_SDUUID : sdUUID, - sd.DMDK_TYPE : storageType, - sd.DMDK_CLASS : domClass, - sd.DMDK_DESCRIPTION : domainName, - sd.DMDK_ROLE : sd.REGULAR_DOMAIN, - sd.DMDK_POOLS : [], - sd.DMDK_LOCK_POLICY : '', - sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC : sd.DEFAULT_LEASE_PARAMS[sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], - sd.DMDK_LEASE_TIME_SEC : sd.DEFAULT_LEASE_PARAMS[sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], - sd.DMDK_IO_OP_TIMEOUT_SEC : sd.DEFAULT_LEASE_PARAMS[sd.DMDK_IO_OP_TIMEOUT_SEC], - sd.DMDK_LEASE_RETRIES : sd.DEFAULT_LEASE_PARAMS[sd.DMDK_LEASE_RETRIES], - REMOTE_PATH : remotePath + sd.DMDK_VERSION: version, + sd.DMDK_SDUUID: sdUUID, + sd.DMDK_TYPE: storageType, + sd.DMDK_CLASS: domClass, + sd.DMDK_DESCRIPTION: domainName, + sd.DMDK_ROLE: sd.REGULAR_DOMAIN, + sd.DMDK_POOLS: [], + sd.DMDK_LOCK_POLICY: '', + sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC: + sd.DEFAULT_LEASE_PARAMS[sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], + sd.DMDK_LEASE_TIME_SEC: sd.DEFAULT_LEASE_PARAMS[ + sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], + sd.DMDK_IO_OP_TIMEOUT_SEC: + sd.DEFAULT_LEASE_PARAMS[sd.DMDK_IO_OP_TIMEOUT_SEC], + sd.DMDK_LEASE_RETRIES: + sd.DEFAULT_LEASE_PARAMS[sd.DMDK_LEASE_RETRIES], + REMOTE_PATH: remotePath })
def getReadDelay(self): @@ -207,7 +219,6 @@ filesDict[fileName] = stats return filesDict
- def produceVolume(self, imgUUID, volUUID): """ Produce a type specific volume object @@ -215,13 +226,11 @@ repoPath = self._getRepoPath() return fileVolume.FileVolume(repoPath, self.sdUUID, imgUUID, volUUID)
- def getVolumeClass(self): """ Return a type specific volume generator object """ return fileVolume.FileVolume -
def volumeExists(self, imgPath, volUUID): """ @@ -229,7 +238,6 @@ """ volPath = os.path.join(imgPath, volUUID) return self.oop.fileUtils.pathExists(volPath) -
@classmethod def validateCreateVolumeParams(cls, volFormat, preallocate, srcVolUUID): @@ -239,10 +247,11 @@ 'volFormat' - volume format RAW/QCOW2 'preallocate' - sparse/preallocate """ - fileVolume.FileVolume.validateCreateVolumeParams(volFormat, preallocate, srcVolUUID) + fileVolume.FileVolume.validateCreateVolumeParams(volFormat, + preallocate, srcVolUUID)
- - def createVolume(self, imgUUID, size, volFormat, preallocate, diskType, volUUID, desc, srcImgUUID, srcVolUUID): + def createVolume(self, imgUUID, size, volFormat, preallocate, + diskType, volUUID, desc, srcImgUUID, srcVolUUID): """ Create a new volume """ @@ -272,7 +281,7 @@ raise se.StorageDomainAccessError(self.sdUUID)
def validateMasterMount(self): - return self.oop.fileUtils.pathExists(self.getMasterDir()) + return self.oop.fileUtils.pathExists(self.getMasterDir())
def getAllImages(self): """ @@ -301,25 +310,27 @@ metadata. Setting parent = None for compatibility with block version. """ - volMetaPattern = os.path.join(self.mountpoint, self.sdUUID, sd.DOMAIN_IMAGES, "*", "*.meta") + volMetaPattern = os.path.join(self.mountpoint, self.sdUUID, + sd.DOMAIN_IMAGES, "*", "*.meta") volMetaPaths = self.oop.glob.glob(volMetaPattern) volumes = {} for metaPath in volMetaPaths: head, tail = os.path.split(metaPath) volUUID, volExt = os.path.splitext(tail) imgUUID = os.path.basename(head) - if volumes.has_key(volUUID): + if volUUID in volumes: # Templates have no parents volumes[volUUID]['parent'] = sd.BLANK_UUID # Template volumes are hard linked in every image directory # which is derived from that template, therefore: # 1. a template volume which is in use will appear at least - # twice (in the template image dir and in the derived image dir) + # twice (in the template image dir and in the derived image + # dir) # 2. Any volume which appears more than once in the dir tree is # by definition a template volume. # 3. Any image which has more than 1 volume is not a template - # image. Therefore if imgUUID appears in more than one path then - # it is not a template. + # image. Therefore if imgUUID appears in more than one path + # then it is not a template. if len(tuple(vPath for vPath in volMetaPaths if imgUUID in vPath)) > 1: # Add template additonal image @@ -329,7 +340,8 @@ volumes[volUUID]['imgs'].insert(0, imgUUID) else: volumes[volUUID] = {'imgs': [imgUUID], 'parent': None} - return dict((k, sd.ImgsPar(tuple(v['imgs']), v['parent'])) for k, v in volumes.iteritems()) + return dict((k, sd.ImgsPar(tuple(v['imgs']), v['parent'])) + for k, v in volumes.iteritems())
@classmethod def format(cls, sdUUID): @@ -344,7 +356,8 @@ pass else: try: - oop.getProcessPool(sdUUID).fileUtils.cleanupdir(domaindir, ignoreErrors = False) + oop.getProcessPool(sdUUID).fileUtils.cleanupdir( + domaindir, ignoreErrors=False) except RuntimeError as e: raise se.MiscDirCleanupFailure(str(e))
@@ -376,8 +389,12 @@ Get storage domain statistics """ ##self.log.info("sdUUID=%s", self.sdUUID) - stats = {'disktotal':'', 'diskfree':'', 'mdavalid':True, 'mdathreshold':True, - 'mdasize':0, 'mdafree':0} + stats = {'disktotal': '', + 'diskfree': '', + 'mdavalid': True, + 'mdathreshold': True, + 'mdasize': 0, + 'mdafree': 0} try: st = self.oop.os.statvfs(self.domaindir) stats['disktotal'] = str(st.f_frsize * st.f_blocks) @@ -403,7 +420,6 @@ """ pass
- def selftest(self): """ Run internal self test @@ -416,7 +432,8 @@ # measures and unmounting this NFS resource. Chances are # that is the most intelligent thing we can do in this # situation anyway. - self.log.debug("Unmounting stale file system %s", self.mountpoint) + self.log.debug("Unmounting stale file system %s", + self.mountpoint) mount.getMountFromTarget(self.mountpoint).umount() raise se.FileStorageDomainStaleNFSHandle() raise @@ -428,11 +445,13 @@ (on NFS mostly) due to lazy file removal """ removedPattern = os.path.join(self.domaindir, sd.DOMAIN_IMAGES, - image.REMOVED_IMAGE_PREFIX+'*') + image.REMOVED_IMAGE_PREFIX + '*') removedImages = self.oop.glob.glob(removedPattern) - self.log.debug("Removing remnants of deleted images %s" % removedImages) + self.log.debug("Removing remnants of deleted images %s" % + removedImages) for imageDir in removedImages: self.oop.fileUtils.cleanupdir(imageDir) +
def getMountsList(pattern="*"): finalPat = os.path.join(sd.StorageDomain.storage_repository, @@ -441,7 +460,8 @@ # For pattern='*' in mixed pool (block and file domains) # glob will return sd.BLOCKSD_DIR among of real mount points. # Remove sd.BLOCKSD_DIR from glob results. - return [ mnt for mnt in mntList if not mnt.endswith('/'+sd.BLOCKSD_DIR) ] + return [mnt for mnt in mntList if not mnt.endswith('/' + sd.BLOCKSD_DIR)] +
def scanDomains(pattern="*"): log = logging.getLogger("scanDomains") @@ -450,11 +470,14 @@
def collectMetaFiles(possibleDomain): try: - metaFiles = oop.getGlobalProcPool().glob.glob(os.path.join(possibleDomain, - constants.UUID_GLOB_PATTERN, sd.DOMAIN_META_DATA)) + metaFiles = oop.getGlobalProcPool().glob.glob( + os.path.join(possibleDomain, + constants.UUID_GLOB_PATTERN, + sd.DOMAIN_META_DATA))
for metaFile in metaFiles: - if os.path.basename(os.path.dirname(metaFile)) != sd.MASTER_FS_DIR: + if (os.path.basename(os.path.dirname(metaFile)) != + sd.MASTER_FS_DIR): sdUUID = os.path.basename(os.path.dirname(metaFile))
return (sdUUID, os.path.dirname(metaFile)) @@ -462,7 +485,8 @@ except Timeout: pass except Exception: - log.warn("Could not collect metadata file for domain path %s", possibleDomain, exc_info=True) + log.warn("Could not collect metadata file for domain path %s", + possibleDomain, exc_info=True)
for res in misc.itmap(collectMetaFiles, mntList): if res is None: @@ -470,5 +494,6 @@
yield res
+ def getStorageDomainsList(): return [item[0] for item in scanDomains()]
-- To view, visit http://gerrit.ovirt.org/7520 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I79e4e8bf82213c31269a6fe1fefddf619a34b82f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Make vdsm/storage/fileSD.py pep8 clean ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/708/ : FAILURE
-- To view, visit http://gerrit.ovirt.org/7520 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I79e4e8bf82213c31269a6fe1fefddf619a34b82f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Make vdsm/storage/fileSD.py pep8 clean ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/709/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/7520 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I79e4e8bf82213c31269a6fe1fefddf619a34b82f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: Make vdsm/storage/fileSD.py pep8 clean ......................................................................
Patch Set 2: Verified; Looks good to me, but someone else must approve
Run through autobuild.sh. pydiff says the only difference is changing "volumes.has_key(volUUID)" to "volUUID in volumes", so it's acceptable. The detailed report is as follow.
$ pydiff fileSD*.py 1 difference(s) first file: fileSDo.py second file: fileSD.py
((311, "CallFunc(Getattr(Name('volumes'), 'has_key'), [Name('volUUID')], None, None)"), (325, "Compare(Name('volUUID'), [('in', Name('volumes'))])"))
Review the new style, looks good.
-- To view, visit http://gerrit.ovirt.org/7520 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I79e4e8bf82213c31269a6fe1fefddf619a34b82f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Make vdsm/storage/fileSD.py pep8 clean ......................................................................
Patch Set 2: Looks good to me, approved
Thanks!
-- To view, visit http://gerrit.ovirt.org/7520 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I79e4e8bf82213c31269a6fe1fefddf619a34b82f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Make vdsm/storage/fileSD.py pep8 clean ......................................................................
Make vdsm/storage/fileSD.py pep8 clean
Change-Id: I79e4e8bf82213c31269a6fe1fefddf619a34b82f Signed-off-by: Xu He Jie xuhj@linux.vnet.ibm.com --- M Makefile.am M vdsm/storage/fileSD.py 2 files changed, 79 insertions(+), 48 deletions(-)
Approvals: Dan Kenigsberg: Looks good to me, approved Zhou Zheng Sheng: Verified; Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/7520 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I79e4e8bf82213c31269a6fe1fefddf619a34b82f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Xu He Jie xuhj@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
vdsm-patches@lists.fedorahosted.org