Change in vdsm[master]: Reduce the number of getVolumeSize() calls.

Eduardo has uploaded a new change for review.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Reduce the number of getVolumeSize() calls.

Required for reduce the size functions proliferation.

Change-Id: Ic82fab1966bc6606e3c29483bea62dd17b4c56bc
Signed-off-by: Eduardo 
---
M vdsm/storage/blockVolume.py
M vdsm/storage/fileVolume.py
M vdsm/storage/sp.py
M vdsm/storage/volume.py
4 files changed, 8 insertions(+), 8 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/03/18203/1

diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py
index 3ccb9d5..aac18f5 100644
--- a/vdsm/storage/blockVolume.py
+++ b/vdsm/storage/blockVolume.py
@@ -189,7 +189,7 @@

return (dom.sdUUID, mdSlot)

-    def delete(self, postZero, force):
+    def delete(self, zeroLen, force):
         """ Delete volume
             'postZero' - zeroing file before deletion
             'force' is required to remove shared and internal volumes
@@ -198,7 +198,6 @@
                       self.volUUID, self.imgUUID, self.sdUUID)

vol_path = self.getVolumePath()
-        size = self.getVolumeSize(bs=1)
         offs = self.getMetaOffset()

if not force:
@@ -207,12 +206,12 @@
         # Mark volume as illegal before deleting
         self.setLegality(volume.ILLEGAL_VOL)

-        if postZero:
+        if zeroLen:
             self.prepare(justme=True, rw=True, chainrw=force, setrw=True,
                          force=True)
             try:
                 misc.ddWatchCopy(
-                    "/dev/zero", vol_path, vars.task.aborting, int(size),
+                    "/dev/zero", vol_path, vars.task.aborting, int(zeroLen),
                     recoveryCallback=volume.baseAsyncTasksRollback)
             except utils.ActionStopped:
                 raise
diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py
index 0217859..f35ca19 100644
--- a/vdsm/storage/fileVolume.py
+++ b/vdsm/storage/fileVolume.py
@@ -157,7 +157,7 @@

return (volPath,)

-    def delete(self, postZero, force):
+    def delete(self, zeroLen, force):
         """
         Delete volume.
             'postZero' - zeroing file before deletion
diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py
index 6694fe0..445c70d 100644
--- a/vdsm/storage/sp.py
+++ b/vdsm/storage/sp.py
@@ -2009,8 +2009,9 @@
                                       rm.LockType.exclusive):
             dom = sdCache.produce(sdUUID)
             for volUUID in volumes:
-                dom.produceVolume(imgUUID, volUUID).delete(
-                    postZero=postZero, force=force)
+                zeros = dom.getVSize(imgUUID, volUUID, bs=1) if postZero else 0
+                dom.produceVolume(imgUUID, volUUID).delete(zeroLen=zeros,
+                                                           force=force)

def setMaxHostID(self, spUUID, maxID):
         """
diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py
index a49c4de..5c50027 100644
--- a/vdsm/storage/volume.py
+++ b/vdsm/storage/volume.py
@@ -366,7 +366,7 @@
             raise se.createVolumeRollbackError(volUUID)
         pvol = vol.getParentVolume()
         # Remove volume
-        vol.delete(postZero=False, force=True)
+        vol.delete(zeroLen=0, force=True)
         if len(cls.getImageVolumes(repoPath, sdUUID, imgUUID)):
             # Don't remove the image folder itself
             return

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 1: Verified-1

Build Failed

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

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3076/ : FAILURE

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 2: Verified-1

Build Failed

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

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3095/ : FAILURE

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 3: Verified-1

Build Failed

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

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3098/ : FAILURE

Yeela Kaplan has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 3: Code-Review-1

(1 comment)

....................................................
File vdsm/storage/blockVolume.py
Line 188:                          dom.sdUUID, volUUID, exc_info=True)
Line 189: 
Line 190:         return (dom.sdUUID, mdSlot)
Line 191: 
Line 192:     def delete(self, zeroLen, force):
The idea is great!

But I think there's no reason to change the argument purpose and behavior.

postZero is a much better argument and more readable than zeroLen.

I would just move the getVolumeSize call into the 'if postZero' block.

Which would leave us with the better argument 'postZero' and also would save us redundant changes in the other classes.
Line 193:         """ Delete volume
Line 194:             'zeroLen' - zeroes to write before file deletion.
Line 195:             'force' is required to remove shared and internal volumes
Line 196:         """

Eduardo has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 3:

(1 comment)

....................................................
File vdsm/storage/blockVolume.py
Line 188:                          dom.sdUUID, volUUID, exc_info=True)
Line 189: 
Line 190:         return (dom.sdUUID, mdSlot)
Line 191: 
Line 192:     def delete(self, zeroLen, force):
The issue here is (by now) delete is a volume method but geting the volume size should be (and getVSize is) a SD method.

The Volume.getVolumeSize is wrong since there is nothing in the Volume object related or capable to get the size of the backing volume.

This whole module, including delete, will be removed solving the present dicotomy.
Line 193:         """ Delete volume
Line 194:             'zeroLen' - zeroes to write before file deletion.
Line 195:             'force' is required to remove shared and internal volumes
Line 196:         """

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 4: Verified-1

Build Failed

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3307/ : FAILURE

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 5: Verified-1

Build Failed

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3558/ : FAILURE

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 6: Verified-1

Build Failed

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4127/ : FAILURE

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 7: Verified-1

Build Failed

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4133/ : FAILURE

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 8: Verified-1

Build Failed

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4197/ : FAILURE

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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 9: Verified-1

Build Failed

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

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4207/ : FAILURE

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

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Patch Set 9: Code-Review-1

(2 comments)

....................................................
File vdsm/storage/blockVolume.py
Line 204:             self.prepare(justme=True, rw=True, chainrw=force, setrw=True,
Line 205:                          force=True)
Line 206:             try:
Line 207:                 misc.ddWatchCopy(
Line 208:                     "/dev/zero", vol_path, vars.task.aborting, int(zeroLen),
int() is now redundant and misleading.
Line 209:                     recoveryCallback=volume.baseAsyncTasksRollback)
Line 210:             except utils.ActionStopped:
Line 211:                 raise
Line 212:             except Exception:

....................................................
File vdsm/storage/hsm.py
Line 695: 
Line 696:         if volFormat != volume.COW_FORMAT:
Line 697:             # This method is used only with COW volumes (see docstring),
Line 698:             # for RAW volumes we just return the volume size.
Line 699:             return dict(size=str(domain.getVSize(imgUUID, volUUID)))
needs a rebase - this bug has been fixed by http://gerrit.ovirt.org/19279
Line 700: 
Line 701:         qemuImgFormat = volume.fmt2str(volume.COW_FORMAT)
Line 702: 
Line 703:         volToExtend.prepare()

Itamar Heim has abandoned this change.

Change subject: Reduce the number of getVolumeSize() calls.
......................................................................

Abandoned

old. not touched for a while. please restore if relevant