[NEW PATCH] Change getChain() to return dictionary with chain itself and its template. (via gerrit-bot)

Igor Lvovsky ilvovsky at redhat.com
Tue Jul 12 15:35:06 UTC 2011


New patch submitted by Igor Lvovsky (ilvovsky at redhat.com)

You can review this change at: http://gerrit.usersys.redhat.com/692

commit 3a09f874889cb372ee84b04a234f7b653458e26f
Author: Igor Lvovsky <ilvovsky at redhat.com>
Date:   Tue Jul 12 15:18:45 2011 +0300

    Change getChain() to return dictionary with chain itself and its template.
    
    This change will avoid unneeded produceVolume and improve
    behaviour of different flows.
    
    Change-Id: Ife46d592171aedea087e0d6462b735cd7e0d75e0

diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index 1c70a34..102ea6c 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -104,7 +104,7 @@ class Image:
         # 1. Remove template's image:  Create 'fake' template instead of deleted one
         # 2. Remove regular image:  Remove parent-'fake' template if nobody need it already
         try:
-            pvol = self.getTemplate(sdUUID=sdUUID, imgUUID=imgUUID)
+            pvol = self.getChain(sdUUID, imgUUID)['template']
             # 1. If we required to delete template's image that have VMs
             # based on it, we should create similar 'fake' template instead
             if pvol:
@@ -269,17 +269,16 @@ class Image:
 
         return newImgUUID
 
-    def __chainSizeCalc(self, sdUUID, imgUUID, volUUID, size):
+    def __chainSizeCalc(self, sdUUID, imgUUID, size):
         """
         Compute an estimate of the whole chain size
         using the sum of the actual size of the chain's volumes
         """
-        chain = self.getChain(sdUUID, imgUUID, volUUID)
+        chainDict = self.getChain(sdUUID, imgUUID)
         newsize = 0
-        template = chain[0].getParentVolume()
-        if template:
-            newsize = template.getVolumeSize()
-        for vol in chain:
+        if chainDict['template']:
+            newsize = chainDict['template'].getVolumeSize()
+        for vol in chainDict['chain']:
             newsize += vol.getVolumeSize()
         if newsize > size:
             newsize = size
@@ -301,12 +300,14 @@ class Image:
         newsize = int(newsize * 1.1)    # allocate %10 more for cow metadata
         return newsize
 
-    def getChain(self, sdUUID, imgUUID, volUUID=None):
+    def getChain(self, sdUUID, imgUUID):
         """
-        Return the chain of volumes of image as a sorted list
-        (not including a shared base (template) if any)
+        Return the dictionary with chain of volumes of image as a sorted list
+        (not including a shared base (template)) and the template if any
         """
-        chain = []
+        # 'chain' - chain of volumes of image as a sorted list
+        # 'template' - shared base (template) if any
+        chainDict = {'chain':[], 'template':None}
         # Find all volumes of image
         volclass = SDF.produce(sdUUID).getVolumeClass()
         uuidlist = volclass.getImageVolumes(self.repoPath, sdUUID, imgUUID)
@@ -316,14 +317,13 @@ class Image:
         srcVol = volclass(self.repoPath, sdUUID, imgUUID, uuidlist[0])
         # For template image include only one volume (template itself)
         if len(uuidlist) == 1 and srcVol.isShared():
-            return [srcVol]
+            return {'chain':[srcVol], 'template':srcVol}
 
         # find the leaf
         for vol in uuidlist:
             srcVol = volclass(self.repoPath, sdUUID, imgUUID, vol)
             if srcVol.isLeaf():
-                if not volUUID or volUUID == srcVol.volUUID:
-                    break
+                break
             srcVol = None
 
         if not srcVol:
@@ -332,38 +332,25 @@ class Image:
 
         # Build up the sorted (parent->child) chain
         while not srcVol.isShared():
-            chain.insert(0, srcVol)
+            chainDict['chain'].insert(0, srcVol)
             if srcVol.getParent() == volume.BLANK_UUID:
                 break
             srcVol = srcVol.getParentVolume()
 
-        self.log.info("sdUUID=%s imgUUID=%s chain=%s ", sdUUID, imgUUID, str(chain))
-        return chain
+        if srcVol.isShared():
+            chainDict['template'] = srcVol
 
-    def getTemplate(self, sdUUID, imgUUID):
-        """
-        Return template of the image
-        """
-        tmpl = None
-        # Find all volumes of image (excluding template)
-        chain = self.getChain(sdUUID, imgUUID)
-        # check if the chain is build above a template, or it is a standalone
-        pvol = chain[0].getParentVolume()
-        if pvol:
-            tmpl = pvol
-        elif chain[0].isShared():
-            tmpl = chain[0]
-
-        return tmpl
+        self.log.info("sdUUID=%s imgUUID=%s chainDict=%s ", sdUUID, imgUUID, str(chainDict))
+        return chainDict
 
     def validate(self, srcSdUUID, dstSdUUID, imgUUID, op=MOVE_OP):
         """
         Validate template on destination domain
         """
-        # Find all volumes of source image
-        chain = self.getChain(srcSdUUID, imgUUID)
-        leafVol = chain[-1]
         srcDom = SDF.produce(srcSdUUID)
+        # Find all volumes of source image
+        chainDict = self.getChain(srcSdUUID, imgUUID)
+        leafVol = chainDict['chain'][-1]
         # Avoid move template's image if there is a VM based on it (except 'Backup' domain)
         if op == MOVE_OP and leafVol.isShared() and not srcDom.isBackup():
             chList = leafVol.getAllChildrenList(self.repoPath, srcSdUUID, imgUUID, leafVol.volUUID)
@@ -371,18 +358,16 @@ class Image:
                 raise se.MoveTemplateImageError(imgUUID)
 
         # check if the chain is build above a template, or it is a standalone
-        pvol = chain[0].getParentVolume()
+        pvol = chainDict['template']
         if pvol:    # this is a shared template based chain
-            if not pvol.isShared():
-                raise se.ImageIsNotLegalChain("Base image parent vol %s is not shared" % pvol.volUUID)
             pimg = pvol.getImage()      # pimg == template image
             try:
                 volclass = SDF.produce(dstSdUUID).getVolumeClass()
                 # Validate that the destination template exists and accessible
                 volclass(self.repoPath, dstSdUUID, pimg, pvol.volUUID)
-            except se.StorageException, e:
-                self.log.error("Unexpected error", exc_info=True)
-                raise se.CouldNotValideTemplateOnTargetDomain("Template %s Destination domain %s: %s" % (pimg, dstSdUUID, str(e)))
+            except se.StorageException:
+                self.log.error("Cannot validate template %s on target domain %s", pimg, dstSdUUID, exc_info=True)
+                raise se.CouldNotValideTemplateOnTargetDomain()
 
     def __templateRelink(self, destDom, imgUUID, volUUID):
         """
@@ -473,20 +458,15 @@ class Image:
     def _createTargetImage(self, destDom, srcSdUUID, imgUUID):
         # Before actual data copying we need perform several operation
         # such as: create all volumes, create fake template if needed, ...
-        try:
-            # Find all volumes of source image
-            srcChain = self.getChain(srcSdUUID, imgUUID)
-        except se.StorageException:
-            self.log.error("Unexpected error", exc_info=True)
-            raise
-        except Exception, e:
-            self.log.error("Unexpected error", exc_info=True)
-            raise se.SourceImageActionError(imgUUID, srcSdUUID, str(e))
+
+        # Find all volumes of source image
+        srcChainDict = self.getChain(srcSdUUID, imgUUID)
+        srcChain = srcChainDict['chain']
 
         fakeTemplate = False
         pimg = volume.BLANK_UUID    # standalone chain
         # check if the chain is build above a template, or it is a standalone
-        pvol = srcChain[0].getParentVolume()
+        pvol = srcChainDict['template']
         if pvol:
             # find out parent volume parameters
             volParams = pvol.getVolumeParams()
@@ -686,11 +666,9 @@ class Image:
         """
         if not self.isLegal(sdUUID, imgUUID):
             raise se.ImageIsNotLegalChain(imgUUID)
-        chain = self.getChain(sdUUID, imgUUID)
-        # check if the chain is build above a template, or it is a standalone
-        pvol = chain[0].getParentVolume()
-        if pvol:
-            if not pvol.isLegal() or pvol.isFake():
+        chainDict = self.getChain(sdUUID, imgUUID)
+        if chainDict['template']:
+            if not chainDict['template'].isLegal() or chainDict['template'].isFake():
                 raise se.ImageIsNotLegalChain(imgUUID)
 
     def copy(self, sdUUID, vmUUID, srcImgUUID, srcVolUUID, dstImgUUID, dstVolUUID,
@@ -735,8 +713,7 @@ class Image:
                     # using the sum of the actual size of the chain's volumes
                     if volParams['volFormat'] != volume.COW_FORMAT or volParams['prealloc'] != volume.SPARSE_VOL:
                         raise se.IncorrectFormat(self)
-                    volParams['apparentsize'] = self.__chainSizeCalc(sdUUID, srcImgUUID,
-                                                                   srcVolUUID, volParams['size'])
+                    volParams['apparentsize'] = self.__chainSizeCalc(sdUUID, srcImgUUID, volParams['size'])
 
                 # Find out dest volume parameters
                 if preallocate in [volume.PREALLOCATED_VOL, volume.SPARSE_VOL]:
diff --git a/vdsm/storage/resourceFactories.py b/vdsm/storage/resourceFactories.py
index ce3ec70..07df0f0 100644
--- a/vdsm/storage/resourceFactories.py
+++ b/vdsm/storage/resourceFactories.py
@@ -106,20 +106,18 @@ class ImageResourceFactory(rm.SimpleResourceFactory):
         # Get the list of the volumes
         repoPath = os.path.join(self.storage_repository, dom.getPools()[0])
         try:
-            chain = image.Image(repoPath).getChain(sdUUID=self.sdUUID, imgUUID=resourceName)
+            chainDict = image.Image(repoPath).getChain(sdUUID=self.sdUUID, imgUUID=resourceName)
+            chain = chainDict['chain']
         except se.ImageDoesNotExistInSD:
             log.debug("Image %s does not exist in domain %s", resourceName, self.sdUUID)
             return []
 
-        # check if the chain is build above a template, or it is a standalone
-        pvol = chain[0].getParentVolume()
-        if pvol:
-            template = pvol.volUUID
-        elif chain[0].isShared():
-            # Image of template itself,
-            # with no other volumes in chain
-            template = chain[0].volUUID
-            del chain[:]
+        # Image of template itself,
+        # with no other volumes in chain
+        if chainDict['template']:
+            template = chainDict['template'].volUUID
+            if chainDict['template'] in chain:
+                del chain[:]
 
         volUUIDChain = [vol.volUUID for vol in chain]
         volUUIDChain.sort()




More information about the vdsm-patches mailing list