Change in vdsm[master]: Make vdsm/storage/fileSD.py pep8 clean

zhshzhou at linux.vnet.ibm.com zhshzhou at linux.vnet.ibm.com
Tue Aug 28 05:16:45 UTC 2012


Hello Xu He Jie,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/7520

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 at 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 at linux.vnet.ibm.com>
Gerrit-Reviewer: Xu He Jie <xuhj at linux.vnet.ibm.com>


More information about the vdsm-patches mailing list