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 <alitke(a)redhat.com&gt;
---
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):
         """


-- 
To view, visit https://gerrit.ovirt.org/44049
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;

automation(a)ovirt.org 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'])

-- 
To view, visit https://gerrit.ovirt.org/44049
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org 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'])

-- 
To view, visit https://gerrit.ovirt.org/44049
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org 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'])

-- 
To view, visit https://gerrit.ovirt.org/44049
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org 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'])

-- 
To view, visit https://gerrit.ovirt.org/44049
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org 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'])

-- 
To view, visit https://gerrit.ovirt.org/44049
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[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

-- 
To view, visit https://gerrit.ovirt.org/44049
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;

gerrit-hooks has posted comments on this change.

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


Patch Set 5:

* Update tracker: IGNORE, no Bug-Url found

-- 
To view, visit https://gerrit.ovirt.org/44049
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iffd24be3ba74ccdea8dfc744b579c0cf63771cdb
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;
Gerrit-HasComments: No