[blivet:master 2/3] Put size values in Size universe eagerly.

David Lehman dlehman at redhat.com
Fri Oct 31 19:57:40 UTC 2014


On 10/31/2014 10:31 AM, mulhern wrote:
> Make everything that should be a Size a Size as soon as it is created,
> for clarity, consistency, correctness.
>
> * Clarity --- it's a way for the programmer to state, yes, I believe this is
> a Size and I'm pretty confident about its units.
> * Consistency --- a lot of methods say they return a Size, but many return a
> Size or an int, depending on the path taken through the method. Better to
> make these methods actually return a Size always.
> * Correctness --- type correctness and logical correctness. The more we
> ensure that we have the right type, the less likely we are to get an
> AttributeError invoking convertTo() or humanReadable() on an int or a
> Decimal. When we use Size we are required to specify the units, at least
> implicitly, so we can better avoid intepreting a bare number as, for example,
> 1/1024th of its intended value.
>
> Other changes in this patch:
> * For the first argument of sum(), use a generator, not a list.
> * Use // for division when a whole number is required (instead of casting to
> an int after the division).

This is changing at least one documented int to a Decimal -- probably 
more. Have you verified that pyparted is content with Decimal for 
sectors and lengths? Any autopart installation using anaconda in a vm 
should be sufficient to establish this.

>
> This patch can not be guaranteed to be complete. These instance of values
> that should be Sizes() were found by grepping and by instrumenting Size
> to raise an error if it detected a computation with an incorrect type.
> There are probably some more lurking.
>
> Note: Currently blivet only has two types of units to compute with,
> numbers (which could represent anything), and Sizes() which represent
> numbers of bytes. This patch turns numbers into Sizes as much as possible.
> At the present time, Size is implemented as a subclass of decimal.Decimal.
> It overloads a few of the operations of Decimal, typically returning a Size
> result. Sometimes this is the correct thing to do, and sometimes not.
>
> For example, a number times a Size evaluates to a Size and a Size divided
> by a number is a Size, which are both correct results with correct units.

It depends on the order of the operands:

 >>> 5 * Size("1 MiB")
Decimal('5242880')
 >>> Size("1 MiB") * 5
Size('5 MiB')

>
> However, it is wrong for a Size divided by a Size to evaluate to a Size;
> as a ratio of two Sizes, the result should be a plain number.
> It is wrong for the product of two Sizes to yield a Size, and almost certainly
> a sign that there is a mistake, as most of our computations should not
> result in products of Sizes. A Size + a plain number is a Size, which is
> the typical way to let in errors where a plain number that represents MiBs
> is added to a Size() yielding an incorrect Size result.

The issues above seem fairly easy to remedy, aside from preventing 
someone adding a non-size representing something other than bytes to a 
Size. I can certainly live with the latter problem.

>
> Eventually this problem should be fixed. In the meantime, however, the changes
> I have made in this patch are intended to be compatible with a corrected
> implementation of Size. The most sigificant instance is that sum() is
> given a start value of Size(0), in case the list being summed over is empty.
> The alternative, to convert the result of the sum operation to a Size,
> is incompatible with a correct version of Size which would consider adding
> a Size to a plain number to be an error.

Overall I think this is a good change.

A couple more comments inline, below...

>
> Signed-off-by: mulhern <amulhern at redhat.com>
> ---
>   blivet/__init__.py                 |  2 +-
>   blivet/deviceaction.py             |  3 ++-
>   blivet/devicefactory.py            | 12 ++++++------
>   blivet/devicelibs/lvm.py           |  6 +++---
>   blivet/devicelibs/mdraid.py        |  3 ++-
>   blivet/devicelibs/raid.py          | 13 +++++++------
>   blivet/devices/btrfs.py            |  6 +++---
>   blivet/devices/disk.py             |  3 ++-
>   blivet/devices/lvm.py              | 14 +++++---------
>   blivet/devices/md.py               |  4 ++--
>   blivet/devices/nodev.py            |  3 ++-
>   blivet/devices/partition.py        | 12 ++++++------
>   blivet/devices/storage.py          |  4 ++--
>   blivet/formats/disklabel.py        |  7 +++----
>   blivet/formats/fs.py               | 16 +++++++---------
>   blivet/partitioning.py             | 12 ++++++------
>   tests/devicelibs_test/raid_test.py | 24 ++++++++++++------------
>   tests/partitioning_test.py         |  4 ++--
>   tests/size_test.py                 |  4 ++--
>   19 files changed, 75 insertions(+), 77 deletions(-)
>
> diff --git a/blivet/__init__.py b/blivet/__init__.py
> index ab44eff..cbc3f60 100644
> --- a/blivet/__init__.py
> +++ b/blivet/__init__.py
> @@ -1537,7 +1537,7 @@ class Blivet(object):
>       def fileSystemFreeSpace(self):
>           """ Combined free space in / and /usr as :class:`~.size.Size`. """
>           mountpoints = ["/", "/usr"]
> -        free = 0
> +        free = Size(0)
>           btrfs_volumes = []
>           for mountpoint in mountpoints:
>               device = self.mountpoints.get(mountpoint)
> diff --git a/blivet/deviceaction.py b/blivet/deviceaction.py
> index 07d2493..0f7f2b0 100644
> --- a/blivet/deviceaction.py
> +++ b/blivet/deviceaction.py
> @@ -33,6 +33,7 @@ from parted import partitionFlag, PARTITION_LBA
>   from .i18n import _, N_
>   from .callbacks import CreateFormatPreData, CreateFormatPostData
>   from .callbacks import ResizeFormatPreData, ResizeFormatPostData
> +from .size import Size
>
>   import logging
>   log = logging.getLogger("blivet")
> @@ -435,7 +436,7 @@ class ActionResizeDevice(DeviceAction):
>               self.dir = RESIZE_GROW
>           else:
>               self.dir = RESIZE_SHRINK
> -        if device.targetSize > 0:
> +        if device.targetSize > Size(0):
>               self.origsize = device.targetSize
>           else:
>               self.origsize = device.size
> diff --git a/blivet/devicefactory.py b/blivet/devicefactory.py
> index 8b0d163..c383b4d 100644
> --- a/blivet/devicefactory.py
> +++ b/blivet/devicefactory.py
> @@ -549,7 +549,7 @@ class DeviceFactory(object):
>       #
>       def _create_device(self):
>           """ Create the factory device. """
> -        if self.size == 0:
> +        if self.size == Size(0):
>               # A factory with a size of zero means you're adjusting a container
>               # after removing a device from it.
>               return
> @@ -573,7 +573,7 @@ class DeviceFactory(object):
>           # this gets us a size value that takes into account the actual size of
>           # the container
>           size = self._get_device_size()
> -        if size <= 0:
> +        if size <= Size(0):
>               raise DeviceFactoryError("not enough free space for new device")
>
>           parents = self._get_parent_devices()
> @@ -1511,11 +1511,11 @@ class LVMThinPFactory(LVMFactory):
>
>       def _create_pool(self):
>           """ Create a pool large enough to contain the new device. """
> -        if self.size == 0:
> +        if self.size == Size(0):
>               return
>
>           size = self._get_pool_size()
> -        if size == 0:
> +        if size == Size(0):
>               raise DeviceFactoryError("not enough free space for thin pool")
>
>           self.pool = self._get_new_pool(size=size, parents=[self.container])
> @@ -1632,11 +1632,11 @@ class BTRFSFactory(DeviceFactory):
>           if self.container_size == SIZE_POLICY_AUTO:
>               # automatic
>               if self.container and not self.device:
> -                if self.size != 0:
> +                if self.size != Size(0):
>                       # For new subvols the size is in addition to the volume's size.
>                       size += self.container.size
>                   else:
> -                    size += sum(s.req_size for s in self.container.subvolumes)
> +                    size += sum((s.req_size for s in self.container.subvolumes), Size(0))
>
>               size += self._get_device_space()
>           elif self.container_size == SIZE_POLICY_MAX:
> diff --git a/blivet/devicelibs/lvm.py b/blivet/devicelibs/lvm.py
> index 44ac34c..cf21902 100644
> --- a/blivet/devicelibs/lvm.py
> +++ b/blivet/devicelibs/lvm.py
> @@ -164,8 +164,8 @@ def get_pv_space(size, disks, pesize=LVM_PE_SIZE):
>       # TODO: handle striped and mirrored
>       # this is adding one extent for the lv's metadata
>       # pylint: disable=unused-argument
> -    if size == 0:
> -        return size
> +    if size == Size(0):
> +        return Size(0)
>
>       space = clampSize(size, pesize, roundup=True) + pesize
>       return space
> @@ -214,7 +214,7 @@ def is_valid_thin_pool_chunk_size(size, discard=False):
>       if discard:
>           return (math.log(size, 2) % 1.0 == 0)
>       else:
> -        return (size % LVM_THINP_MIN_CHUNK_SIZE == 0)
> +        return (size % LVM_THINP_MIN_CHUNK_SIZE == Size(0))
>
>   def strip_lvm_warnings(buf):
>       """ Strip out lvm warning lines
> diff --git a/blivet/devicelibs/mdraid.py b/blivet/devicelibs/mdraid.py
> index 7d031e2..11e06d7 100644
> --- a/blivet/devicelibs/mdraid.py
> +++ b/blivet/devicelibs/mdraid.py
> @@ -70,8 +70,9 @@ def get_raid_superblock_size(size, version=None):
>           # MDADM: operations, but limit this to 128Meg (0.1% of 10Gig)
>           # MDADM: which is plenty for efficient reshapes
>           # NOTE: In the mdadm code this is in 512b sectors. Converted to use MiB
> +        MIN_HEADROOM = Size("1 MiB")
>           headroom = int(Size("128 MiB"))
> -        while headroom << 10 > size and headroom > Size("1 MiB"):
> +        while Size(headroom << 10) > size and Size(headroom) > MIN_HEADROOM:
>               headroom >>= 1
>
>           headroom = Size(headroom)
> diff --git a/blivet/devicelibs/raid.py b/blivet/devicelibs/raid.py
> index b3f7c49..0f0f1b4 100644
> --- a/blivet/devicelibs/raid.py
> +++ b/blivet/devicelibs/raid.py
> @@ -26,6 +26,7 @@ import abc
>   from six import add_metaclass
>
>   from ..errors import RaidError
> +from ..size import Size
>
>   def div_up(a,b):
>       """Rounds up integer division.  For example, div_up(3, 2) is 2.
> @@ -188,7 +189,7 @@ class RAIDn(RAIDLevel):
>           """
>           if member_count < self.min_members:
>               raise RaidError("%s requires at least %d disks" % (self.name, self.min_members))
> -        if smallest_member_size < 0:
> +        if smallest_member_size < Size(0):
>               raise RaidError("size is a negative number")
>           return self._get_net_array_size(member_count, smallest_member_size)
>
> @@ -268,12 +269,12 @@ class RAIDn(RAIDLevel):
>              under construction.
>           """
>           if not member_sizes:
> -            return 0
> +            return Size(0)
>
>           if num_members is None:
>               num_members = len(member_sizes)
>
> -        if chunk_size is None or chunk_size == 0:
> +        if chunk_size is None or chunk_size == Size(0):
>               raise RaidError("chunk_size parameter value %s is not acceptable")
>
>           if superblock_size_func is None:
> @@ -588,7 +589,7 @@ class Container(RAIDLevel):
>           raise RaidError("get_recommended_stride is not defined for level container")
>       def get_size(self, member_sizes, num_members=None, chunk_size=None, superblock_size_func=None):
>           # pylint: disable=unused-argument
> -        return sum(member_sizes)
> +        return sum(member_sizes, Size(0))
>
>   Container = Container()
>   ALL_LEVELS.addRaidLevel(Container)
> @@ -623,12 +624,12 @@ class ErsatzRAID(RAIDLevel):
>       def get_size(self, member_sizes, num_members=None, chunk_size=None, superblock_size_func=None):
>           # pylint: disable=unused-argument
>           if not member_sizes:
> -            return 0
> +            return Size(0)
>
>           if superblock_size_func is None:
>               raise RaidError("superblock_size_func value of None is not acceptable")
>
> -        total_space = sum(member_sizes)
> +        total_space = sum(member_sizes, Size(0))
>           superblock_size = superblock_size_func(total_space)
>           return total_space - len(member_sizes) * superblock_size
>
> diff --git a/blivet/devices/btrfs.py b/blivet/devices/btrfs.py
> index e2931e3..b405a1e 100644
> --- a/blivet/devices/btrfs.py
> +++ b/blivet/devices/btrfs.py
> @@ -31,6 +31,7 @@ from ..flags import flags
>   from ..storage_log import log_method_call
>   from .. import udev
>   from ..formats import getFormat, DeviceFormat
> +from ..size import Size
>
>   import logging
>   log = logging.getLogger("blivet")
> @@ -73,8 +74,7 @@ class BTRFSDevice(StorageDevice):
>           self.setupParents(orig=True)
>
>       def _getSize(self):
> -        size = sum([d.size for d in self.parents])
> -        return size
> +        return sum((d.size for d in self.parents), Size(0))
>
>       def _setSize(self, newsize):
>           raise RuntimeError("cannot directly set size of btrfs volume")
> @@ -276,7 +276,7 @@ class BTRFSVolumeDevice(BTRFSDevice, ContainerDevice, RaidDevice):
>               self.format.mountopts = self.parents[0].format.mountopts
>
>       def _getSize(self):
> -        size = sum([d.size for d in self.parents])
> +        size = sum((d.size for d in self.parents), Size(0))
>           if self.dataLevel in (raid.RAID1, raid.RAID10):
>               size /= len(self.parents)
>
> diff --git a/blivet/devices/disk.py b/blivet/devices/disk.py
> index 3874714..f14638d 100644
> --- a/blivet/devices/disk.py
> +++ b/blivet/devices/disk.py
> @@ -27,6 +27,7 @@ from .. import util
>   from ..flags import flags
>   from ..storage_log import log_method_call
>   from .. import udev
> +from ..size import Size
>
>   from ..fcoe import fcoe
>
> @@ -103,7 +104,7 @@ class DiskDevice(StorageDevice):
>           # Some drivers (cpqarray <blegh>) make block device nodes for
>           # controllers with no disks attached and then report a 0 size,
>           # treat this as no media present
> -        return self.partedDevice.getLength(unit="B") != 0
> +        return Size(self.partedDevice.getLength(unit="B")) != Size(0)
>
>       @property
>       def description(self):
> diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py
> index 1f00dae..3dd77de 100644
> --- a/blivet/devices/lvm.py
> +++ b/blivet/devices/lvm.py
> @@ -315,7 +315,7 @@ class LVMVolumeGroupDevice(ContainerDevice):
>           reserved = Size(0)
>           if self.reserved_percent > 0:
>               reserved = self.reserved_percent * Decimal('0.01') * self.size
> -        elif self.reserved_space > 0:
> +        elif self.reserved_space > Size(0):
>               reserved = self.reserved_space
>
>           return self.align(reserved, roundup=True)
> @@ -326,11 +326,7 @@ class LVMVolumeGroupDevice(ContainerDevice):
>           # TODO: just ask lvm if isModified returns False
>
>           # sum up the sizes of the PVs and align to pesize
> -        size = 0
> -        for pv in self.pvs:
> -            size += max(0, self.align(pv.size - pv.format.peStart))
> -
> -        return size
> +        return sum((max(Size(0), self.align(pv.size - pv.format.peStart)) for pv in self.pvs), Size(0))
>
>       @property
>       def extents(self):
> @@ -352,7 +348,7 @@ class LVMVolumeGroupDevice(ContainerDevice):
>
>           # total the sizes of any LVs
>           log.debug("%s size is %s", self.name, self.size)
> -        used = sum(lv.vgSpaceUsed for lv in self.lvs)
> +        used = sum((lv.vgSpaceUsed for lv in self.lvs), Size(0))
>           if not self.exists and raid_disks:
>               # (only) we allocate (5 * num_disks) extra extents for LV metadata
>               # on RAID (see the devicefactory.LVMFactory._get_total_space method)
> @@ -1058,7 +1054,7 @@ class LVMThinPoolDevice(LVMLogicalVolumeDevice):
>
>       @property
>       def usedSpace(self):
> -        return sum(l.poolSpaceUsed for l in self.lvs)
> +        return sum((l.poolSpaceUsed for l in self.lvs), Size(0))
>
>       @property
>       def freeSpace(self):
> @@ -1118,7 +1114,7 @@ class LVMThinLogicalVolumeDevice(LVMLogicalVolumeDevice):
>
>       @property
>       def vgSpaceUsed(self):
> -        return 0    # the pool's size is already accounted for in the vg
> +        return Size(0)    # the pool's size is already accounted for in the vg
>
>       def _setSize(self, size):
>           if not isinstance(size, Size):
> diff --git a/blivet/devices/md.py b/blivet/devices/md.py
> index aabd628..dcd0c54 100644
> --- a/blivet/devices/md.py
> +++ b/blivet/devices/md.py
> @@ -182,7 +182,7 @@ class MDRaidArrayDevice(ContainerDevice, RaidDevice):
>               If the array has no redundancy, a bitmap is just pointless.
>           """
>           try:
> -            return self.level.has_redundancy() and self.size >= 1000 and  self.format.type != "swap"
> +            return self.level.has_redundancy() and self.size >= Size(1000) and  self.format.type != "swap"
>           except errors.RaidError:
>               # If has_redundancy() raises an exception then this device has
>               # a level for which the redundancy question is meaningless. In
> @@ -220,7 +220,7 @@ class MDRaidArrayDevice(ContainerDevice, RaidDevice):
>                       self.getSuperBlockSize)
>               except (errors.MDRaidError, errors.RaidError) as e:
>                   log.info("could not calculate size of device %s for raid level %s: %s", self.name, self.level, e)
> -                size = 0
> +                size = Size(0)
>               log.debug("non-existent RAID %s size == %s", self.level, size)
>           else:
>               size = Size(self.partedDevice.getLength(unit="B"))
> diff --git a/blivet/devices/nodev.py b/blivet/devices/nodev.py
> index a66636c..618332d 100644
> --- a/blivet/devices/nodev.py
> +++ b/blivet/devices/nodev.py
> @@ -24,6 +24,7 @@ from ..storage_log import log_method_call
>   import logging
>   log = logging.getLogger("blivet")
>
> +from ..size import Size
>   from .storage import StorageDevice
>
>   class NoDevice(StorageDevice):
> @@ -91,7 +92,7 @@ class TmpFSDevice(NoDevice):
>           elif self.format:
>               return self.format.size
>           else:
> -            return 0
> +            return Size(0)
>
>       @property
>       def fstabSpec(self):
> diff --git a/blivet/devices/partition.py b/blivet/devices/partition.py
> index 6ad107a..1868678 100644
> --- a/blivet/devices/partition.py
> +++ b/blivet/devices/partition.py
> @@ -133,7 +133,7 @@ class PartitionDevice(StorageDevice):
>           self._partType = None
>           self._partedPartition = None
>           self._origPath = None
> -        self._currentSize = 0
> +        self._currentSize = Size(0)
>
>           # FIXME: Validate size, but only if this is a new partition.
>           #        For existing partitions we will get the size from
> @@ -162,7 +162,7 @@ class PartitionDevice(StorageDevice):
>               #     for new partitions.
>               if not self._size:
>                   if start is not None and end is not None:
> -                    self._size = 0
> +                    self._size = Size(0)
>                   else:
>                       # default size for new partition requests
>                       self._size = self.defaultSize
> @@ -506,11 +506,11 @@ class PartitionDevice(StorageDevice):
>
>           start = self.partedPartition.geometry.start
>           part_len = self.partedPartition.geometry.end - start
> -        bs = self.partedPartition.geometry.device.sectorSize
> +        bs = Size(self.partedPartition.geometry.device.sectorSize)
>           device = self.partedPartition.geometry.device.path
>
>           # Erase 1MiB or to end of partition
> -        count = int(Size("1 MiB") / bs)
> +        count = Size("1 MiB") // bs
>           count = min(count, part_len)
>
>           cmd = ["dd", "if=/dev/zero", "of=%s" % device, "bs=%s" % bs,
> @@ -577,7 +577,7 @@ class PartitionDevice(StorageDevice):
>           # compute new size for partition
>           currentGeom = partition.geometry
>           currentDev = currentGeom.device
> -        newLen = int(self.targetSize) / currentDev.sectorSize
> +        newLen = self.targetSize // Size(currentDev.sectorSize)
>           newGeometry = parted.Geometry(device=currentDev,
>                                         start=currentGeom.start,
>                                         length=newLen)
> @@ -738,7 +738,7 @@ class PartitionDevice(StorageDevice):
>           if self.exists:
>               return self._currentSize
>           else:
> -            return 0
> +            return Size(0)
>
>       @property
>       def resizable(self):
> diff --git a/blivet/devices/storage.py b/blivet/devices/storage.py
> index e1ee390..f6542c2 100644
> --- a/blivet/devices/storage.py
> +++ b/blivet/devices/storage.py
> @@ -539,7 +539,7 @@ class StorageDevice(Device):
>       def _getSize(self):
>           """ Get the device's size, accounting for pending changes. """
>           if self.exists and not self.mediaPresent:
> -            return 0
> +            return Size(0)
>
>           if self.exists and self.partedDevice:
>               self._size = self.currentSize
> @@ -572,7 +572,7 @@ class StorageDevice(Device):
>
>               If the device does not exist, then the actual size is 0.
>           """
> -        size = 0
> +        size = Size(0)
>           if self.exists and self.partedDevice:
>               size = Size(self.partedDevice.getLength(unit="B"))
>           elif self.exists:
> diff --git a/blivet/formats/disklabel.py b/blivet/formats/disklabel.py
> index d4d7ae7..afb8679 100644
> --- a/blivet/formats/disklabel.py
> +++ b/blivet/formats/disklabel.py
> @@ -204,7 +204,7 @@ class DiskLabel(DeviceFormat):
>       @property
>       def sectorSize(self):
>           try:
> -            return self.partedDevice.sectorSize
> +            return Size(self.partedDevice.sectorSize)
>           except AttributeError:
>               log_exception_info()
>               return None
> @@ -221,7 +221,7 @@ class DiskLabel(DeviceFormat):
>                   size = Size(self.partedDevice.getLength(unit="B"))
>               except Exception: # pylint: disable=broad-except
>                   log_exception_info()
> -                size = 0
> +                size = Size(0)
>
>           return size
>
> @@ -395,8 +395,7 @@ class DiskLabel(DeviceFormat):
>
>       @property
>       def free(self):
> -        return Size(sum(f.getLength(unit="B")
> -                        for f in self.partedDisk.getFreeSpacePartitions()))
> +        return sum((Size(f.getLength(unit="B")) for f in self.partedDisk.getFreeSpacePartitions()), Size(0))
>
>       @property
>       def magicPartitionNumber(self):
> diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py
> index 1bd3b93..a188c18 100644
> --- a/blivet/formats/fs.py
> +++ b/blivet/formats/fs.py
> @@ -310,7 +310,7 @@ class FS(DeviceFormat):
>                           break
>
>                   if len(values) != len(self._existingSizeFields):
> -                    return 0
> +                    return Size(0)
>
>                   size = 1
>                   for value in values:
> @@ -332,11 +332,10 @@ class FS(DeviceFormat):
>
>       @property
>       def free(self):
> -        free = 0
> +        free = Size(0)
>           if self.exists:
> -            if self.currentSize and self.minSize and \
> -               self.currentSize != self.minSize:
> -                free = int(max(0, self.currentSize - self.minSize)) # truncate
> +            if self.currentSize is not None and self.minSize is not None:
> +                free = max(Size(0), self.currentSize - self.minSize)
>
>           return free
>
> @@ -1305,7 +1304,7 @@ class MacEFIFS(HFSPlus):
>       _name = N_("Linux HFS+ ESP")
>       _labelfs = fslabeling.HFSPlusLabeling()
>       _udevTypes = []
> -    _minSize = 50
> +    _minSize = Size("50 MiB")
>
>       @property
>       def supported(self):
> @@ -1505,9 +1504,9 @@ class TmpFS(NoDevFS):
>       # it is Linux-native
>       _linuxNative = True
>       # empty tmpfs has zero overhead
> -    _minInstanceSize = 0
> +    _minInstanceSize = Size(0)
>       # tmpfs really does not occupy any space by itself
> -    _minSize = 0
> +    _minSize = Size(0)
>       # in a sense, I guess tmpfs is formattable
>       # in the regard that the format is automatically created
>       # once mounted
> @@ -1591,7 +1590,6 @@ class TmpFS(NoDevFS):
>
>       @property
>       def free(self):
> -        free_space = 0
>           if self._mountpoint:
>               # If self._mountpoint is defined, it means this tmpfs mount
>               # has been mounted and there is a path we can use as a handle to
> diff --git a/blivet/partitioning.py b/blivet/partitioning.py
> index 7107fd3..6ddc222 100644
> --- a/blivet/partitioning.py
> +++ b/blivet/partitioning.py
> @@ -666,7 +666,7 @@ def sizeToSectors(size, sectorSize):
>           :returns: sector count
>           :rtype: int
>       """
> -    sectors = int(size / sectorSize)
> +    sectors = size // sectorSize
>       return sectors

This now returns a Decimal while the docstring says it returns an int.

>
>   def removeNewPartitions(disks, remove, all_partitions):
> @@ -1315,12 +1315,12 @@ class LVRequest(Request):
>
>           # Round up to nearest pe. For growable requests this will mean that
>           # first growth is to fill the remainder of any unused extent.
> -        self.base = int(lv.vg.align(lv.req_size, roundup=True) / lv.vg.peSize)
> +        self.base = lv.vg.align(lv.req_size, roundup=True) // lv.vg.peSize
>
>           if lv.req_grow:
> -            limits = [int(l / lv.vg.peSize) for l in
> -                        [lv.vg.align(lv.req_max_size),
> -                         lv.vg.align(lv.format.maxSize)] if l > 0]
> +            limits = [l // lv.vg.peSize for l in
> +                        (lv.vg.align(lv.req_max_size),
> +                         lv.vg.align(lv.format.maxSize)) if l > Size(0)]
>
>               if limits:
>                   max_units = min(limits)
> @@ -1821,7 +1821,7 @@ class TotalSizeSet(object):
>
>           self.requests = []
>
> -        self.allocated = sum([d.req_base_size for d in self.devices])
> +        self.allocated = sum((d.req_base_size for d in self.devices), Size(0))
>           log.debug("set.allocated = %d", self.allocated)
>
>       def allocate(self, amount):
> diff --git a/tests/devicelibs_test/raid_test.py b/tests/devicelibs_test/raid_test.py
> index e7b2f60..ba1a580 100644
> --- a/tests/devicelibs_test/raid_test.py
> +++ b/tests/devicelibs_test/raid_test.py
> @@ -86,13 +86,13 @@ class RaidTestCase(unittest.TestCase):
>           ##
>           ## get_net_array_size
>           ##
> -        self.assertEqual(raid.RAID0.get_net_array_size(4, 2), 8)
> -        self.assertEqual(raid.RAID1.get_net_array_size(4, 2), 2)
> -        self.assertEqual(raid.RAID4.get_net_array_size(4, 2), 6)
> -        self.assertEqual(raid.RAID5.get_net_array_size(4, 2), 6)
> -        self.assertEqual(raid.RAID6.get_net_array_size(4, 2), 4)
> -        self.assertEqual(raid.RAID10.get_net_array_size(4, 2), 4)
> -        self.assertEqual(raid.RAID10.get_net_array_size(5, 2), 4)
> +        self.assertEqual(raid.RAID0.get_net_array_size(4, Size(2)), Size(8))
> +        self.assertEqual(raid.RAID1.get_net_array_size(4, Size(2)), Size(2))
> +        self.assertEqual(raid.RAID4.get_net_array_size(4, Size(2)), Size(6))
> +        self.assertEqual(raid.RAID5.get_net_array_size(4, Size(2)), Size(6))
> +        self.assertEqual(raid.RAID6.get_net_array_size(4, Size(2)), Size(4))
> +        self.assertEqual(raid.RAID10.get_net_array_size(4, Size(2)), Size(4))
> +        self.assertEqual(raid.RAID10.get_net_array_size(5, Size(2)), Size(4))
>
>           ##
>           ## get_recommended_stride
> @@ -115,24 +115,24 @@ class RaidTestCase(unittest.TestCase):
>           sizes = [Size("32MiB"), Size("128MiB"), Size("128MiB"), Size("64MiB")]
>           for r in (l for l in raid.ALL_LEVELS if l not in (raid.Container, raid.Dup)):
>               self.assertEqual(r.get_size(sizes, 4, Size("1MiB"), lambda x: Size(0)),
> -               r.get_net_array_size(4, Size("32MiB")) if isinstance(r, raid.RAIDn) else sum(sizes))
> +               r.get_net_array_size(4, Size("32MiB")) if isinstance(r, raid.RAIDn) else sum(sizes, Size(0)))
>
>           for r in (l for l in raid.ALL_LEVELS if l not in (raid.Container, raid.Dup)):
>               self.assertEqual(r.get_size(sizes, 5, Size("1MiB"), lambda x: Size(0)),
> -               r.get_net_array_size(5, Size("32MiB")) if isinstance(r, raid.RAIDn) else sum(sizes))
> +               r.get_net_array_size(5, Size("32MiB")) if isinstance(r, raid.RAIDn) else sum(sizes, Size(0)))
>
>           for r in (l for l in raid.ALL_LEVELS if l not in (raid.Container, raid.Dup)):
>               self.assertEqual(r.get_size(sizes, 4, Size("1MiB"), lambda x: Size("32MiB")),
> -               0 if isinstance(r, raid.RAIDn) else (sum(sizes) - 4 * Size("32MiB")))
> +               Size(0) if isinstance(r, raid.RAIDn) else (sum(sizes, Size(0)) - 4 * Size("32MiB")))
>
>           for r in (l for l in raid.ALL_LEVELS if l not in (raid.Container, raid.Dup)):
>               if isinstance(r, raid.RAIDn):
>                   if r not in (raid.RAID1, raid.RAID10):
> -                    self.assertEqual(r.get_size(sizes, 4, Size("2MiB"), lambda x: Size("31MiB")), 0)
> +                    self.assertEqual(r.get_size(sizes, 4, Size("2MiB"), lambda x: Size("31MiB")), Size(0))
>                   else:
>                       self.assertEqual(r.get_size(sizes, 4, Size("2MiB"), lambda x: Size("31MiB")), r.get_net_array_size(4, Size("1MiB")))
>               else:
> -                self.assertEqual(r.get_size(sizes, 4, Size("2MiB"), lambda x: Size("31MiB")), sum(sizes) - 4 * Size("31MiB"))
> +                self.assertEqual(r.get_size(sizes, 4, Size("2MiB"), lambda x: Size("31MiB")), sum(sizes, Size(0)) - 4 * Size("31MiB"))
>
>           ##
>           ## names
> diff --git a/tests/partitioning_test.py b/tests/partitioning_test.py
> index 41ca1a0..87c994f 100644
> --- a/tests/partitioning_test.py
> +++ b/tests/partitioning_test.py
> @@ -161,7 +161,7 @@ class PartitioningTestCase(unittest.TestCase):
>               sector_size = geom.device.sectorSize
>               self.assertEqual(alignment.isAligned(free, geom.start), True)
>               self.assertEqual(alignment.isAligned(free, geom.end + 1), True)
> -            self.assertEqual(part.geometry.length, Size("10 MiB") / sector_size)
> +            self.assertEqual(Size(part.geometry.length), Size("10 MiB") / sector_size)

length is an integer. I think you want to cast the second argument to 
int instead of making length a size.

>
>               disk.format.removePartition(part)
>               self.assertEqual(len(disk.format.partitions), 0)
> @@ -456,7 +456,7 @@ class PartitioningTestCase(unittest.TestCase):
>
>           self.assertEqual(chunk.length, vg.extents)
>           self.assertEqual(chunk.pool, vg.freeExtents)
> -        base_size = vg.align(sum(lv.size for lv in vg.lvs), roundup=True)
> +        base_size = vg.align(sum((lv.size for lv in vg.lvs), Size(0)), roundup=True)
>           base = base_size / vg.peSize
>           self.assertEqual(chunk.base, base)
>
> diff --git a/tests/size_test.py b/tests/size_test.py
> index ead34d6..d6731cc 100644
> --- a/tests/size_test.py
> +++ b/tests/size_test.py
> @@ -41,7 +41,7 @@ class SizeTestCase(unittest.TestCase):
>
>       def testExceptions(self):
>           zero = Size(0)
> -        self.assertEqual(zero, 0.0)
> +        self.assertEqual(zero, Size(0.0))
>
>           s = Size(500)
>           with self.assertRaises(SizePlacesError):
> @@ -53,7 +53,7 @@ class SizeTestCase(unittest.TestCase):
>           c = numbytes * factor
>
>           s = Size(c)
> -        self.assertEquals(s, c)
> +        self.assertEquals(s, Size(c))
>
>           if prefix:
>               u = "%sbytes" % prefix
>



More information about the anaconda-patches mailing list