Change in vdsm[master]: [wip] volume: make volume._share idempotent

[Federico Simoncelli]

Federico Simoncelli has uploaded a new change for review.

Change subject: [wip] volume: make volume._share idempotent
......................................................................

[wip] volume: make volume._share idempotent

With this patch it will possible to run the volume._share command
multiple times without harm (idempotent) for the image and for the
running VMs.

Signed-off-by: Federico Simoncelli 
Change-Id: I237e4a8f094ba04dcd4ef7bff418e03f81162d8d
---
M vdsm/storage/blockVolume.py
M vdsm/storage/fileVolume.py
M vdsm/storage/volume.py
3 files changed, 39 insertions(+), 16 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/70/8270/1

diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py
index 38285c1..ee8ba67 100644
--- a/vdsm/storage/blockVolume.py
+++ b/vdsm/storage/blockVolume.py
@@ -324,6 +324,7 @@
         dstPath = os.path.join(dstImgPath, self.volUUID)

self.log.debug("Share volume %s to %s", self.volUUID, dstImgPath)
+        # TODO: make this idempotent
         os.symlink(self.getDevPath(), dstPath)

@classmethod
diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py
index 0aa9043..2e76944 100644
--- a/vdsm/storage/fileVolume.py
+++ b/vdsm/storage/fileVolume.py
@@ -227,29 +227,49 @@
         """
         Share this volume to dstImgPath, including the metadata and the lease
         """
-        dstVolPath = os.path.join(dstImgPath, self.volUUID)
-        dstMetaPath = self._getMetaVolumePath(dstVolPath)
-
         self.log.debug("Share volume %s to %s", self.volUUID, dstImgPath)

-        self.oop.fileUtils.safeUnlink(dstVolPath)
-        self.oop.os.link(self.getVolumePath(), dstVolPath)
+        dstVolumePath = os.path.join(dstImgPath, self.volUUID)

-        self.log.debug("Share volume metadata of %s to %s", self.volUUID,
-                       dstImgPath)
+        # Format: itemName, srcPath, dstPath, itemRequired
+        itemsList = [
+            ('volume', self.getVolumePath(), dstVolumePath, True),
+            ('metadata', self._getMetaVolumePath(),
+                         self._getMetaVolumePath(dstVolumePath), True),
+            ('lease', self._getLeaseVolumePath(),
+                      self._getLeaseVolumePath(dstVolumePath),
+                      sdCache.produce(self.sdUUID).hasVolumeLeases()),
+        ]

-        self.oop.fileUtils.safeUnlink(dstMetaPath)
-        self.oop.os.link(self._getMetaVolumePath(), dstMetaPath)
+        for itemName, srcPath, dstPath, itemRequired in itemsList:
+            self.log.debug("Sharing %s %s to %s (required: %s)", itemName,
+                           self.volUUID, dstPath, itemRequired)

-        # Link the lease file if the domain uses sanlock
-        if sdCache.produce(self.sdUUID).hasVolumeLeases():
-            dstLeasePath = self._getLeaseVolumePath(dstVolPath)
+            try:
+                srcInfo = self.oop.stat(srcPath)
+            except OSError, e:
+                if e != os.errno.ENOENT or itemRequired:
+                    # Source not found
+                    self.log.debug("TODO: <message>")
+                    raise

-            self.log.debug("Share volume lease of %s to %s", self.volUUID,
-                           dstImgPath)
+            try:
+                dstInfo = self.oop.stat(dstPath)
+            except OSError, e:
+                if e != os.errno.ENOENT:
+                    raise

-            self.oop.fileUtils.safeUnlink(dstLeasePath)
-            self.oop.os.link(self._getLeaseVolumePath(), dstLeasePath)
+                # Destination file not present, linking
+                self.log.debug("TODO: <message>")
+                self.oop.os.link(srcPath, dstPath)
+            else:
+                # If both the source and destination are present and they are
+                # the same file (already linked) then continue.
+                if srcInfo.st_ino == dstInfo.st_ino:
+                    self.log.debug("TODO: <message>")
+                else:
+                    self.log.debug("TODO: <message>")
+                    raise OSError(os.errno.EEXIST, "Destination file exists")

@classmethod
     def shareVolumeRollback(cls, taskObj, volPath):
diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py
index b9eb356..7938c6f 100644
--- a/vdsm/storage/volume.py
+++ b/vdsm/storage/volume.py
@@ -297,6 +297,8 @@
             self._share(dstImgPath)

except Exception, e:
+            self.log.debug("Cannot share volume %s to %s", self.volUUID,
+                           dstImgPath, exc_info=True)
             raise se.CannotShareVolume(self.getVolumePath(), dstPath, str(e))

def refreshVolume(self):

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I237e4a8f094ba04dcd4ef7bff418e03f81162d8d
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli