Change in vdsm[master]: Relink template hard links to meta and lease files

Yeela Kaplan has uploaded a new change for review.

Change subject: Relink template hard links to meta and lease files
......................................................................

Relink template hard links to meta and lease files

Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=864073
Signed-off-by: Yeela Kaplan 
---
M vdsm/storage/fileSD.py
M vdsm/storage/fileVolume.py
M vdsm/storage/image.py
M vdsm/storage/sd.py
4 files changed, 63 insertions(+), 38 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/37/12837/1

diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py
index fa837fc..1d7e7d7 100644
--- a/vdsm/storage/fileSD.py
+++ b/vdsm/storage/fileSD.py
@@ -519,6 +519,50 @@
         for imageDir in removedImages:
             self.oop.fileUtils.cleanupdir(imageDir)

+    def templateRelink(self, imgUUID, volUUID):
+        """
+        Relink all hardlinks of the template 'volUUID' in all VMs based on it.
+
+        This function assumes that dom is backup dom and that template image is
+        used by other volumes.
+        """
+        # Avoid relink templates for non-NFS domains
+        if self.getStorageType() not in [sd.NFS_DOMAIN]:
+            self.log.debug("Doesn't relink templates non-NFS domain %s",
+                           self.sdUUID)
+            return
+
+        allVols = self.getAllVolumes()
+        tImgs = allVols[volUUID].imgs
+        if len(tImgs) < 2:
+            self.log.debug("Volume %s is an unused template or a regular "
+                           "volume. Found  in images: %s allVols: %s", volUUID,
+                           tImgs, allVols)
+            return
+        templateImage = tImgs[0]
+        relinkImgs = tuple(tImgs[1:])
+        for rImg in relinkImgs:
+            # This function assumes that all relevant images and template
+            # namespaces are locked.
+            repoPath = self._getRepoPath()
+            tLink = os.path.join(repoPath, self.sdUUID,
+                                 sd.DOMAIN_IMAGES, rImg, volUUID)
+            for ext in ['', fileVolume.META_FILEEXT]:
+                self.oop.os.unlink(tLink + ext)
+                self.oop.os.link(os.path.join(repoPath, self.sdUUID,
+                                 sd.DOMAIN_IMAGES, templateImage,
+                                 volUUID + ext), tLink + ext)
+
+            try:
+                self.oop.os.unlink(tLink + fileVolume.LEASE_FILEEXT)
+                self.oop.os.link(os.path.join(
+                    repoPath, self.sdUUID, sd.DOMAIN_IMAGES,
+                    templateImage, volUUID + fileVolume.LEASE_FILEEXT),
+                    tLink + fileVolume.LEASE_FILEEXT)
+            except OSError as e:
+                if e.errno != os.errno.ENOENT:
+                    raise
+

def getMountsList(pattern="*"):
     finalPat = os.path.join(sd.StorageDomain.storage_repository,
diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py
index 01575e0..6e12cd8 100644
--- a/vdsm/storage/fileVolume.py
+++ b/vdsm/storage/fileVolume.py
@@ -34,6 +34,7 @@
 import task
 from threadLocal import vars

+META_FILEEXT = ".meta"
 LEASE_FILEEXT = ".lease"
 LEASE_FILEOFFSET = 0

@@ -538,7 +539,7 @@
     @classmethod
     def __metaVolumePath(cls, vol_path):
         if vol_path:
-            return vol_path + '.meta'
+            return vol_path + META_FILEEXT
         else:
             return None

diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index 025885f..3eada2b 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -36,7 +36,6 @@
 from threadLocal import vars
 import resourceFactories
 import resourceManager as rm
-import outOfProcess as oop

log = logging.getLogger('Storage.Image')
 rmanager = rm.ResourceManager.getInstance()
@@ -368,38 +367,6 @@
         # Do not deactivate the template yet (might be in use by an other vm)
         # TODO: reference counting to deactivate when unused

-    def __templateRelink(self, destDom, imgUUID, volUUID):
-        """
-        Relink all hardlinks of the template 'volUUID' in all VMs based on it.
-
-        This function assumes that dom is backup dom and that template image is
-        used by other volumes.
-        """
-        # Avoid relink templates for non-NFS domains
-        if destDom.getStorageType() not in [sd.NFS_DOMAIN]:
-            self.log.debug("Doesn't relink templates non-NFS domain %s",
-                           destDom.sdUUID)
-            return
-
-        allVols = destDom.getAllVolumes()
-        tImgs = allVols[volUUID].imgs
-        if len(tImgs) < 2:
-            self.log.debug("Volume %s is an unused template or a regular "
-                           "volume. Found  in images: %s allVols: %s", volUUID,
-                           tImgs, allVols)
-            return
-        templateImage = tImgs[0]
-        relinkImgs = tuple(tImgs[1:])
-        for rImg in relinkImgs:
-            # This function assumes that all relevant images and template
-            # namespaces are locked.
-            tLink = os.path.join(self.repoPath, destDom.sdUUID,
-                                 sd.DOMAIN_IMAGES, rImg, volUUID)
-            oop.getProcessPool(destDom.sdUUID).os.unlink(tLink)
-            oop.getProcessPool(destDom.sdUUID).os.link(os.path.join(
-                self.repoPath, destDom.sdUUID, sd.DOMAIN_IMAGES, templateImage,
-                volUUID), tLink)
-
     def createFakeTemplate(self, sdUUID, volParams):
         """
         Create fake template (relevant for Backup domain only)
@@ -431,8 +398,8 @@
                     vol.setShared()
                     # Now we should re-link all hardlinks of this template in
                     # all VMs based on it
-                    self.__templateRelink(destDom, volParams['imgUUID'],
-                                          volParams['volUUID'])
+                    destDom.templateRelink(volParams['imgUUID'],
+                                           volParams['volUUID'])

self.log.debug("Succeeded to create fake image %s in "
                                    "domain %s", volParams['imgUUID'],
@@ -658,7 +625,7 @@
         if force:
             leafVol = chains['dstChain'][-1]
             # Now we should re-link all deleted hardlinks, if exists
-            self.__templateRelink(destDom, imgUUID, leafVol.volUUID)
+            destDom.templateRelink(imgUUID, leafVol.volUUID)

# At this point we successfully finished the 'copy' part of the
         # operation and we can clear all recoveries.
@@ -931,7 +898,7 @@

if force:
                     # Now we should re-link all deleted hardlinks, if exists
-                    self.__templateRelink(destDom, dstImgUUID, dstVolUUID)
+                    destDom.templateRelink(dstImgUUID, dstVolUUID)
             except se.StorageException:
                 self.log.error("Unexpected error", exc_info=True)
                 raise
diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py
index 9ce836b..c65abac 100644
--- a/vdsm/storage/sd.py
+++ b/vdsm/storage/sd.py
@@ -785,3 +785,16 @@
         (on NFS mostly) due to lazy file removal
         """
         pass
+
+    def templateRelink(self, imgUUID, volUUID):
+        """
+        Relink all hardlinks of the template 'volUUID' in all VMs based on it.
+
+        This function assumes that dom is backup dom and that template image is
+        used by other volumes.
+        """
+        # Avoid relink templates for non-NFS domains
+        if self.getStorageType() not in [NFS_DOMAIN]:
+            self.log.debug("Doesn't relink templates non-NFS domain %s",
+                           self.sdUUID)
+            return

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1475/ (2/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 1:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1512/ (1/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1512/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1475/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(5 inline comments)

....................................................
Commit Message
Line 3: AuthorDate: 2013-03-05 11:36:36 +0200
Line 4: Commit:     Yeela Kaplan 
Line 5: CommitDate: 2013-03-07 17:15:38 +0200
Line 6: 
Line 7: Relink template hard links to meta and lease files
why?
Line 8: 
Line 9: Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Line 10: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=864073

....................................................
File vdsm/storage/fileSD.py
Line 526:         This function assumes that dom is backup dom and that template image is
Line 527:         used by other volumes.
Line 528:         """
Line 529:         # Avoid relink templates for non-NFS domains
Line 530:         if self.getStorageType() not in [sd.NFS_DOMAIN]:
why only NFS and not all file domains? (hard links are supported by all posix compliant file systems and then some)
Line 531:             self.log.debug("Doesn't relink templates non-NFS domain %s",
Line 532:                            self.sdUUID)
Line 533:             return
Line 534:

Line 545:             # This function assumes that all relevant images and template
Line 546:             # namespaces are locked.
Line 547:             repoPath = self._getRepoPath()
Line 548:             tLink = os.path.join(repoPath, self.sdUUID,
Line 549:                                  sd.DOMAIN_IMAGES, rImg, volUUID)
How about:

tVolPath = os.path.join(repoPath, selfSdUUID, sd.DOMAIN_IMAGES, "%s", volUUID)
for ext...
    linkName = tVolPath % rImg + ext  # or if you prefer: (tvolPath + ext) % rimg
    tVolName = tVolPath % templateImage + ext
    self.oop.os.unlink(linkName)
    self.oop.os.link(tVolName, linkName)
Line 550:             for ext in ['', fileVolume.META_FILEEXT]:
Line 551:                 self.oop.os.unlink(tLink + ext)
Line 552:                 self.oop.os.link(os.path.join(repoPath, self.sdUUID,
Line 553:                                  sd.DOMAIN_IMAGES, templateImage,

Line 558:                 self.oop.os.link(os.path.join(
Line 559:                     repoPath, self.sdUUID, sd.DOMAIN_IMAGES,
Line 560:                     templateImage, volUUID + fileVolume.LEASE_FILEEXT),
Line 561:                     tLink + fileVolume.LEASE_FILEEXT)
Line 562:             except OSError as e:
I see no reason not to ignore the 'fake' volume / metafile not existing just the same and unify this into the loop above
i.e.:
for ext in ['', fileVolume.META_FILEEXT, fileVolume.LEASE_FILEEXT]:
    try:
        ...
    except OSError as e:
        ...
Line 563:                 if e.errno != os.errno.ENOENT:
Line 564:                     raise
Line 565: 
Line 566:

....................................................
File vdsm/storage/sd.py
Line 793:         This function assumes that dom is backup dom and that template image is
Line 794:         used by other volumes.
Line 795:         """
Line 796:         # Avoid relink templates for non-NFS domains
Line 797:         if self.getStorageType() not in [NFS_DOMAIN]:
since fileSD inherits from sd this 'if' will always be true and if it isn't then we most certainly want to log that
Line 798:             self.log.debug("Doesn't relink templates non-NFS domain %s",
Line 799:                            self.sdUUID)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

Eduardo has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(3 inline comments)

....................................................
File vdsm/storage/fileSD.py
Line 525: 
Line 526:         This function assumes that dom is backup dom and that template image is
Line 527:         used by other volumes.
Line 528:         """
Line 529:         # Avoid relink templates for non-NFS domains
This check is redundant since an instance of this class _is_ a FileSD image and should be re-linked.
Line 530:         if self.getStorageType() not in [sd.NFS_DOMAIN]:
Line 531:             self.log.debug("Doesn't relink templates non-NFS domain %s",
Line 532:                            self.sdUUID)
Line 533:             return

....................................................
File vdsm/storage/fileVolume.py
Line 540:     def __metaVolumePath(cls, vol_path):
Line 541:         if vol_path:
Line 542:             return vol_path + META_FILEEXT
Line 543:         else:
Line 544:             return None
vol_path is mandatory and I see no reason for dealing with a "None" (or False or weahtever param.)
Should be removed in __leaseVolumePath too.
Line 545: 
Line 546:     @classmethod
Line 547:     def __leaseVolumePath(cls, vol_path):
Line 548:         if vol_path:

....................................................
File vdsm/storage/sd.py
Line 785:         (on NFS mostly) due to lazy file removal
Line 786:         """
Line 787:         pass
Line 788: 
Line 789:     def templateRelink(self, imgUUID, volUUID):
Since this class should be not instantiated in any form, this method should not be here.
Only instances that not support re-link, i.e. BlockSD, should have it, or gracefully raise an AttributeError (default).
IMHO trying to do this should be and error.
But this will be addressed in a further patch.
Line 790:         """
Line 791:         Relink all hardlinks of the template 'volUUID' in all VMs based on it.
Line 792: 
Line 793:         This function assumes that dom is backup dom and that template image is

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: oVirt Jenkins CI Server

Yeela Kaplan has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 1: (8 inline comments)

....................................................
Commit Message
Line 3: AuthorDate: 2013-03-05 11:36:36 +0200
Line 4: Commit:     Yeela Kaplan 
Line 5: CommitDate: 2013-03-07 17:15:38 +0200
Line 6: 
Line 7: Relink template hard links to meta and lease files
Done
Line 8: 
Line 9: Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Line 10: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=864073

....................................................
File vdsm/storage/fileSD.py
Line 525: 
Line 526:         This function assumes that dom is backup dom and that template image is
Line 527:         used by other volumes.
Line 528:         """
Line 529:         # Avoid relink templates for non-NFS domains
Done
Line 530:         if self.getStorageType() not in [sd.NFS_DOMAIN]:
Line 531:             self.log.debug("Doesn't relink templates non-NFS domain %s",
Line 532:                            self.sdUUID)
Line 533:             return

Line 526:         This function assumes that dom is backup dom and that template image is
Line 527:         used by other volumes.
Line 528:         """
Line 529:         # Avoid relink templates for non-NFS domains
Line 530:         if self.getStorageType() not in [sd.NFS_DOMAIN]:
Done
Line 531:             self.log.debug("Doesn't relink templates non-NFS domain %s",
Line 532:                            self.sdUUID)
Line 533:             return
Line 534:

Line 545:             # This function assumes that all relevant images and template
Line 546:             # namespaces are locked.
Line 547:             repoPath = self._getRepoPath()
Line 548:             tLink = os.path.join(repoPath, self.sdUUID,
Line 549:                                  sd.DOMAIN_IMAGES, rImg, volUUID)
Done
Line 550:             for ext in ['', fileVolume.META_FILEEXT]:
Line 551:                 self.oop.os.unlink(tLink + ext)
Line 552:                 self.oop.os.link(os.path.join(repoPath, self.sdUUID,
Line 553:                                  sd.DOMAIN_IMAGES, templateImage,

Line 558:                 self.oop.os.link(os.path.join(
Line 559:                     repoPath, self.sdUUID, sd.DOMAIN_IMAGES,
Line 560:                     templateImage, volUUID + fileVolume.LEASE_FILEEXT),
Line 561:                     tLink + fileVolume.LEASE_FILEEXT)
Line 562:             except OSError as e:
Done
Line 563:                 if e.errno != os.errno.ENOENT:
Line 564:                     raise
Line 565: 
Line 566:

....................................................
File vdsm/storage/fileVolume.py
Line 540:     def __metaVolumePath(cls, vol_path):
Line 541:         if vol_path:
Line 542:             return vol_path + META_FILEEXT
Line 543:         else:
Line 544:             return None
this is less relevant to the patch and is a change of behavior. I would prefer to leave this discussion to a different patch.
Also when we deal with NFS we don't always have a lease file (only on v3) .
Line 545: 
Line 546:     @classmethod
Line 547:     def __leaseVolumePath(cls, vol_path):
Line 548:         if vol_path:

....................................................
File vdsm/storage/sd.py
Line 785:         (on NFS mostly) due to lazy file removal
Line 786:         """
Line 787:         pass
Line 788: 
Line 789:     def templateRelink(self, imgUUID, volUUID):
Done
Line 790:         """
Line 791:         Relink all hardlinks of the template 'volUUID' in all VMs based on it.
Line 792: 
Line 793:         This function assumes that dom is backup dom and that template image is

Line 793:         This function assumes that dom is backup dom and that template image is
Line 794:         used by other volumes.
Line 795:         """
Line 796:         # Avoid relink templates for non-NFS domains
Line 797:         if self.getStorageType() not in [NFS_DOMAIN]:
Done
Line 798:             self.log.debug("Doesn't relink templates non-NFS domain %s",
Line 799:                            self.sdUUID)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1541/ (1/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 2:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1504/ (2/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1541/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1504/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 2: I would prefer that you didn't submit this

(5 inline comments)

....................................................
Commit Message
Line 5: CommitDate: 2013-03-10 16:04:22 +0200
Line 6: 
Line 7: Relink template hard links to meta and lease files
Line 8: 
Line 9: Every image that is based on a template contains a hard
In file based domains every image...
Line 10: link to the template volumes.
Line 11: When copying the template to the storage domain to replace
Line 12: the old/fake one we relink the volumes in the dependant
Line 13: image to the new template volumes.

Line 7: Relink template hard links to meta and lease files
Line 8: 
Line 9: Every image that is based on a template contains a hard
Line 10: link to the template volumes.
Line 11: When copying the template to the storage domain to replace
s/.*/Since an export domain may contain images without the backing template,
when copying the template to the export domain we need to relink the derived images to it to be able to later on collapse the chain on import (without qemu-img crashing). In addition, in case we're importing a template to override a corrupted template we also need to relink.
Line 12: the old/fake one we relink the volumes in the dependant
Line 13: image to the new template volumes.
Line 14: 
Line 15: Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0

....................................................
File vdsm/storage/blockSD.py
Line 1250:         """
Line 1251:         Relink all hardlinks of the template 'volUUID' in all VMs based on it.
Line 1252:         No need to relink template for block domains.
Line 1253:         """
Line 1254:         self.log.debug("Doesn't relink templates non-File domain %s",
s/.*/Skipping relink of template, domain %s is not file based/
Line 1255:                        self.sdUUID)
Line 1256: 
Line 1257: 
Line 1258: def _createVMSfs(dev):

....................................................
File vdsm/storage/fileSD.py
Line 544:             for ext in ['', fileVolume.META_FILEEXT, fileVolume.LEASE_FILEEXT]:
Line 545:                 tLink = (tVolPath + ext) % rImg
Line 546:                 tVol = (tVolPath + ext) % templateImage
Line 547:                 try:
Line 548:                     self.oop.os.unlink(tLink)
sorry for not noticing this before but we should be using self.oop.fileUtils.safeUnlink
and remove the try except here.
ENOENT is relevant for the unlink only (and handled in safeUnlink) and here we wouldn't be linking the template in case the fakeTemplate is missing for some reason which doesn't seem right.
Line 549:                     self.oop.os.link(tVol, tLink)
Line 550:                 except OSError as e:
Line 551:                     if e.errno != os.errno.ENOENT:
Line 552:                         raise

....................................................
File vdsm/storage/fileVolume.py
Line 538: 
Line 539:     @classmethod
Line 540:     def __metaVolumePath(cls, vol_path):
Line 541:         if vol_path:
Line 542:             return vol_path + META_FILEEXT
since you're making changes here, it would be nice (but not mandatory) if you renamed vol_path to volPath
Line 543:         else:
Line 544:             return None
Line 545: 
Line 546:     @classmethod

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Yeela Kaplan has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 2: (5 inline comments)

....................................................
Commit Message
Line 5: CommitDate: 2013-03-10 16:04:22 +0200
Line 6: 
Line 7: Relink template hard links to meta and lease files
Line 8: 
Line 9: Every image that is based on a template contains a hard
Done
Line 10: link to the template volumes.
Line 11: When copying the template to the storage domain to replace
Line 12: the old/fake one we relink the volumes in the dependant
Line 13: image to the new template volumes.

Line 7: Relink template hard links to meta and lease files
Line 8: 
Line 9: Every image that is based on a template contains a hard
Line 10: link to the template volumes.
Line 11: When copying the template to the storage domain to replace
Done
Line 12: the old/fake one we relink the volumes in the dependant
Line 13: image to the new template volumes.
Line 14: 
Line 15: Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0

....................................................
File vdsm/storage/blockSD.py
Line 1250:         """
Line 1251:         Relink all hardlinks of the template 'volUUID' in all VMs based on it.
Line 1252:         No need to relink template for block domains.
Line 1253:         """
Line 1254:         self.log.debug("Doesn't relink templates non-File domain %s",
Done
Line 1255:                        self.sdUUID)
Line 1256: 
Line 1257: 
Line 1258: def _createVMSfs(dev):

....................................................
File vdsm/storage/fileSD.py
Line 544:             for ext in ['', fileVolume.META_FILEEXT, fileVolume.LEASE_FILEEXT]:
Line 545:                 tLink = (tVolPath + ext) % rImg
Line 546:                 tVol = (tVolPath + ext) % templateImage
Line 547:                 try:
Line 548:                     self.oop.os.unlink(tLink)
Done
Line 549:                     self.oop.os.link(tVol, tLink)
Line 550:                 except OSError as e:
Line 551:                     if e.errno != os.errno.ENOENT:
Line 552:                         raise

....................................................
File vdsm/storage/fileVolume.py
Line 538: 
Line 539:     @classmethod
Line 540:     def __metaVolumePath(cls, vol_path):
Line 541:         if vol_path:
Line 542:             return vol_path + META_FILEEXT
Done
Line 543:         else:
Line 544:             return None
Line 545: 
Line 546:     @classmethod

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 3:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1690/ (1/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 3:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1645/ (2/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 3: Fails

Build Failed

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1645/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1690/ : FAILURE

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 4:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1731/ (2/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 4:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1782/ (1/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 4:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1731/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1782/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 4: (2 inline comments)

....................................................
File vdsm/storage/blockSD.py
Line 1245:             return vg.name
Line 1246: 
Line 1247:         raise se.StorageDomainDoesNotExist()
Line 1248: 
Line 1249:     def templateRelink(self, imgUUID, volUUID):
I feel like we already discussed this (maybe on irc/phone?). I think I suggested to move this to sd.py so that it's inherited by default by all the other classes.
Line 1250:         """
Line 1251:         Relink all hardlinks of the template 'volUUID' in all VMs based on it.
Line 1252:         No need to relink template for block domains.
Line 1253:         """

....................................................
File vdsm/storage/fileSD.py
Line 542:             # namespaces are locked.
Line 543: 
Line 544:             for ext in ['', fileVolume.META_FILEEXT, fileVolume.LEASE_FILEEXT]:
Line 545:                 tLink = (tVolPath + ext) % rImg
Line 546:                 tVol = (tVolPath + ext) % templateImage
I'm not particularly fond of using string formatting for paths (one quick reason is that "%" is a valid character for a path and you might end up formatting what you're not expecting). What about:

basePath = os.path.join(self._getRepoPath(), self.sdUUID, sd.DOMAIN_IMAGES)

for volFile in [volUUID, volUUID + fileVolume.META_FILEEXT,
                 volUUID + fileVolume.LEASE_FILEEXT]:
     templateLink = os.path.join(basePath, rImg, volFile)
     templateVolume = os.path.join(basePath, templateImage, volFile)
Line 547:                 self.oop.fileUtils.safeUnlink(tLink)
Line 548:                 self.oop.os.link(tVol, tLink)
Line 549: 
Line 550:

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Eduardo has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 4: (1 inline comment)

....................................................
File vdsm/storage/blockSD.py
Line 1245:             return vg.name
Line 1246: 
Line 1247:         raise se.StorageDomainDoesNotExist()
Line 1248: 
Line 1249:     def templateRelink(self, imgUUID, volUUID):
templateRelink() is not an StorageDomain method. Is a special method of FileStorageDomains. Then it should not be inherited be all the SDs.
Really, it should not be called for blockStorageDomains and will not, when we will  finished with the split of image functions. Meanwhile this is BC.
Line 1250:         """
Line 1251:         Relink all hardlinks of the template 'volUUID' in all VMs based on it.
Line 1252:         No need to relink template for block domains.
Line 1253:         """

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Yeela Kaplan has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 4: (1 inline comment)

....................................................
File vdsm/storage/fileSD.py
Line 542:             # namespaces are locked.
Line 543: 
Line 544:             for ext in ['', fileVolume.META_FILEEXT, fileVolume.LEASE_FILEEXT]:
Line 545:                 tLink = (tVolPath + ext) % rImg
Line 546:                 tVol = (tVolPath + ext) % templateImage
Done
Line 547:                 self.oop.fileUtils.safeUnlink(tLink)
Line 548:                 self.oop.os.link(tVol, tLink)
Line 549: 
Line 550:

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1791/ (1/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1740/ (2/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 5:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1740/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1791/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 5: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 5: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/fileSD.py
Line 543:                             volUUID + fileVolume.LEASE_FILEEXT]:
Line 544:                 tLink = os.path.join(basePath, rImg, volFile)
Line 545:                 tVol = os.path.join(basePath, templateImage, volFile)
Line 546:                 self.oop.fileUtils.safeUnlink(tLink)
Line 547:                 self.oop.os.link(tVol, tLink)
Now that I think of it you should also be careful with the lease file because it might not be present (and this would probably fail). The volume and the meta file are mandatory but you might want to add the lease file to the list only if the domain version is >= 3 (you should probably use hasVolumeLeases).
Line 548: 
Line 549: 
Line 550: def getMountsList(pattern="*"):
Line 551:     finalPat = os.path.join(sd.StorageDomain.storage_repository,

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Yeela Kaplan has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 5: (1 inline comment)

....................................................
File vdsm/storage/fileSD.py
Line 543:                             volUUID + fileVolume.LEASE_FILEEXT]:
Line 544:                 tLink = os.path.join(basePath, rImg, volFile)
Line 545:                 tVol = os.path.join(basePath, templateImage, volFile)
Line 546:                 self.oop.fileUtils.safeUnlink(tLink)
Line 547:                 self.oop.os.link(tVol, tLink)
Done
Line 548: 
Line 549: 
Line 550: def getMountsList(pattern="*"):
Line 551:     finalPat = os.path.join(sd.StorageDomain.storage_repository,

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 6:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1763/ (1/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 6:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1814/ (2/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 6:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1763/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1814/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 6: (1 inline comment)

....................................................
File vdsm/storage/fileSD.py
Line 543:                             volUUID + fileVolume.LEASE_FILEEXT]:
Line 544:                 tLink = os.path.join(basePath, rImg, volFile)
Line 545:                 tVol = os.path.join(basePath, templateImage, volFile)
Line 546:                 self.oop.fileUtils.safeUnlink(tLink)
Line 547:                 if self.hasVolumeLeases() or not volFile.contains(
this 'if' is confusing.  Instead I'd suggest something like:
files = [volUUID, volUUID + fileVolume.META_FILEEXT]
if self.hasVolumeLeases():
    files.append(volUUID + fileVolume.LEASE_FILEEXT)
for volFile in files:
...
Line 548:                         fileVolume.LEASE_FILEEXT):
Line 549:                     self.oop.os.link(tVol, tLink)
Line 550: 
Line 551:

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 7:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1842/ (2/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 7:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1791/ (1/2)

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 7:

Build Successful

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/1791/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/1842/ : SUCCESS

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Federico Simoncelli]

Federico Simoncelli has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 7: Looks good to me, but someone else must approve

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Yeela Kaplan has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 7: Verified

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

Ayal Baron has posted comments on this change.

Change subject: Relink template hard links to meta and lease files
......................................................................

Patch Set 7: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Relink template hard links to meta and lease files
......................................................................

Relink template hard links to meta and lease files

In file based domains every image that is based on a
template contains a hard link to the template volumes.

Since an export domain may contain images without the
backing template, when copying the template to the export
domain we need to relink the derived images to it to be able
to later on collapse the chain on import
(without qemu-img crashing).
In addition, in case we're importing a template to override a
corrupted template we also need to relink.

Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=864073
Signed-off-by: Yeela Kaplan 
---
M vdsm/storage/fileSD.py
M vdsm/storage/fileVolume.py
M vdsm/storage/image.py
M vdsm/storage/sd.py
4 files changed, 45 insertions(+), 40 deletions(-)

Approvals:
  Ayal Baron: Looks good to me, approved
  Yeela Kaplan: Verified
  Federico Simoncelli: Looks good to me, but someone else must approve
  Dan Kenigsberg:

--
To view, visit http://gerrit.ovirt.org/12837
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan 
Gerrit-Reviewer: Ayal Baron 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: oVirt Jenkins CI Server