[blivet:master 14/20] Change convertTo() and roundToNearest() so each takes a units specifier.

mulhern amulhern at redhat.com
Tue Dec 23 23:42:27 UTC 2014


It's a lot nicer to pass them a named constant that they can use directly,
rather than a spec, which must be parsed as if it was entered by a user.

If there is ever a situation where the target of the conversion is user
specified, there is always _parseUnit() for converting a string to a unit.

Get rid of xlate parameter on convertTo(), it's pointless if the spec is
no longer being passed.

Change calling code appropriately.

This change requires a corresponding change in anaconda as convertTo() is
called there in two places.

The gains are clarity, simplicity, and efficiency.

Signed-off-by: mulhern <amulhern at redhat.com>
---
 blivet/devicelibs/lvm.py    | 20 +++++++-------
 blivet/devices/lvm.py       | 16 ++++++------
 blivet/devices/partition.py |  8 +++---
 blivet/formats/fs.py        | 20 +++++++-------
 blivet/size.py              | 14 ++++------
 tests/size_test.py          | 64 ++++++++++++++++++++++-----------------------
 tests/storagetestcase.py    |  4 ++-
 7 files changed, 71 insertions(+), 75 deletions(-)

diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py
index 3559dab..ce576e2 100644
--- a/blivet/devicelibs/lvm.py
+++ b/blivet/devicelibs/lvm.py
@@ -27,7 +27,7 @@ import logging
 log = logging.getLogger("blivet")
 
 from . import raid
-from ..size import Size
+from ..size import Size, KiB, MiB
 from .. import util
 from .. import arch
 from ..errors import LVMError
@@ -245,7 +245,7 @@ def pvcreate(device):
 
 def pvresize(device, size):
     args = ["pvresize",
-            "--setphysicalvolumesize", ("%dm" % size.convertTo(spec="mib")),
+            "--setphysicalvolumesize", ("%dm" % size.convertTo(MiB)),
             device]
 
     try:
@@ -340,7 +340,7 @@ def pvinfo(device=None):
 def vgcreate(vg_name, pv_list, pe_size):
     argv = ["vgcreate"]
     if pe_size:
-        argv.extend(["-s", "%dm" % pe_size.convertTo(spec="mib")])
+        argv.extend(["-s", "%dm" % pe_size.convertTo(MiB)])
     argv.append(vg_name)
     argv.extend(pv_list)
 
@@ -478,7 +478,7 @@ def lvcreate(vg_name, lv_name, size, pvs=None):
     pvs = pvs or []
 
     args = ["lvcreate"] + \
-            ["-L", "%dm" % size.convertTo(spec="mib")] + \
+            ["-L", "%dm" % size.convertTo(MiB)] + \
             ["-n", lv_name] + \
             ["-y"] + \
             [vg_name] + pvs
@@ -502,7 +502,7 @@ def lvremove(vg_name, lv_name, force=False):
 
 def lvresize(vg_name, lv_name, size):
     args = ["lvresize"] + \
-            ["--force", "-L", "%dm" % size.convertTo(spec="mib")] + \
+            ["--force", "-L", "%dm" % size.convertTo(MiB)] + \
             ["%s/%s" % (vg_name, lv_name)]
 
     try:
@@ -555,7 +555,7 @@ def lvsnapshotcreate(vg_name, snap_name, size, origin_name):
         :param :class:`~.size.Size` size: the snapshot's size
         :param str origin_name: the name of the origin logical volume
     """
-    args = ["lvcreate", "-s", "-L", "%dm" % size.convertTo(spec="MiB"),
+    args = ["lvcreate", "-s", "-L", "%dm" % size.convertTo(MiB),
             "-n", snap_name, "%s/%s" % (vg_name, origin_name)]
 
     try:
@@ -565,15 +565,15 @@ def lvsnapshotcreate(vg_name, snap_name, size, origin_name):
 
 def thinpoolcreate(vg_name, lv_name, size, metadatasize=None, chunksize=None, profile=None):
     args = ["lvcreate", "--thinpool", "%s/%s" % (vg_name, lv_name),
-            "--size", "%dm" % size.convertTo(spec="mib")]
+            "--size", "%dm" % size.convertTo(MiB)]
 
     if metadatasize:
         # default unit is MiB
-        args += ["--poolmetadatasize", "%d" % metadatasize.convertTo(spec="mib")]
+        args += ["--poolmetadatasize", "%d" % metadatasize.convertTo(MiB)]
 
     if chunksize:
         # default unit is KiB
-        args += ["--chunksize", "%d" % chunksize.convertTo(spec="kib")]
+        args += ["--chunksize", "%d" % chunksize.convertTo(KiB)]
 
     if profile:
         args += ["--profile=%s" % profile]
@@ -585,7 +585,7 @@ def thinpoolcreate(vg_name, lv_name, size, metadatasize=None, chunksize=None, pr
 
 def thinlvcreate(vg_name, pool_name, lv_name, size):
     args = ["lvcreate", "--thinpool", "%s/%s" % (vg_name, pool_name),
-            "--virtualsize", "%dm" % size.convertTo(spec="MiB"),
+            "--virtualsize", "%dm" % size.convertTo(MiB),
             "-n", lv_name]
 
     try:
diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py
index 3dd77de..d1817dd 100644
--- a/blivet/devices/lvm.py
+++ b/blivet/devices/lvm.py
@@ -34,7 +34,7 @@ from .. import util
 from ..formats import getFormat
 from ..storage_log import log_method_call
 from .. import udev
-from ..size import Size
+from ..size import Size, KiB, MiB
 
 import logging
 log = logging.getLogger("blivet")
@@ -411,7 +411,7 @@ class LVMVolumeGroupDevice(ContainerDevice):
         data.physvols = ["pv.%d" % p.id for p in self.parents]
         data.preexist = self.exists
         if not self.exists:
-            data.pesize = self.peSize.convertTo(spec="KiB")
+            data.pesize = self.peSize.convertTo(KiB)
 
         # reserved percent/space
 
@@ -752,14 +752,14 @@ class LVMLogicalVolumeDevice(DMDevice):
         if not self.exists:
             data.grow = self.req_grow
             if self.req_grow:
-                data.size = self.req_size.convertTo(spec="MiB")
-                data.maxSizeMB = self.req_max_size.convertTo(spec="MiB")
+                data.size = self.req_size.convertTo(MiB)
+                data.maxSizeMB = self.req_max_size.convertTo(MiB)
             else:
-                data.size = self.size.convertTo(spec="MiB")
+                data.size = self.size.convertTo(MiB)
 
             data.percent = self.req_percent
         elif data.resize:
-            data.size = self.targetSize.convertTo(spec="MiB")
+            data.size = self.targetSize.convertTo(MiB)
 
     @classmethod
     def isNameValid(cls, name):
@@ -1085,8 +1085,8 @@ class LVMThinPoolDevice(LVMLogicalVolumeDevice):
         super(LVMThinPoolDevice, self).populateKSData(data)
         data.mountpoint = "none"
         data.thin_pool = True
-        data.metadata_size = self.metaDataSize.convertTo(spec="MiB")
-        data.chunk_size = self.chunkSize.convertTo(spec="KiB")
+        data.metadata_size = self.metaDataSize.convertTo(MiB)
+        data.chunk_size = self.chunkSize.convertTo(KiB)
         if self.profile:
             data.profile = self.profile.name
 
diff --git a/blivet/devices/partition.py b/blivet/devices/partition.py
index 24f85b3..be42de5 100644
--- a/blivet/devices/partition.py
+++ b/blivet/devices/partition.py
@@ -30,7 +30,7 @@ from ..flags import flags
 from ..storage_log import log_method_call
 from .. import udev
 from ..formats import DeviceFormat, getFormat
-from ..size import Size
+from ..size import Size, MiB
 
 from ..devicelibs import dm
 
@@ -842,10 +842,10 @@ class PartitionDevice(StorageDevice):
         data.resize = (self.exists and self.targetSize and
                        self.targetSize != self.currentSize)
         if not self.exists:
-            data.size = self.req_base_size.convertTo(spec="MiB")
+            data.size = self.req_base_size.convertTo(MiB)
             data.grow = self.req_grow
             if self.req_grow:
-                data.maxSizeMB = self.req_max_size.convertTo(spec="MiB")
+                data.maxSizeMB = self.req_max_size.convertTo(MiB)
 
             ##data.disk = self.disk.name                      # by-id
             if self.req_disks and len(self.req_disks) == 1:
@@ -855,4 +855,4 @@ class PartitionDevice(StorageDevice):
             data.onPart = self.name                     # by-id
 
             if data.resize:
-                data.size = self.size.convertTo(spec="MiB")
+                data.size = self.size.convertTo(MiB)
diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py
index 55da105..e09d1e5 100644
--- a/blivet/formats/fs.py
+++ b/blivet/formats/fs.py
@@ -35,7 +35,7 @@ from ..flags import flags
 from parted import fileSystemType
 from ..storage_log import log_exception_info, log_method_call
 from .. import arch
-from ..size import Size, ROUND_UP, ROUND_DOWN
+from ..size import Size, ROUND_UP, ROUND_DOWN, MiB, B, unitStr
 from ..i18n import _, N_
 from .. import udev
 
@@ -431,7 +431,7 @@ class FS(DeviceFormat):
 
     @property
     def resizeArgs(self):
-        argv = [self.device, "%d" % (self.targetSize.convertTo("MiB"),)]
+        argv = [self.device, "%d" % (self.targetSize.convertTo(MiB),)]
         return argv
 
     def doResize(self):
@@ -495,7 +495,7 @@ class FS(DeviceFormat):
         # early.
         if self.targetSize > self.currentSize and rounded < self.currentSize:
             log.info("rounding target size down to next %s obviated resize of "
-                     "filesystem on %s", self._resizefsUnit, self.device)
+                     "filesystem on %s", unitStr(self._resizefsUnit), self.device)
             return
         else:
             self.targetSize = rounded
@@ -942,7 +942,7 @@ class Ext2FS(FS):
     _infofs = "dumpe2fs"
     _defaultInfoOptions = ["-h"]
     _existingSizeFields = ["Block count:", "Block size:"]
-    _resizefsUnit = "MiB"
+    _resizefsUnit = MiB
     _fsProfileSpecifier = "-T"
     partedSystem = fileSystemType["ext2"]
 
@@ -1041,7 +1041,7 @@ class Ext2FS(FS):
 
     @property
     def resizeArgs(self):
-        argv = ["-p", self.device, "%dM" % (self.targetSize.convertTo("MiB"))]
+        argv = ["-p", self.device, "%dM" % (self.targetSize.convertTo(MiB))]
         return argv
 
 register_device_format(Ext2FS)
@@ -1383,7 +1383,7 @@ class NTFS(FS):
     _infofs = "ntfsinfo"
     _defaultInfoOptions = ["-m"]
     _existingSizeFields = ["Cluster Size:", "Volume Size in Clusters:"]
-    _resizefsUnit = "B"
+    _resizefsUnit = B
     partedSystem = fileSystemType["ntfs"]
 
     def _fsckFailed(self, rc):
@@ -1440,7 +1440,7 @@ class NTFS(FS):
     def resizeArgs(self):
         # You must supply at least two '-f' options to ntfsresize or
         # the proceed question will be presented to you.
-        argv = ["-ff", "-s", "%d" % self.targetSize.convertTo(spec="b"),
+        argv = ["-ff", "-s", "%d" % self.targetSize.convertTo(B),
                 self.device]
         return argv
 
@@ -1587,7 +1587,7 @@ class TmpFS(NoDevFS):
             self._options = fsoptions
         if self._size:
             # absolute size for the tmpfs mount has been specified
-            self._size_option = "size=%dm" % self._size.convertTo(spec="MiB")
+            self._size_option = "size=%dm" % self._size.convertTo(MiB)
         else:
             # if no size option is specified, the tmpfs mount size will be 50%
             # of system RAM by default
@@ -1669,7 +1669,7 @@ class TmpFS(NoDevFS):
         # the mount command
 
         # add the remount flag, size and any options
-        remount_options = 'remount,size=%dm' % self.targetSize.convertTo(spec="MiB")
+        remount_options = 'remount,size=%dm' % self.targetSize.convertTo(MiB)
         # if any mount options are defined, append them
         if self._options:
             remount_options = "%s,%s" % (remount_options, self._options)
@@ -1689,7 +1689,7 @@ class TmpFS(NoDevFS):
             # update the size option string
             # -> please note that resizing always sets the
             # size of this tmpfs mount to an absolute value
-            self._size_option = "size=%dm" % self._size.convertTo(spec="MiB")
+            self._size_option = "size=%dm" % self._size.convertTo(MiB)
 
 register_device_format(TmpFS)
 
diff --git a/blivet/size.py b/blivet/size.py
index f37a873..2275b22 100644
--- a/blivet/size.py
+++ b/blivet/size.py
@@ -294,18 +294,14 @@ class Size(Decimal):
     def __mod__(self, other, context=None):
         return Size(Decimal.__mod__(self, other, context=context))
 
-    def convertTo(self, spec="", xlate=False):
+    def convertTo(self, spec=None):
         """ Return the size in the units indicated by the specifier.
 
-            :param str spec: a units specifier
+            :param spec: a units specifier
             :returns: a numeric value in the units indicated by the specifier
             :rtype: Decimal
         """
-        unit = _parseUnits(spec, xlate)
-        if unit:
-            return Decimal(self) / Decimal(unit.factor)
-
-        return None
+        return Decimal(self) / Decimal((spec or B).factor)
 
     def humanReadable(self, max_places=2, strip=True, min_value=1, xlate=True):
         """ Return a string representation of this size with appropriate
@@ -379,7 +375,7 @@ class Size(Decimal):
 
     def roundToNearest(self, unit, rounding=ROUND_DEFAULT):
         """
-            :param str unit: a unit specifier
+            :param unit: a unit specifier, a named constant like KiB
             :keyword rounding: which direction to round
             :type rounding: one of ROUND_UP, ROUND_DOWN, or ROUND_DEFAULT
             :returns: Size rounded to nearest whole specified unit
@@ -389,4 +385,4 @@ class Size(Decimal):
             raise ValueError("invalid rounding specifier")
 
         rounded = self.convertTo(unit).to_integral_value(rounding=rounding)
-        return Size("%s %s" % (rounded, unit))
+        return Size(rounded * unit.factor)
diff --git a/tests/size_test.py b/tests/size_test.py
index 1ec7fbd..a3ee311 100644
--- a/tests/size_test.py
+++ b/tests/size_test.py
@@ -44,34 +44,32 @@ class SizeTestCase(unittest.TestCase):
 
         self.assertEqual(s.humanReadable(max_places=0), "500 B")
 
-    def _prefixTestHelper(self, numunits, factor, prefix, abbr):
+    def _prefixTestHelper(self, numunits, unit):
         """ Test that units and prefix or abbreviation agree.
 
             :param int numunits: this value times factor yields number of bytes
-            :param int factor: this value times numunits yields number of bytes
-            :param str prefix: the prefix used to specify the units
-            :param str abbr: the abbreviation used to specify the units
+            :param unit: a unit specifier
         """
-        c = numunits * factor
+        c = numunits * unit.factor
 
         s = Size(c)
         self.assertEquals(s, Size(c))
 
-        u = size._makeSpec(prefix, size._BYTES_WORDS[0], False)
+        u = size._makeSpec(unit.prefix, size._BYTES_WORDS[0], False)
         s = Size("%ld %s" % (numunits, u))
         self.assertEquals(s, c)
-        self.assertEquals(s.convertTo(spec=u), numunits)
+        self.assertEquals(s.convertTo(unit), numunits)
 
-        u = size._makeSpec(abbr, size._BYTES_SYMBOL, False)
+        u = size._makeSpec(unit.abbr, size._BYTES_SYMBOL, False)
         s = Size("%ld %s" % (numunits, u))
         self.assertEquals(s, c)
-        self.assertEquals(s.convertTo(spec=u), numunits)
+        self.assertEquals(s.convertTo(unit), numunits)
 
     def testPrefixes(self):
         numbytes = 47
 
-        for factor, prefix, abbr in [_EMPTY_PREFIX] + _BINARY_PREFIXES + _DECIMAL_PREFIXES:
-            self._prefixTestHelper(numbytes, factor, prefix, abbr)
+        for unit in [_EMPTY_PREFIX] + _BINARY_PREFIXES + _DECIMAL_PREFIXES:
+            self._prefixTestHelper(numbytes, unit)
 
     def testHumanReadable(self):
         s = Size(58929971)
@@ -170,15 +168,15 @@ class SizeTestCase(unittest.TestCase):
 
     def testConvertToPrecision(self):
         s = Size(1835008)
-        self.assertEquals(s.convertTo(spec=""), 1835008)
-        self.assertEquals(s.convertTo(spec="b"), 1835008)
-        self.assertEquals(s.convertTo(spec="KiB"), 1792)
-        self.assertEquals(s.convertTo(spec="MiB"), 1.75)
+        self.assertEquals(s.convertTo(None), 1835008)
+        self.assertEquals(s.convertTo(size.B), 1835008)
+        self.assertEquals(s.convertTo(size.KiB), 1792)
+        self.assertEquals(s.convertTo(size.MiB), 1.75)
 
     def testNegative(self):
         s = Size("-500MiB")
         self.assertEquals(s.humanReadable(), "-500 MiB")
-        self.assertEquals(s.convertTo(spec="b"), -524288000)
+        self.assertEquals(s.convertTo(size.B), -524288000)
 
     def testPartialBytes(self):
         self.assertEquals(Size(1024.6), Size(1024))
@@ -297,40 +295,40 @@ class TranslationTestCase(unittest.TestCase):
         self.assertEqual(size.ROUND_DEFAULT, size.ROUND_HALF_UP)
 
         s = Size("10.3 GiB")
-        self.assertEqual(s.roundToNearest("GiB"), Size("10 GiB"))
-        self.assertEqual(s.roundToNearest("GiB", rounding=size.ROUND_DEFAULT),
+        self.assertEqual(s.roundToNearest(size.GiB), Size("10 GiB"))
+        self.assertEqual(s.roundToNearest(size.GiB, rounding=size.ROUND_DEFAULT),
                          Size("10 GiB"))
-        self.assertEqual(s.roundToNearest("GiB", rounding=size.ROUND_DOWN),
+        self.assertEqual(s.roundToNearest(size.GiB, rounding=size.ROUND_DOWN),
                          Size("10 GiB"))
-        self.assertEqual(s.roundToNearest("GiB", rounding=size.ROUND_UP),
+        self.assertEqual(s.roundToNearest(size.GiB, rounding=size.ROUND_UP),
                          Size("11 GiB"))
-        # >>> Size("10.3 GiB").convertTo("MiB")
+        # >>> Size("10.3 GiB").convertTo(size.MiB)
         # Decimal('10547.19999980926513671875')
-        self.assertEqual(s.roundToNearest("MiB"), Size("10547 MiB"))
-        self.assertEqual(s.roundToNearest("MiB", rounding=size.ROUND_UP),
+        self.assertEqual(s.roundToNearest(size.MiB), Size("10547 MiB"))
+        self.assertEqual(s.roundToNearest(size.MiB, rounding=size.ROUND_UP),
                          Size("10548 MiB"))
-        self.assertIsInstance(s.roundToNearest("MiB"), Size)
+        self.assertIsInstance(s.roundToNearest(size.MiB), Size)
         with self.assertRaises(ValueError):
-            s.roundToNearest("MiB", rounding='abc')
+            s.roundToNearest(size.MiB, rounding='abc')
 
         # arbitrary decimal rounding constants are not allowed
         from decimal import ROUND_HALF_DOWN
         with self.assertRaises(ValueError):
-            s.roundToNearest("MiB", rounding=ROUND_HALF_DOWN)
+            s.roundToNearest(size.MiB, rounding=ROUND_HALF_DOWN)
 
         s = Size("10.51 GiB")
-        self.assertEqual(s.roundToNearest("GiB"), Size("11 GiB"))
-        self.assertEqual(s.roundToNearest("GiB", rounding=size.ROUND_DEFAULT),
+        self.assertEqual(s.roundToNearest(size.GiB), Size("11 GiB"))
+        self.assertEqual(s.roundToNearest(size.GiB, rounding=size.ROUND_DEFAULT),
                          Size("11 GiB"))
-        self.assertEqual(s.roundToNearest("GiB", rounding=size.ROUND_DOWN),
+        self.assertEqual(s.roundToNearest(size.GiB, rounding=size.ROUND_DOWN),
                          Size("10 GiB"))
-        self.assertEqual(s.roundToNearest("GiB", rounding=size.ROUND_UP),
+        self.assertEqual(s.roundToNearest(size.GiB, rounding=size.ROUND_UP),
                          Size("11 GiB"))
 
         s = Size("513 GiB")
-        self.assertEqual(s.roundToNearest("GiB"), s)
-        self.assertEqual(s.roundToNearest("TiB"), Size("1 TiB"))
-        self.assertEqual(s.roundToNearest("TiB", rounding=size.ROUND_DOWN),
+        self.assertEqual(s.roundToNearest(size.GiB), s)
+        self.assertEqual(s.roundToNearest(size.TiB), Size("1 TiB"))
+        self.assertEqual(s.roundToNearest(size.TiB, rounding=size.ROUND_DOWN),
                          Size(0))
 
 class UtilityMethodsTestCase(unittest.TestCase):
diff --git a/tests/storagetestcase.py b/tests/storagetestcase.py
index 54d6c15..0b3f26d 100644
--- a/tests/storagetestcase.py
+++ b/tests/storagetestcase.py
@@ -12,6 +12,8 @@ from blivet.formats import getFormat
 from blivet.devices import StorageDevice
 from blivet.devices import PartitionDevice
 
+from blivet.size import B
+
 class StorageTestCase(unittest.TestCase):
     """ StorageTestCase
 
@@ -94,7 +96,7 @@ class StorageTestCase(unittest.TestCase):
         if exists:
             # set up mock parted.Device w/ correct size
             device._partedDevice = Mock()
-            device._partedDevice.getLength = Mock(return_value=int(device._size.convertTo(spec="B")))
+            device._partedDevice.getLength = Mock(return_value=int(device._size.convertTo(B)))
             device._partedDevice.sectorSize = 512
 
         if isinstance(device, blivet.devices.PartitionDevice):
-- 
1.9.3



More information about the anaconda-patches mailing list