Change in vdsm[master]: virt: Unify Drive size units

nsoffer at redhat.com nsoffer at redhat.com
Wed Feb 11 17:23:40 UTC 2015


Nir Soffer has uploaded a new change for review.

Change subject: virt: Unify Drive size units
......................................................................

virt: Unify Drive size units

Previously we used to keep some Drive size values in bytes and some in
megabytes, and return next volume size in megabytes. We had lot of code
converting sizes from megabytes to bytes. This mess makes understanding
the code harder and leads to bugs as seen the previous patch.

Now we keep all size values in bytes and return next volume size in
bytes, simplifying lot of code.

Change-Id: Ice01c9706fc27d4472c90e47bf7e808564ba0bfe
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
M tests/vmStorageTests.py
M vdsm/virt/vm.py
M vdsm/virt/vmdevices/storage.py
3 files changed, 28 insertions(+), 25 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/27/37727/1

diff --git a/tests/vmStorageTests.py b/tests/vmStorageTests.py
index d661401..f5ca892 100644
--- a/tests/vmStorageTests.py
+++ b/tests/vmStorageTests.py
@@ -247,13 +247,13 @@
 
     def test_first_extend(self):
         drive = self.chunked_drive(1024, 8192)
-        apparentsize_mb = drive.apparentsize / constants.MEGAB
-        size_mb = apparentsize_mb + drive.volExtensionChunk
-        self.assertEqual(drive.getNextVolumeSize(drive.apparentsize), size_mb)
+        self.assertEqual(drive.getNextVolumeSize(drive.apparentsize),
+                         drive.apparentsize + drive.volExtensionChunk)
 
     def test_truesize_limit(self):
         drive = self.chunked_drive(8191, 8192)
-        self.assertEqual(drive.getNextVolumeSize(drive.apparentsize), 8192)
+        self.assertEqual(drive.getNextVolumeSize(drive.apparentsize),
+                         drive.truesize)
 
     def chunked_drive(self, size_mb, maxsize_mb):
         conf = drive_config(
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 21a6af1..b44ad56 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -1428,7 +1428,7 @@
         # may result in the VM being paused) we can't use the regular
         # getNextVolumeSize call as it relies on a cached value of the
         # drive apparentsize.
-        nextPhysSize = physical + drive.VOLWM_CHUNK_MB * constants.MEGAB
+        nextPhysSize = physical + drive.VOLWM_CHUNK_MB
 
         # NOTE: the intent of this check is to prevent faulty images to
         # trick qemu in requesting extremely large extensions (BZ#998443).
@@ -1480,7 +1480,7 @@
 
         Must be called only when the drive or the replica are chunked.
         """
-        newSize = vmDrive.getNextVolumeSize(curSize)  # newSize is in megabytes
+        newSize = vmDrive.getNextVolumeSize(curSize)
 
         if getattr(vmDrive, 'diskReplicate', None):
             volInfo = {'poolID': vmDrive.diskReplicate['poolID'],
@@ -1491,7 +1491,7 @@
             self.log.debug("Requesting an extension for the volume "
                            "replication: %s", volInfo)
             self.cif.irs.sendExtendMsg(vmDrive.poolID, volInfo,
-                                       newSize * constants.MEGAB,
+                                       newSize,
                                        self.__afterReplicaExtension)
         else:
             self.__extendDriveVolume(vmDrive, volumeID, newSize)
@@ -1523,9 +1523,9 @@
 
         self.log.debug("Verifying extension for volume %s, requested size %s, "
                        "current size %s", volInfo['volumeID'],
-                       volInfo['newSize'] * constants.MEGAB, apparentSize)
+                       volInfo['newSize'], apparentSize)
 
-        if apparentSize < volInfo['newSize'] * constants.MEGAB:  # in bytes
+        if apparentSize < volInfo['newSize']:
             raise RuntimeError(
                 "Volume extension failed for %s (domainID: %s, volumeID: %s)" %
                 (volInfo['name'], volInfo['domainID'], volInfo['volumeID']))
@@ -1549,7 +1549,7 @@
         self.cif.irs.sendExtendMsg(
             vmDrive.poolID,
             volInfo,
-            newSize * constants.MEGAB,
+            newSize,
             self.__afterVolumeExtension)
 
     def __afterVolumeExtension(self, volInfo):
diff --git a/vdsm/virt/vmdevices/storage.py b/vdsm/virt/vmdevices/storage.py
index 800128d..d8a5dd8 100644
--- a/vdsm/virt/vmdevices/storage.py
+++ b/vdsm/virt/vmdevices/storage.py
@@ -47,7 +47,8 @@
                  'volumeChain', 'baseVolumeID', 'serial', 'reqsize', 'cache',
                  '_blockDev', 'extSharedState', 'drv', 'sgio', 'GUID',
                  'diskReplicate')
-    VOLWM_CHUNK_MB = config.getint('irs', 'volume_utilization_chunk_mb')
+    VOLWM_CHUNK_SIZE = (config.getint('irs', 'volume_utilization_chunk_mb') *
+                        constants.MEGAB)
     VOLWM_FREE_PCT = 100 - config.getint('irs', 'volume_utilization_percent')
     VOLWM_CHUNK_REPLICATE_MULT = 2  # Chunk multiplier during replication
 
@@ -113,35 +114,37 @@
     @property
     def volExtensionChunk(self):
         """
-        Returns the volume extension chunks (used for the thin provisioning
-        on block devices). The value is based on the vdsm configuration but
-        can also dynamically change according to the VM needs (e.g. increase
-        during a live storage migration).
+        Returns the volume extension chunk size in bytes.
+
+        This size is used for the thin provisioning on block devices. The value
+        is based on the vdsm configuration but can also dynamically change
+        according to the VM needs (e.g. increase during a live storage
+        migration).
         """
         if self.isDiskReplicationInProgress():
-            return self.VOLWM_CHUNK_MB * self.VOLWM_CHUNK_REPLICATE_MULT
-        return self.VOLWM_CHUNK_MB
+            return self.VOLWM_CHUNK_SIZE * self.VOLWM_CHUNK_REPLICATE_MULT
+        return self.VOLWM_CHUNK_SIZE
 
     @property
     def watermarkLimit(self):
         """
-        Returns the watermark limit, when the LV usage reaches this limit an
-        extension is in order (thin provisioning on block devices).
+        Returns the watermark limit in bytes.
+
+        When the LV usage reaches this limit an extension is in order (thin
+        provisioning on block devices).
         """
-        return (self.VOLWM_FREE_PCT * self.volExtensionChunk *
-                constants.MEGAB / 100)
+        return self.VOLWM_FREE_PCT * self.volExtensionChunk / 100
 
     def getNextVolumeSize(self, curSize):
         """
-        Returns the next volume size in megabytes. This value is based on the
+        Returns the next volume size in bytes. This value is based on the
         volExtensionChunk property and it's the size that should be requested
         for the next LV extension.  curSize is the current size of the volume
         to be extended.  For the leaf volume curSize == self.apparentsize.
         For internal volumes it is discovered by calling irs.getVolumeSize().
         """
-        curSizeMB = (curSize + constants.MEGAB - 1) / constants.MEGAB
-        nextSizeMB = curSizeMB + self.volExtensionChunk
-        return min(nextSizeMB, self.truesize / constants.MEGAB)
+        nextSize = curSize + self.volExtensionChunk
+        return min(nextSize, self.truesize)
 
     @property
     def chunked(self):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ice01c9706fc27d4472c90e47bf7e808564ba0bfe
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list