Change in vdsm[master]: ImageManifest: move getChain

Adam Litke has uploaded a new change for review.

Change subject: ImageManifest: move getChain
......................................................................

ImageManifest: move getChain

The implementation of getChain instantiates storagedomain and volume
objects.  This must be fixed.  We must fix all callers of getChain to
work with VolumeMetadata objects instead of Volume objects.

Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb
Signed-off-by: Adam Litke 
---
M vdsm/storage/image.py
1 file changed, 71 insertions(+), 66 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/49/44049/1

diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index bd79f6e..f852f3c 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -89,6 +89,8 @@

class ImageManifest(object):
+    log = logging.getLogger('Storage.ImageManifest')
+
     def __init__(self, repoPath):
         self.repoPath = repoPath

@@ -105,6 +107,74 @@
             log.info("Create placeholder %s for image's volumes", img_dir)
             os.mkdir(img_dir)
         return img_dir
+
+    def getChain(self, sdUUID, imgUUID, volUUID=None):
+        """
+        Return the chain of volumes of image as a sorted list
+        (not including a shared base (template) if any)
+        """
+        chain = []
+        volclass = sdCache.produce(sdUUID).getVolumeClass()
+
+        # Use volUUID when provided
+        if volUUID:
+            srcVol = volclass(self.repoPath, sdUUID, imgUUID, volUUID)
+
+            # For template images include only one volume (the template itself)
+            # NOTE: this relies on the fact that in a template there is only
+            #       one volume
+            if srcVol.isShared():
+                return [srcVol]
+
+        # Find all the volumes when volUUID is not provided
+        else:
+            # Find all volumes of image
+            uuidlist = volclass.getImageVolumes(self.repoPath, sdUUID, imgUUID)
+
+            if not uuidlist:
+                raise se.ImageDoesNotExistInSD(imgUUID, sdUUID)
+
+            srcVol = volclass(self.repoPath, sdUUID, imgUUID, uuidlist[0])
+
+            # For template images include only one volume (the template itself)
+            if len(uuidlist) == 1 and srcVol.isShared():
+                return [srcVol]
+
+            # Searching for the leaf
+            for vol in uuidlist:
+                srcVol = volclass(self.repoPath, sdUUID, imgUUID, vol)
+
+                if srcVol.isLeaf():
+                    break
+
+                srcVol = None
+
+            if not srcVol:
+                self.log.error("There is no leaf in the image %s", imgUUID)
+                raise se.ImageIsNotLegalChain(imgUUID)
+
+        # We have seen corrupted chains that cause endless loops here.
+        # https://bugzilla.redhat.com/1125197
+        seen = set()
+
+        # Build up the sorted parent -> child chain
+        while not srcVol.isShared():
+            chain.insert(0, srcVol)
+            seen.add(srcVol.volUUID)
+
+            parentUUID = srcVol.getParentId()
+            if parentUUID == volume.BLANK_UUID:
+                break
+
+            if parentUUID in seen:
+                self.log.error("Image %s volume %s has invalid parent UUID %s",
+                               imgUUID, srcVol.volUUID, parentUUID)
+                raise se.ImageIsNotLegalChain(imgUUID)
+
+            srcVol = srcVol.produceParent()
+
+        self.log.info("sdUUID=%s imgUUID=%s chain=%s ", sdUUID, imgUUID, chain)
+        return chain

class Image:
@@ -179,72 +249,7 @@
         return newsize

def getChain(self, sdUUID, imgUUID, volUUID=None):
-        """
-        Return the chain of volumes of image as a sorted list
-        (not including a shared base (template) if any)
-        """
-        chain = []
-        volclass = sdCache.produce(sdUUID).getVolumeClass()
-
-        # Use volUUID when provided
-        if volUUID:
-            srcVol = volclass(self.repoPath, sdUUID, imgUUID, volUUID)
-
-            # For template images include only one volume (the template itself)
-            # NOTE: this relies on the fact that in a template there is only
-            #       one volume
-            if srcVol.isShared():
-                return [srcVol]
-
-        # Find all the volumes when volUUID is not provided
-        else:
-            # Find all volumes of image
-            uuidlist = volclass.getImageVolumes(self.repoPath, sdUUID, imgUUID)
-
-            if not uuidlist:
-                raise se.ImageDoesNotExistInSD(imgUUID, sdUUID)
-
-            srcVol = volclass(self.repoPath, sdUUID, imgUUID, uuidlist[0])
-
-            # For template images include only one volume (the template itself)
-            if len(uuidlist) == 1 and srcVol.isShared():
-                return [srcVol]
-
-            # Searching for the leaf
-            for vol in uuidlist:
-                srcVol = volclass(self.repoPath, sdUUID, imgUUID, vol)
-
-                if srcVol.isLeaf():
-                    break
-
-                srcVol = None
-
-            if not srcVol:
-                self.log.error("There is no leaf in the image %s", imgUUID)
-                raise se.ImageIsNotLegalChain(imgUUID)
-
-        # We have seen corrupted chains that cause endless loops here.
-        # https://bugzilla.redhat.com/1125197
-        seen = set()
-
-        # Build up the sorted parent -> child chain
-        while not srcVol.isShared():
-            chain.insert(0, srcVol)
-            seen.add(srcVol.volUUID)
-
-            parentUUID = srcVol.getParentId()
-            if parentUUID == volume.BLANK_UUID:
-                break
-
-            if parentUUID in seen:
-                self.log.error("Image %s volume %s has invalid parent UUID %s",
-                               imgUUID, srcVol.volUUID, parentUUID)
-                raise se.ImageIsNotLegalChain(imgUUID)
-
-            srcVol = srcVol.produceParent()
-
-        self.log.info("sdUUID=%s imgUUID=%s chain=%s ", sdUUID, imgUUID, chain)
-        return chain
+        return self.manifest.getChain(sdUUID, imgUUID, volUUID)

def getTemplate(self, sdUUID, imgUUID):
         """

[e-mail] has posted comments on this change.

Change subject: ImageManifest: move getChain
......................................................................

Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[e-mail] has posted comments on this change.

Change subject: ImageManifest: move getChain
......................................................................

Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[e-mail] has posted comments on this change.

Change subject: ImageManifest: move getChain
......................................................................

Patch Set 3:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[e-mail] has posted comments on this change.

Change subject: ImageManifest: move getChain
......................................................................

Patch Set 4:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[e-mail] has posted comments on this change.

Change subject: ImageManifest: move getChain
......................................................................

Patch Set 5:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

[Jenkins CI RO]

Jenkins CI RO has abandoned this change.

Change subject: ImageManifest: move getChain
......................................................................

Abandoned

Abandoned due to no activity - please restore if still relevant

gerrit-hooks has posted comments on this change.

Change subject: ImageManifest: move getChain
......................................................................

Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found