Change in vdsm[master]: Remove redundant calls that slow down getVolumeInfo

Yeela Kaplan has uploaded a new change for review.

Change subject: Remove redundant calls that slow down getVolumeInfo
......................................................................

Remove redundant calls that slow down getVolumeInfo

Change-Id: I30639134f9200fb6bfcf605b5d8fbc483b51d6e7
Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com&gt;
---
M vdsm/storage/blockVolume.py
M vdsm/storage/fileVolume.py
M vdsm/storage/volume.py
3 files changed, 47 insertions(+), 40 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/73/18273/1

diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py
index effd3a5..63d1593 100644
--- a/vdsm/storage/blockVolume.py
+++ b/vdsm/storage/blockVolume.py
@@ -631,24 +631,22 @@
         """
         Return the volume size in blocks
         """
-        # Just call the class method getVSize() - apparently it does what
-        # we need. We consider incurred overhead of producing the SD object
-        # to be a small price for code de-duplication.
         sdobj = sdCache.produce(sdUUID=self.sdUUID)
         return self.getVSize(sdobj, self.imgUUID, self.volUUID, bs)
 
-    getVolumeTrueSize = getVolumeSize
-
-    def getVolumeMtime(self):
+    def getVolumeSizes(self, bs=BLOCK_SIZE):
         """
-        Return the volume mtime in msec epoch
+        Logical volumes apparent and true size are the same so return the
+        volume size for both
         """
-        try:
-            mtime = self.getMetaParam(volume.MTIME)
-        except se.MetaDataKeyNotFoundError:
-            mtime = 0
+        size = self.getVolumeSize(bs)
+        return (size, size)
 
-        return mtime
+    def getMtime(self):
+        """
+        logical volumes don't have a modified time, so just return 0
+        """
+        return 0
 
     def _extendSizeRaw(self, newSize):
         # Since this method relies on lvm.extendLV (lvextend) when the
diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py
index cc61dde..337026b 100644
--- a/vdsm/storage/fileVolume.py
+++ b/vdsm/storage/fileVolume.py
@@ -405,18 +405,6 @@
         self.setMetaParam(volume.IMAGE, imgUUID)
 
     @classmethod
-    def getVSize(cls, sdobj, imgUUID, volUUID, bs=BLOCK_SIZE):
-        volPath = os.path.join(sdobj.mountpoint, sdobj.sdUUID, 'images',
-                               imgUUID, volUUID)
-        return int(sdobj.oop.os.stat(volPath).st_size / bs)
-
-    @classmethod
-    def getVTrueSize(cls, sdobj, imgUUID, volUUID, bs=BLOCK_SIZE):
-        volPath = os.path.join(sdobj.mountpoint, sdobj.sdUUID, 'images',
-                               imgUUID, volUUID)
-        return int(int(sdobj.oop.os.stat(volPath).st_blocks) * BLOCK_SIZE / bs)
-
-    @classmethod
     def renameVolumeRollback(cls, taskObj, oldPath, newPath):
         try:
             cls.log.info("oldPath=%s newPath=%s", oldPath, newPath)
@@ -538,30 +526,46 @@
         if not self.oop.fileUtils.pathExists(metaVolumePath):
             raise se.VolumeDoesNotExist(self.volUUID)
 
+    @classmethod
+    def getSizes(cls, volPath, bs=BLOCK_SIZE):
+        st = cls.oop.os.stat(volPath)
+        apparentSize = int(int(st.st_size) / bs)
+        trueSize = int(int(st.st_blocks) * BLOCK_SIZE / bs)
+        return (apparentSize, trueSize)
+
+    @classmethod
+    def getVSize(cls, sdobj, imgUUID, volUUID, bs=BLOCK_SIZE):
+        volPath = os.path.join(sdobj.mountpoint, sdobj.sdUUID, 'images',
+                               imgUUID, volUUID)
+        return int(sdobj.oop.os.stat(volPath).st_size / bs)
+
+    @classmethod
+    def getVTrueSize(cls, sdobj, imgUUID, volUUID, bs=BLOCK_SIZE):
+        volPath = os.path.join(sdobj.mountpoint, sdobj.sdUUID, 'images',
+                               imgUUID, volUUID)
+        return int(int(sdobj.oop.os.stat(volPath).st_blocks) * BLOCK_SIZE / bs)
+
     def getVolumeSize(self, bs=BLOCK_SIZE):
         """
         Return the volume size in blocks
         """
         volPath = self.getVolumePath()
-        return int(int(self.oop.os.stat(volPath).st_size) / bs)
+        return self.getSizes(volPath, bs)[0]
 
-    def getVolumeTrueSize(self, bs=BLOCK_SIZE):
+    def getVolumeSizes(self, bs=BLOCK_SIZE):
         """
         Return the size of the storage allocated for this volume
         on underlying storage
         """
         volPath = self.getVolumePath()
-        return int(int(self.oop.os.stat(volPath).st_blocks) * BLOCK_SIZE / bs)
+        return self.getSizes(volPath, bs)
 
-    def getVolumeMtime(self):
+    def getMtime(self):
         """
         Return the volume mtime in msec epoch
         """
         volPath = self.getVolumePath()
-        try:
-            return self.getMetaParam(volume.MTIME)
-        except se.MetaDataKeyNotFoundError:
-            return self.oop.os.stat(volPath).st_mtime
+        return self.oop.os.stat(volPath).st_mtime
 
     def _extendSizeRaw(self, newSize):
         volPath = self.getVolumePath()
diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py
index 10b3363..014f733 100644
--- a/vdsm/storage/volume.py
+++ b/vdsm/storage/volume.py
@@ -884,7 +884,6 @@
         """
         self.log.info("Info request: sdUUID=%s imgUUID=%s volUUID = %s ",
                       self.sdUUID, self.imgUUID, self.volUUID)
-        image_corrupted = False
         info = {}
         try:
             meta = self.getMetadata()
@@ -892,11 +891,14 @@
             info["capacity"] = str(int(info["size"]) * BLOCK_SIZE)
             del info["size"]
             # Get the image actual size on disk
-            vsize = self.getVolumeSize(bs=1)
-            avsize = self.getVolumeTrueSize(bs=1)
+            (vsize, avsize) = self.getVolumeSizes(bs=1)
             info['apparentsize'] = str(vsize)
             info['truesize'] = str(avsize)
-            info['mtime'] = self.getVolumeMtime()
+            if MTIME in info:
+                info['mtime'] = info[MTIME]
+            else:
+                info['mtime'] = self.getMtime()
+
             info['status'] = "OK"
         except se.StorageException as e:
             self.log.debug("exception: %s:%s" % (str(e.message),
str(e.value)))
@@ -905,11 +907,13 @@
             info['mtime'] = "0"
             info['status'] = "INVALID"
 
-        info['children'] = self.getChildrenList()
+        # Both engine and dumpStorageTable don't use this option so
+        # only keeping it to not break existing scripts that look for the key
+        info['children'] = []
 
         # If image was set to illegal, mark the status same
         # (because of VDC constraints)
-        if info.get('legality', None) == ILLEGAL_VOL or image_corrupted:
+        if info.get('legality', None) == ILLEGAL_VOL:
             info['status'] = ILLEGAL_VOL
         self.log.info("%s/%s/%s info is %s",
                       self.sdUUID, self.imgUUID, self.volUUID, str(info))
@@ -991,8 +995,9 @@
         # to byte multiples everywhere to avoid conversion errors and change
         # only at the end
         volParams['size'] = self.getSize()
-        volParams['apparentsize'] = self.getVolumeSize(bs=bs)
-        volParams['truesize'] = self.getVolumeTrueSize(bs=bs)
+        (apparentSize, trueSize) = self.getVolumeSizes(bs=bs)
+        volParams['apparentsize'] = apparentSize
+        volParams['truesize'] = trueSize
         volParams['parent'] = self.getParent()
         volParams['descr'] = self.getDescription()
         volParams['legality'] = self.getLegality()


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I30639134f9200fb6bfcf605b5d8fbc483b51d6e7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;

Itamar Heim has abandoned this change.

Change subject: Remove redundant calls that slow down getVolumeInfo
......................................................................


Abandoned

abandoning per no reply. please restore if still relevant.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I30639134f9200fb6bfcf605b5d8fbc483b51d6e7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Remove redundant calls that slow down getVolumeInfo
......................................................................


Patch Set 1:

Build Successful 

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

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3110/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30639134f9200fb6bfcf605b5d8fbc483b51d6e7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

Itamar Heim has posted comments on this change.

Change subject: Remove redundant calls that slow down getVolumeInfo
......................................................................


Patch Set 1:

ping

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I30639134f9200fb6bfcf605b5d8fbc483b51d6e7
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com&gt;
Gerrit-Reviewer: Itamar Heim <iheim(a)redhat.com&gt;
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No