[PATCH] Use blivet.size.Size for all size quantities.

David Lehman dlehman at redhat.com
Fri Jan 10 21:44:19 UTC 2014


On Fri, 2014-01-10 at 15:29 -0500, Anne Mulhern wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "David Lehman" <dlehman at redhat.com>
> > To: anaconda-patches at lists.fedorahosted.org
> > Sent: Friday, January 10, 2014 10:28:27 AM
> > Subject: [PATCH] Use blivet.size.Size for all size quantities.
> > 
> > In addition to resolving a host of long-standing issues with handling of
> > size units, this changes the default unit in the GUI changes from MB
> > (SI/decimal) to MiB (IEC/binary). The reason for this is that everything
> > is binary at the bottom of the stack. I do not care what disk
> > manufacturers write on their boxes. Disk sectors are normally 512 bytes
> > -- not 500 bytes. Sizes of disk partitions will rarely look reasonable
> > expressed as decimal-prefixed units since an even number in base-10 is
> > almost never evenly divisible by 512. Since we need to be able to detect
> > resize based on the contents of the size entry, regardless of units, we
> > cannot lose any precision in our display of sizes, which rules out lying
> > to users to make it seem like their decimal-prefixed sizes can be
> > attained. Sizes can still be expressed in decimal units, but we
> > will display binary units.
> > ---
> >  pyanaconda/bootloader.py                  | 19 +++++----
> >  pyanaconda/installclass.py                | 11 +++--
> >  pyanaconda/kickstart.py                   | 69
> >  ++++++++++++++++++++-----------
> >  pyanaconda/ui/gui/spokes/custom.glade     |  2 +-
> >  pyanaconda/ui/gui/spokes/custom.py        | 54 ++++++++++--------------
> >  pyanaconda/ui/gui/spokes/filter.py        |  6 +--
> >  pyanaconda/ui/gui/spokes/lib/accordion.py |  7 +---
> >  pyanaconda/ui/gui/spokes/lib/cart.py      |  5 +--
> >  pyanaconda/ui/gui/spokes/lib/resize.py    | 65 ++++++++++++++---------------
> >  pyanaconda/ui/gui/spokes/storage.py       | 34 ++++++---------
> >  pyanaconda/ui/lib/disks.py                | 10 +----
> >  pyanaconda/ui/tui/spokes/storage.py       |  5 +--
> >  12 files changed, 139 insertions(+), 148 deletions(-)
> > 
> > diff --git a/pyanaconda/bootloader.py b/pyanaconda/bootloader.py
> > index 737cb6b..ee7fe57 100644
> > --- a/pyanaconda/bootloader.py
> > +++ b/pyanaconda/bootloader.py
> > @@ -39,6 +39,7 @@ from blivet.fcoe import fcoe
> >  import pyanaconda.network
> >  from pyanaconda.nm import nm_device_hwaddress
> >  from blivet import platform
> > +from blivet.size import Size
> >  from pyanaconda.i18n import _, N_
> >  
> >  import logging
> > @@ -243,7 +244,7 @@ class BootLoader(object):
> >      stage2_bootable = False
> >      stage2_must_be_primary = True
> >      stage2_description = N_("/boot filesystem")
> > -    stage2_max_end_mb = 2 * 1024 * 1024
> > +    stage2_max_end = Size(en_spec="2 TiB")
> >  
> >      @property
> >      def stage2_format_types(self):
> > @@ -490,15 +491,15 @@ class BootLoader(object):
> >          log.debug("_is_valid_size(%s) returning %s", device.name, ret)
> >          return ret
> >  
> > -    def _is_valid_location(self, device, max_mb=None, desc=""):
> > +    def _is_valid_location(self, device, max_end=None, desc=""):
> >          ret = True
> > -        if max_mb and device.type == "partition" and device.partedPartition:
> > +        if max_end and device.type == "partition" and
> > device.partedPartition:
> >              end_sector = device.partedPartition.geometry.end
> >              sector_size = device.partedPartition.disk.device.sectorSize
> > -            end_mb = (sector_size * end_sector) / (1024.0 * 1024.0)
> > -            if end_mb > max_mb:
> > -                self.errors.append(_("%(desc)s must be within the first
> > %(max_mb)dMB of "
> > -                                     "the disk.") % {"desc": desc, "max_mb":
> > max_mb})
> > +            end = Size(bytes=sector_size * end_sector)
> > +            if end > max_end:
> > +                self.errors.append(_("%(desc)s must be within the first
> > %(max_end)s of "
> > +                                     "the disk.") % {"desc": desc,
> > "max_end": max_end})
> >                  ret = False
> >  
> >          log.debug("_is_valid_location(%s) returning %s", device.name, ret)
> > @@ -612,7 +613,7 @@ class BootLoader(object):
> >              valid = False
> >  
> >          if not self._is_valid_location(device,
> > -                                       max_mb=constraint["max_end_mb"],
> > +                                       max_end=constraint["max_end"],
> >                                         desc=description):
> >              valid = False
> >  
> > @@ -714,7 +715,7 @@ class BootLoader(object):
> >              valid = False
> >  
> >          if not self._is_valid_location(device,
> > -                                       max_mb=self.stage2_max_end_mb,
> > +                                       max_end=self.stage2_max_end,
> >                                         desc=_(self.stage2_description)):
> >              valid = False
> >  
> > diff --git a/pyanaconda/installclass.py b/pyanaconda/installclass.py
> > index 6293a27..f3d4bc5 100644
> > --- a/pyanaconda/installclass.py
> > +++ b/pyanaconda/installclass.py
> > @@ -28,6 +28,7 @@ import imputil
> >  from blivet.partspec import PartSpec
> >  from blivet.devicelibs import swap
> >  from blivet.platform import platform
> > +from blivet.size import Size
> >  
> >  import logging
> >  log = logging.getLogger("anaconda")
> > @@ -78,10 +79,14 @@ class BaseInstallClass(object):
> >  
> >      def setDefaultPartitioning(self, storage):
> >          autorequests = [PartSpec(mountpoint="/",
> >          fstype=storage.defaultFSType,
> > -                                 size=1024, maxSize=50*1024, grow=True,
> > +                                 size=Size(spec="1GiB"),
> > +                                 maxSize=Size(spec="50GiB"),
> > +                                 grow=True,
> >                                   btr=True, lv=True, thin=True,
> >                                   encrypted=True),
> > -                        PartSpec(mountpoint="/home",
> > fstype=storage.defaultFSType,
> > -                                 size=500, grow=True, requiredSpace=50*1024,
> > +                        PartSpec(mountpoint="/home",
> > +                                 fstype=storage.defaultFSType,
> > +                                 size=Size(spec="500MiB"), grow=True,
> > +                                 requiredSpace=Size(spec="50GiB"),
> >                                   btr=True, lv=True, thin=True,
> >                                   encrypted=True)]
> >  
> >          bootreqs = platform.setDefaultPartitioning()
> > diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
> > index e59a896..6c83d26 100644
> > --- a/pyanaconda/kickstart.py
> > +++ b/pyanaconda/kickstart.py
> > @@ -26,6 +26,7 @@ from blivet.devicelibs import swap
> >  from blivet.formats import getFormat
> >  from blivet.partitioning import doPartitioning
> >  from blivet.partitioning import growLVM
> > +from blivet.size import Size
> >  from blivet import udev
> >  import blivet.iscsi
> >  import blivet.fcoe
> > @@ -682,12 +683,17 @@ class LogVolData(commands.logvol.F20_LogVolData):
> >          # we might have truncated or otherwise changed the specified vg name
> >          vgname = ksdata.onPart.get(self.vgname, self.vgname)
> >  
> > +        try:
> > +            size = Size(spec="%d MiB" % self.size)
> > +        except Exception:
> > +            size = None
> > +
> >          if self.mountpoint == "swap":
> >              ty = "swap"
> >              self.mountpoint = ""
> >              if self.recommended or self.hibernation:
> >                  disk_space = getAvailableDiskSpace(storage)
> > -                self.size =
> > swap.swapSuggestion(hibernation=self.hibernation, disk_space=disk_space)
> > +                size = swap.swapSuggestion(hibernation=self.hibernation,
> > disk_space=disk_space)
> >                  self.grow = False
> >          else:
> >              if self.fstype != "":
> > @@ -729,19 +735,19 @@ class LogVolData(commands.logvol.F20_LogVolData):
> >                  raise KickstartValueError(formatErrorMsg(self.lineno,
> >                  msg="No preexisting logical volume with the name \"%s\" was
> >                  found." % self.name))
> >  
> >              if self.resize:
> > -                if self.size < dev.currentSize:
> > +                if size < dev.currentSize:
> >                      # shrink
> >                      try:
> > -                        devicetree.registerAction(ActionResizeFormat(dev,
> > self.size))
> > -                        devicetree.registerAction(ActionResizeDevice(dev,
> > self.size))
> > +                        devicetree.registerAction(ActionResizeFormat(dev,
> > size))
> > +                        devicetree.registerAction(ActionResizeDevice(dev,
> > size))
> >                      except ValueError:
> >                          raise
> >                          KickstartValueError(formatErrorMsg(self.lineno,
> >                                  msg="Invalid target size (%d) for device %s"
> >                                  % (self.size, dev.name)))
> >                  else:
> >                      # grow
> >                      try:
> > -                        devicetree.registerAction(ActionResizeDevice(dev,
> > self.size))
> > -                        devicetree.registerAction(ActionResizeFormat(dev,
> > self.size))
> > +                        devicetree.registerAction(ActionResizeDevice(dev,
> > size))
> > +                        devicetree.registerAction(ActionResizeFormat(dev,
> > size))
> >                      except ValueError:
> >                          raise
> >                          KickstartValueError(formatErrorMsg(self.lineno,
> >                                  msg="Invalid target size (%d) for device %s"
> >                                  % (self.size, dev.name)))
> > @@ -760,9 +766,9 @@ class LogVolData(commands.logvol.F20_LogVolData):
> >  
> >              # Size specification checks
> >              if not self.percent:
> > -                if not self.size:
> > +                if not size:
> >                      raise KickstartValueError(formatErrorMsg(self.lineno,
> >                      msg="Size required"))
> > -                elif not self.grow and self.size*1024 < vg.peSize:
> > +                elif not self.grow and size < vg.peSize:
> >                      raise KickstartValueError(formatErrorMsg(self.lineno,
> >                      msg="Logical volume size must be larger than the volume
> >                      group physical extent size."))
> >              elif self.percent <= 0 or self.percent > 100:
> >                  raise KickstartValueError(formatErrorMsg(self.lineno,
> >                  msg="Percentage must be between 0 and 100"))
> > @@ -789,7 +795,7 @@ class LogVolData(commands.logvol.F20_LogVolData):
> >  
> >              if self.resize:
> >                  try:
> > -                    devicetree.registerAction(ActionResizeDevice(device,
> > self.size))
> > +                    devicetree.registerAction(ActionResizeDevice(device,
> > size))
> >                  except ValueError:
> >                      raise KickstartValueError(formatErrorMsg(self.lineno,
> >                              msg="Invalid target size (%d) for device %s" %
> >                              (self.size, device.name)))
> > @@ -813,19 +819,24 @@ class LogVolData(commands.logvol.F20_LogVolData):
> >                  parents = [vg]
> >  
> >              if self.thin_pool:
> > -                pool_args = { "metadatasize": self.metadata_size,
> > -                              "chunksize": self.chunk_size / 1024.0 }
> > +                pool_args = { "metadatasize": Size(spec="%d MiB" %
> > self.metadata_size),
> > +                              "chunksize": Size(spec="%d KiB" %
> > self.chunk_size) }
> >              else:
> >                  pool_args = {}
> >  
> > +            if self.maxSizeMB:
> > +                max_size = Size(spec="%d MiB" % self.maxSizeMB)
> > +            else:
> > +                max_size = None
> > +
> >              request = storage.newLV(format=fmt,
> >                                      name=self.name,
> >                                      parents=parents,
> > -                                    size=self.size,
> > +                                    size=size,
> >                                      thin_pool=self.thin_pool,
> >                                      thin_volume=self.thin_volume,
> >                                      grow=self.grow,
> > -                                    maxsize=self.maxSizeMB,
> > +                                    maxsize=max_size,
> >                                      percent=self.percent,
> >                                      **pool_args)
> >  
> > @@ -901,6 +912,11 @@ class PartitionData(commands.partition.F18_PartData):
> >  
> >          storage.doAutoPart = False
> >  
> > +        try:
> > +            size = Size(spec="%d MiB" % self.size)
> > +        except Exception:
> > +            size = None
> > +
> >          if self.onbiosdisk != "":
> >              for (disk, biosdisk) in storage.eddDict.iteritems():
> >                  if "%x" % biosdisk == self.onbiosdisk:
> > @@ -915,7 +931,7 @@ class PartitionData(commands.partition.F18_PartData):
> >              self.mountpoint = ""
> >              if self.recommended or self.hibernation:
> >                  disk_space = getAvailableDiskSpace(storage)
> > -                self.size =
> > swap.swapSuggestion(hibernation=self.hibernation, disk_space=disk_space)
> > +                size = swap.swapSuggestion(hibernation=self.hibernation,
> > disk_space=disk_space)
> >                  self.grow = False
> >          # if people want to specify no mountpoint for some reason, let them
> >          # this is really needed for pSeries boot partitions :(
> > @@ -989,19 +1005,19 @@ class PartitionData(commands.partition.F18_PartData):
> >                  raise KickstartValueError(formatErrorMsg(self.lineno,
> >                  msg="No preexisting partition with the name \"%s\" was
> >                  found." % self.onPart))
> >  
> >              if self.resize:
> > -                if self.size < dev.currentSize:
> > +                if size < dev.currentSize:
> >                      # shrink
> >                      try:
> > -                        devicetree.registerAction(ActionResizeFormat(dev,
> > self.size))
> > -                        devicetree.registerAction(ActionResizeDevice(dev,
> > self.size))
> > +                        devicetree.registerAction(ActionResizeFormat(dev,
> > size))
> > +                        devicetree.registerAction(ActionResizeDevice(dev,
> > size))
> >                      except ValueError:
> >                          raise
> >                          KickstartValueError(formatErrorMsg(self.lineno,
> >                                  msg="Invalid target size (%d) for device %s"
> >                                  % (self.size, dev.name)))
> >                  else:
> >                      # grow
> >                      try:
> > -                        devicetree.registerAction(ActionResizeDevice(dev,
> > self.size))
> > -                        devicetree.registerAction(ActionResizeFormat(dev,
> > self.size))
> > +                        devicetree.registerAction(ActionResizeDevice(dev,
> > size))
> > +                        devicetree.registerAction(ActionResizeFormat(dev,
> > size))
> >                      except ValueError:
> >                          raise
> >                          KickstartValueError(formatErrorMsg(self.lineno,
> >                                  msg="Invalid target size (%d) for device %s"
> >                                  % (self.size, dev.name)))
> > @@ -1018,7 +1034,7 @@ class PartitionData(commands.partition.F18_PartData):
> >                                       label=self.label,
> >                                       fsprofile=self.fsprofile,
> >                                       mountopts=self.fsopts,
> > -                                     size=self.size)
> > +                                     size=size)
> >          if not kwargs["format"].type:
> >              raise KickstartValueError(formatErrorMsg(self.lineno, msg="The
> >              \"%s\" filesystem type is not supported." % ty))
> >  
> > @@ -1051,8 +1067,10 @@ class PartitionData(commands.partition.F18_PartData):
> >                  raise KickstartValueError(formatErrorMsg(self.lineno,
> >                  msg="Specified nonexistent disk %s in partition command" %
> >                  self.disk))
> >  
> >          kwargs["grow"] = self.grow
> > -        kwargs["size"] = self.size
> > -        kwargs["maxsize"] = self.maxSizeMB
> > +        kwargs["size"] = size
> > +        if self.maxSizeMB:
> > +            kwargs["maxsize"] = Size(spec="%d MiB" % self.maxSizeMB)
> > +
> >          kwargs["primary"] = self.primOnly
> >  
> >          # If we were given a pre-existing partition to create a filesystem
> >          on,
> > @@ -1067,7 +1085,7 @@ class PartitionData(commands.partition.F18_PartData):
> >              removeExistingFormat(device, storage)
> >              if self.resize:
> >                  try:
> > -                    devicetree.registerAction(ActionResizeDevice(device,
> > self.size))
> > +                    devicetree.registerAction(ActionResizeDevice(device,
> > size))
> >                  except ValueError:
> >                      raise KickstartValueError(formatErrorMsg(self.lineno,
> >                              msg="Invalid target size (%d) for device %s" %
> >                              (self.size, device.name)))
> > @@ -1436,7 +1454,8 @@ class
> > VolGroupData(commands.volgroup.FC16_VolGroupData):
> >          if len(pvs) == 0 and not self.preexist:
> >              raise KickstartValueError(formatErrorMsg(self.lineno,
> >              msg="Volume group defined without any physical volumes.  Either
> >              specify physical volumes or use --useexisting."))
> >  
> > -        if self.pesize not in getPossiblePhysicalExtents(floor=1024):
> > +        pesize = Size(spec="%d KiB" % self.pesize)
> > +        if pesize not in getPossiblePhysicalExtents(floor=1024):
> >              raise KickstartValueError(formatErrorMsg(self.lineno,
> >              msg="Volume group specified invalid pesize"))
> >  
> >          # If --noformat or --useexisting was given, there's really nothing
> >          to do.
> > @@ -1452,7 +1471,7 @@ class
> > VolGroupData(commands.volgroup.FC16_VolGroupData):
> >          else:
> >              request = storage.newVG(parents=pvs,
> >                                      name=self.vgname,
> > -                                    peSize=self.pesize/1024.0)
> > +                                    peSize=pesize)
> >  
> >              storage.createDevice(request)
> >              if self.reserved_space:
> > diff --git a/pyanaconda/ui/gui/spokes/custom.glade
> > b/pyanaconda/ui/gui/spokes/custom.glade
> > index 48a1be0..d6e99ee 100644
> > --- a/pyanaconda/ui/gui/spokes/custom.glade
> > +++ b/pyanaconda/ui/gui/spokes/custom.glade
> > @@ -1002,7 +1002,7 @@ use.  Try something else?</property>
> >                                      <property
> >                                      name="can_focus">True</property>
> >                                      <property name="halign">start</property>
> >                                      <property
> >                                      name="invisible_char">●</property>
> > -                                    <property
> > name="width_chars">10</property>
> > +                                    <property
> > name="width_chars">20</property>
> >                                      <signal name="changed"
> >                                      handler="on_value_changed"
> >                                      swapped="no"/>
> >                                    </object>
> >                                    <packing>
> > diff --git a/pyanaconda/ui/gui/spokes/custom.py
> > b/pyanaconda/ui/gui/spokes/custom.py
> > index 46fd657..fc0f6c3 100644
> > --- a/pyanaconda/ui/gui/spokes/custom.py
> > +++ b/pyanaconda/ui/gui/spokes/custom.py
> > @@ -63,7 +63,7 @@ from blivet.partitioning import doAutoPartition
> >  from blivet.errors import StorageError
> >  from blivet.errors import NoDisksError
> >  from blivet.errors import NotEnoughFreeSpaceError
> > -from blivet.errors import SizeParamsError, SizeNotPositiveError
> > +from blivet.errors import SizeParamsError
> >  from blivet.devicelibs import mdraid
> >  from blivet.devices import LUKSDevice
> >  
> > @@ -79,7 +79,6 @@ from pyanaconda.ui.gui.spokes.lib.summary import
> > ActionSummaryDialog
> >  from pyanaconda.ui.gui.utils import setViewportBackground, gtk_action_wait,
> >  enlightbox, fancy_set_sensitive, ignoreEscape,\
> >          really_hide, really_show
> >  from pyanaconda.ui.gui.categories.system import SystemCategory
> > -from pyanaconda.ui.lib.disks import size_str
> >  
> >  from gi.repository import Gdk, Gtk
> >  from gi.repository.AnacondaWidgets import MountpointSelector
> > @@ -151,18 +150,18 @@ def size_from_entry(entry):
> >      size_text = entry.get_text().decode("utf-8").strip()
> >  
> >      try:
> > -        # if no unit was specified, default to MB. Assume that a string
> > +        # if no unit was specified, default to MiB. Assume that a string
> >          # ending with any kind of a letter has a unit suffix.
> >          if size_text and
> >          unicodedata.category(size_text[-1]).startswith("L"):
> >              size = Size(spec=size_text)
> >          else:
> > -            size = Size(en_spec="%sMB" % size_text)
> > -    except (SizeParamsError, SizeNotPositiveError, ValueError):
> > +            size = Size(en_spec="%sMiB" % size_text)
> > +    except (SizeParamsError, ValueError):
> >          return None
> >      else:
> > -        # Minimium size for ui-created partitions is 1MB.
> > -        if size.convertTo(en_spec="mb") < 1:
> > -            size = Size(en_spec="1mb")
> > +        # Minimium size for ui-created partitions is 1MiB.
> > +        if size.convertTo(en_spec="mib") < 1:
> > +            size = Size(en_spec="1mib")
> >  
> >      return size
> >  
> > @@ -358,7 +357,7 @@ class DisksDialog(GUIObject):
> >          # populate the store
> >          for disk in self._disks:
> >              self._store.append([disk.description,
> > -                                str(Size(en_spec="%dMB" % disk.size)),
> > +                                str(disk.size),
> >                                  str(free[disk.name][0]),
> >                                  disk.serial,
> >                                  disk.id])
> > @@ -420,7 +419,7 @@ class ContainerDialog(GUIObject):
> >          self.exists = kwargs.pop("exists", False)
> >  
> >          self.size_policy = kwargs.pop("size_policy", SIZE_POLICY_AUTO)
> > -        self.size = kwargs.pop("size", 0)
> > +        self.size = kwargs.pop("size", Size(bytes=0))
> >  
> >          self._error = None
> >          GUIObject.__init__(self, *args, **kwargs)
> > @@ -444,7 +443,7 @@ class ContainerDialog(GUIObject):
> >          # populate the store
> >          for disk in self._disks:
> >              self._store.append([disk.description,
> > -                                str(Size(en_spec="%dMB" % disk.size)),
> > +                                str(disk.size),
> >                                  str(free[disk.name][0]),
> >                                  disk.serial,
> >                                  disk.id])
> > @@ -470,8 +469,7 @@ class ContainerDialog(GUIObject):
> >          self._raidStoreFilter.refilter()
> >          self._populate_raid()
> >  
> > -        size = Size(en_spec="%d mb" % self.size)
> > -        self._sizeEntry.set_text(size.humanReadable(max_places=None))
> > +        self._sizeEntry.set_text(self.size.humanReadable(max_places=None))
> >          if self.size_policy == SIZE_POLICY_AUTO:
> >              self._sizeCombo.set_active(0)
> >          elif self.size_policy == SIZE_POLICY_MAX:
> > @@ -545,9 +543,7 @@ class ContainerDialog(GUIObject):
> >              size = SIZE_POLICY_MAX
> >          elif idx == 2:
> >              size = size_from_entry(self._sizeEntry)
> > -            if size:
> > -                size = int(size.convertTo(en_spec="MB"))
> > -            elif size is None:
> > +            if size is None:
> >                  size = SIZE_POLICY_MAX
> >  
> >          # now save the changes
> > @@ -843,12 +839,9 @@ class CustomPartitioningSpoke(NormalSpoke,
> > StorageChecker):
> >  
> >      def _currentTotalSpace(self):
> >          """Add up the sizes of all selected disks and return it as a
> >          Size."""
> > -        totalSpace = 0
> > -
> > -        for disk in self._clearpartDevices:
> > -            totalSpace += disk.size
> > -
> > -        return Size(en_spec="%f MB" % totalSpace)
> > +        totalSpace = Size(bytes=0)
> > +        totalSpace += sum(disk.size for disk in self._clearpartDevices)
> > +        return totalSpace
> >  
> >      def _updateSpaceDisplay(self):
> >          # Set up the free space/available space displays in the bottom left.
> > @@ -1161,10 +1154,8 @@ class CustomPartitioningSpoke(NormalSpoke,
> > StorageChecker):
> >          # SIZE
> >          old_size = device.size
> >          size = size_from_entry(self._sizeEntry)
> > -        if size:
> > -            size = int(size.convertTo(en_spec="MB"))
> >          changed_size = ((use_dev.resizable or not use_dev.exists) and
> > -                        size != int(old_size))
> > +                        size != old_size)
> >          log.debug("old size: %s", old_size)
> >          log.debug("new size: %s", size)
> >  
> > @@ -1476,7 +1467,7 @@ class CustomPartitioningSpoke(NormalSpoke,
> > StorageChecker):
> >              # And then we need to re-check that the max size is actually
> >              # different from the current size.
> >              _changed_size = False
> > -            if size != device.size and int(size) == int(device.currentSize):
> > +            if size != device.size and size == device.currentSize:
> >                  # size has been set back to its original value
> >                  actions =
> >                  self.__storage.devicetree.findActions(type="resize",
> >                                                                  devid=device.id)
> > @@ -1485,7 +1476,7 @@ class CustomPartitioningSpoke(NormalSpoke,
> > StorageChecker):
> >                          self.__storage.devicetree.cancelAction(action)
> >                          _changed_size = True
> >              elif size != device.size:
> > -                log.debug("scheduling resize of device %s to %s MB",
> > device.name, size)
> > +                log.debug("scheduling resize of device %s to %s",
> > device.name, size)
> >  
> >                  with ui_storage_logger():
> >                      try:
> > @@ -1507,7 +1498,7 @@ class CustomPartitioningSpoke(NormalSpoke,
> > StorageChecker):
> >                  # update the selector's size property
> >                  for s in self._accordion.allSelectors:
> >                      if s._device == device:
> > -                        s.size = size_str(device.size)
> > +                        s.size = str(device.size)
> >  
> >                  # update size props of all btrfs devices' selectors
> >                  self._update_selectors()
> > @@ -1736,7 +1727,7 @@ class CustomPartitioningSpoke(NormalSpoke,
> > StorageChecker):
> >          else:
> >              self._labelEntry.set_tooltip_text(_("This file system does not
> >              support labels."))
> >  
> > -        self._sizeEntry.set_text(Size(en_spec="%d MB" %
> > device.size).humanReadable(max_places=None))
> > +        self._sizeEntry.set_text(device.size.humanReadable(max_places=None))
> >  
> >          self._reformatCheckbox.set_active(not device.format.exists)
> >          fancy_set_sensitive(self._reformatCheckbox, not device.protected and
> > @@ -2027,9 +2018,6 @@ class CustomPartitioningSpoke(NormalSpoke,
> > StorageChecker):
> >              encrypted = False
> >  
> >          disks = self._clearpartDevices
> > -        if size is not None:
> > -            size = float(size.convertTo(en_spec="mb"))
> > -
> >          self.clear_errors()
> >  
> >          with ui_storage_logger():
> > @@ -2377,7 +2365,7 @@ class CustomPartitioningSpoke(NormalSpoke,
> > StorageChecker):
> >  
> >      def _container_store_row(self, name, freeSpace=None):
> >          if freeSpace is not None:
> > -            return [name, _("(%s free)") % size_str(freeSpace)]
> > +            return [name, _("(%s free)") % freeSpace]
> >          else:
> >              return [name, ""]
> >  
> > diff --git a/pyanaconda/ui/gui/spokes/filter.py
> > b/pyanaconda/ui/gui/spokes/filter.py
> > index aa30115..422a5f1 100644
> > --- a/pyanaconda/ui/gui/spokes/filter.py
> > +++ b/pyanaconda/ui/gui/spokes/filter.py
> > @@ -29,7 +29,7 @@ from blivet.fcoe import has_fcoe
> >  from pyanaconda.flags import flags
> >  from pyanaconda.i18n import CN_, CP_
> >  
> > -from pyanaconda.ui.lib.disks import getDisks, isLocalDisk, size_str
> > +from pyanaconda.ui.lib.disks import getDisks, isLocalDisk
> >  from pyanaconda.ui.gui.utils import enlightbox
> >  from pyanaconda.ui.gui.spokes import NormalSpoke
> >  from pyanaconda.ui.gui.spokes.advstorage.fcoe import FCoEDialog
> > @@ -229,7 +229,7 @@ class MultipathPage(FilterPage):
> >              selected = disk.name in selectedNames
> >  
> >              store.append([True, selected, not disk.protected,
> > -                          disk.name, "", disk.model, size_str(disk.size),
> > +                          disk.name, "", disk.model, str(disk.size),
> >                            disk.vendor, disk.bus, disk.serial,
> >                            disk.wwid, "\n".join(paths), "", "",
> >                            "", ""])
> > @@ -314,7 +314,7 @@ class OtherPage(FilterPage):
> >                  lun = ""
> >  
> >              store.append([True, selected, not disk.protected,
> > -                          disk.name, "", disk.model, size_str(disk.size),
> > +                          disk.name, "", disk.model, str(disk.size),
> >                            disk.vendor, disk.bus, disk.serial,
> >                            self._long_identifier(disk), "", port,
> >                            getattr(disk, "initiator", ""),
> >                            lun, ""])
> > diff --git a/pyanaconda/ui/gui/spokes/lib/accordion.py
> > b/pyanaconda/ui/gui/spokes/lib/accordion.py
> > index af185b7..260b2c0 100644
> > --- a/pyanaconda/ui/gui/spokes/lib/accordion.py
> > +++ b/pyanaconda/ui/gui/spokes/lib/accordion.py
> > @@ -20,7 +20,6 @@
> >  # Red Hat Author(s): Chris Lumens <clumens at redhat.com>
> >  #
> >  
> > -from blivet.size import Size
> >  
> >  from pyanaconda.i18n import _
> >  from pyanaconda.product import productName, productVersion
> > @@ -59,14 +58,12 @@ def selectorFromDevice(device, selector=None,
> > mountpoint=""):
> >      else:
> >          mp = _("Unknown")
> >  
> > -    size = Size(en_spec="%f MB" % device.size)
> > -
> >      if not selector:
> > -        selector = MountpointSelector(device.name, str(size).upper(), mp)
> > +        selector = MountpointSelector(device.name, str(device.size), mp)
> >          selector._root = None
> >      else:
> >          selector.props.name = device.name
> > -        selector.props.size = str(size).upper()
> > +        selector.props.size = str(device.size)
> >          selector.props.mountpoint = mp
> >  
> >      selector._device = device
> > diff --git a/pyanaconda/ui/gui/spokes/lib/cart.py
> > b/pyanaconda/ui/gui/spokes/lib/cart.py
> > index a78a48c..8854424 100644
> > --- a/pyanaconda/ui/gui/spokes/lib/cart.py
> > +++ b/pyanaconda/ui/gui/spokes/lib/cart.py
> > @@ -23,7 +23,6 @@
> >  from gi.repository import Gtk
> >  
> >  from pyanaconda.i18n import C_, P_
> > -from pyanaconda.ui.lib.disks import size_str
> >  from pyanaconda.ui.gui import GUIObject
> >  from pyanaconda.ui.gui.utils import escape_markup
> >  from blivet.size import Size
> > @@ -60,8 +59,8 @@ class SelectedDisksDialog(GUIObject):
> >          for disk in disks:
> >              self._store.append([False,
> >                                  "%s (%s)" % (disk.description, disk.serial),
> > -                                size_str(disk.size),
> > -                                size_str(free[disk.name][0]),
> > +                                str(disk.size),
> > +                                str(free[disk.name][0]),
> >                                  disk.name,
> >                                  disk.id])
> >          self.disks = disks[:]
> > diff --git a/pyanaconda/ui/gui/spokes/lib/resize.py
> > b/pyanaconda/ui/gui/spokes/lib/resize.py
> > index 3cca5a5..ad50bf8 100644
> > --- a/pyanaconda/ui/gui/spokes/lib/resize.py
> > +++ b/pyanaconda/ui/gui/spokes/lib/resize.py
> > @@ -25,7 +25,6 @@ from collections import namedtuple
> >  from gi.repository import Gdk, Gtk
> >  
> >  from pyanaconda.i18n import _, C_, N_, P_
> > -from pyanaconda.ui.lib.disks import size_str
> >  from pyanaconda.ui.gui import GUIObject
> >  from pyanaconda.ui.gui.utils import escape_markup, timed_action
> >  from blivet.size import Size
> > @@ -60,8 +59,8 @@ class ResizeDialog(GUIObject):
> >          self.storage = storage
> >          self.payload = payload
> >  
> > -        self._initialFreeSpace = Size(0)
> > -        self._selectedReclaimableSpace = 0
> > +        self._initialFreeSpace = Size(bytes=0)
> > +        self._selectedReclaimableSpace = Size(bytes=0)
> >  
> >          self._actionStore = self.builder.get_object("actionStore")
> >          self._diskStore = self.builder.get_object("diskStore")
> > @@ -75,7 +74,7 @@ class ResizeDialog(GUIObject):
> >  
> >          self._required_label = self.builder.get_object("requiredSpaceLabel")
> >          markup = self._required_label.get_label()
> > -        self._required_label.set_markup(markup %
> > escape_markup(size_str(self.payload.spaceRequired)))
> > +        self._required_label.set_markup(markup %
> > escape_markup(str(self.payload.spaceRequired)))
> >  
> >          self._reclaimDescLabel = self.builder.get_object("reclaimDescLabel")
> >  
> > @@ -112,10 +111,10 @@ class ResizeDialog(GUIObject):
> >  
> >      def populate(self, disks):
> >          totalDisks = 0
> > -        totalReclaimableSpace = 0
> > +        totalReclaimableSpace = Size(bytes=0)
> >  
> > -        self._initialFreeSpace = Size(0)
> > -        self._selectedReclaimableSpace = 0
> > +        self._initialFreeSpace = Size(bytes=0)
> > +        self._selectedReclaimableSpace = Size(bytes=0)
> >  
> >          canShrinkSomething = False
> >  
> > @@ -127,13 +126,13 @@ class ResizeDialog(GUIObject):
> >  
> >              if disk.partitioned:
> >                  fstype = ""
> > -                diskReclaimableSpace = 0
> > +                diskReclaimableSpace = Size(bytes=0)
> >              else:
> >                  fstype = disk.format.name
> >                  diskReclaimableSpace = disk.size
> >  
> >              itr = self._diskStore.append(None, [disk.id,
> > -                                                "%s %s" %
> > (size_str(disk.size), disk.description),
> > +                                                "%s %s" % (disk.size,
> > disk.description),
> >                                                  fstype,
> >                                                  "<span foreground='grey'
> >                                                  style='italic'>%s
> >                                                  total</span>",
> >                                                  _(PRESERVE),
> > @@ -152,7 +151,7 @@ class ResizeDialog(GUIObject):
> >                      if dev.resizable:
> >                          freeSize = dev.size - dev.minSize
> >                          resizeString = _("%(freeSize)s of %(devSize)s") \
> > -                                       % {"freeSize": size_str(freeSize),
> > "devSize": size_str(dev.size)}
> > +                                       % {"freeSize": freeSize, "devSize":
> > dev.size}
> >                          if not dev.protected:
> >                              canShrinkSomething = True
> >                      else:
> > @@ -174,24 +173,23 @@ class ResizeDialog(GUIObject):
> >              # And then add another uneditable line that lists how much space
> >              is
> >              # already free in the disk.
> >              diskFree = free_space[disk.name][0]
> > -            converted = diskFree.convertTo(en_spec="mb")
> > -            if int(converted):
> > +            if diskFree >= Size(en_spec="1MiB"):
> >                  freeSpaceString = "<span foreground='grey'
> >                  style='italic'>%s</span>" % \
> >                          escape_markup(_("Free space"))
> >                  self._diskStore.append(itr, [disk.id,
> >                                               freeSpaceString,
> >                                               "",
> > -                                             "<span foreground='grey'
> > style='italic'>%s</span>" % size_str(diskFree),
> > +                                             "<span foreground='grey'
> > style='italic'>%s</span>" % diskFree,
> >                                               _(PRESERVE),
> >                                               False,
> >                                               self._get_tooltip(disk),
> > -                                             float(converted),
> > +                                             diskFree,
> >                                               ""])
> >                  self._initialFreeSpace += diskFree
> >  
> >              # And then go back and fill in the total reclaimable space for
> >              the
> >              # disk, now that we know what each partition has reclaimable.
> > -            self._diskStore[itr][RECLAIMABLE_COL] =
> > self._diskStore[itr][RECLAIMABLE_COL] % size_str(diskReclaimableSpace)
> > +            self._diskStore[itr][RECLAIMABLE_COL] =
> > self._diskStore[itr][RECLAIMABLE_COL] % diskReclaimableSpace
> >  
> >              totalDisks += 1
> >              totalReclaimableSpace += diskReclaimableSpace
> > @@ -216,12 +214,12 @@ class ResizeDialog(GUIObject):
> >              text = P_("<b>%(count)s disk; %(size)s reclaimable space</b> (in
> >              filesystems)",
> >                        "<b>%(count)s disks; %(size)s reclaimable space</b>
> >                        (in filesystems)",
> >                        nDisks) % {"count" : escape_markup(str(nDisks)),
> > -                                 "size" :
> > escape_markup(size_str(totalReclaimable))}
> > +                                 "size" : escape_markup(totalReclaimable)}
> >              self._reclaimable_label.set_markup(text)
> >  
> >          if selectedReclaimable is not None:
> >              text = _("Total selected space to reclaim: <b>%s</b>") % \
> > -                    escape_markup(size_str(selectedReclaimable))
> > +                    escape_markup(selectedReclaimable)
> >              self._selected_label.set_markup(text)
> >  
> >      def _setup_slider(self, device, value):
> > @@ -239,9 +237,9 @@ class ResizeDialog(GUIObject):
> >          # The slider needs to be keyboard-accessible.  We'll make small
> >          movements change in
> >          # 1% increments, and large movements in 5% increments.
> >          distance = device.size - device.minSize
> > -        onePercent = distance*0.01
> > -        fivePercent = distance*0.05
> > -        twentyPercent = distance*0.2
> > +        onePercent = Size(bytes=distance / 100)
> > +        fivePercent = Size(bytes=distance / 20)
> > +        twentyPercent = Size(bytes=distance / 5)
> >  
> >          adjustment = self.builder.get_object("resizeAdjustment")
> >          adjustment.configure(value, device.minSize, device.size, onePercent,
> >          fivePercent, 0)
> > @@ -252,8 +250,8 @@ class ResizeDialog(GUIObject):
> >              self._resizeSlider.add_mark(device.minSize + i*twentyPercent,
> >              Gtk.PositionType.BOTTOM, None)
> >  
> >          # Finally, add tick marks for the ends.
> > -        self._resizeSlider.add_mark(device.minSize, Gtk.PositionType.BOTTOM,
> > size_str(device.minSize))
> > -        self._resizeSlider.add_mark(device.size, Gtk.PositionType.BOTTOM,
> > size_str(device.size))
> > +        self._resizeSlider.add_mark(device.minSize, Gtk.PositionType.BOTTOM,
> > str(device.minSize))
> > +        self._resizeSlider.add_mark(device.size, Gtk.PositionType.BOTTOM,
> > str(device.size))
> >  
> >      def _update_action_buttons(self, row):
> >          obj = PartStoreRow(*row)
> > @@ -274,7 +272,7 @@ class ResizeDialog(GUIObject):
> >          self._shrinkButton.set_sensitive(device.resizable)
> >  
> >          if device.resizable:
> > -            self._setup_slider(device, obj.target)
> > +            self._setup_slider(device, Size(bytes=obj.target))
> >  
> >          # Then, disable the button for whatever action is currently
> >          selected.
> >          # It doesn't make a lot of sense to allow clicking that.
> > @@ -293,7 +291,7 @@ class ResizeDialog(GUIObject):
> >          # (2) At least one destructive action has been chosen.  We can
> >          detect
> >          #     this by checking whether got is non-zero.
> >          need = self.payload.spaceRequired
> > -        self._resizeButton.set_sensitive(got+self._initialFreeSpace >= need
> > and got > Size(0))
> > +        self._resizeButton.set_sensitive(got+self._initialFreeSpace >= need
> > and got > Size(bytes=0))
> >  
> >      # pylint: disable-msg=W0221
> >      def refresh(self, disks):
> > @@ -330,7 +328,7 @@ class ResizeDialog(GUIObject):
> >          if obj.action == _(PRESERVE):
> >              return False
> >          if obj.action == _(SHRINK):
> > -            self._selectedReclaimableSpace += device.size - obj.target
> > +            self._selectedReclaimableSpace += device.size -
> > Size(bytes=obj.target)
> >          elif obj.action == _(DELETE):
> >              self._selectedReclaimableSpace += device.size
> >  
> > @@ -376,11 +374,11 @@ class ResizeDialog(GUIObject):
> >  
> >          # And then we're keeping a running tally of how much space the user
> >          # has selected to reclaim, so reflect that in the UI.
> > -        self._selectedReclaimableSpace = 0
> > +        self._selectedReclaimableSpace = Size(bytes=0)
> >          self._diskStore.foreach(self._sumReclaimableSpace, None)
> >          self._update_labels(selectedReclaimable=self._selectedReclaimableSpace)
> >  
> > -        self._update_reclaim_button(Size(en_spec="%f MB" %
> > self._selectedReclaimableSpace))
> > +        self._update_reclaim_button(self._selectedReclaimableSpace)
> >          self._update_action_buttons(selectedRow)
> >  
> >      def _scheduleActions(self, model, path, itr, *args):
> > @@ -394,7 +392,8 @@ class ResizeDialog(GUIObject):
> >              return False
> >          elif obj.action == _(SHRINK):
> >              if device.resizable:
> > -                self.storage.resizeDevice(device, obj.target)
> > +                # round this up to nearest MB? MiB? Maybe in targetSize
> > setter.
> > +                self.storage.resizeDevice(device, Size(bytes=obj.target))
> >              else:
> >                  self.storage.recursiveRemove(device)
> >          elif obj.action == _(DELETE):
> > @@ -451,21 +450,21 @@ class ResizeDialog(GUIObject):
> >      def on_resize_value_changed(self, rng):
> >          (model, itr) = self._selection.get_selected()
> >  
> > -        old_delta = rng.get_adjustment().get_upper()-
> > round(model[itr][RESIZE_TARGET_COL], 2)
> > +        old_delta = Size(bytes=rng.get_adjustment().get_upper()) -
> > int(model[itr][RESIZE_TARGET_COL])
> >          self._selectedReclaimableSpace -= old_delta
> >  
> >          # Update the target size in the store.
> > -        model[itr][RESIZE_TARGET_COL] = round(rng.get_value(), 2)
> > +        model[itr][RESIZE_TARGET_COL] = Size(bytes=rng.get_value())
> >  
> >          # Update the "Total selected space" label.
> > -        delta = rng.get_adjustment().get_upper() - round(rng.get_value(), 2)
> > +        delta = Size(bytes=rng.get_adjustment().get_upper()) -
> > int(rng.get_value())
> >          self._selectedReclaimableSpace += delta
> >          self._update_labels(selectedReclaimable=self._selectedReclaimableSpace)
> >  
> >          # And then the reclaim button, in case they've made enough space.
> > -        self._update_reclaim_button(Size(en_spec="%f MB" %
> > self._selectedReclaimableSpace))
> > +        self._update_reclaim_button(self._selectedReclaimableSpace)
> >  
> >      def resize_slider_format(self, scale, value):
> >          # This makes the value displayed under the slider prettier than just
> >          a
> >          # single number.
> > -        return size_str(value)
> > +        return str(Size(bytes=value))
> > diff --git a/pyanaconda/ui/gui/spokes/storage.py
> > b/pyanaconda/ui/gui/spokes/storage.py
> > index 177282b..c3fecb7 100644
> > --- a/pyanaconda/ui/gui/spokes/storage.py
> > +++ b/pyanaconda/ui/gui/spokes/storage.py
> > @@ -41,7 +41,7 @@
> >  from gi.repository import Gdk, GLib, AnacondaWidgets
> >  
> >  from pyanaconda.ui.communication import hubQ
> > -from pyanaconda.ui.lib.disks import getDisks, isLocalDisk, size_str
> > +from pyanaconda.ui.lib.disks import getDisks, isLocalDisk
> >  from pyanaconda.ui.gui import GUIObject
> >  from pyanaconda.ui.gui.spokes import NormalSpoke
> >  from pyanaconda.ui.gui.spokes.lib.cart import SelectedDisksDialog
> > @@ -227,11 +227,8 @@ class InstallOptions2Dialog(InstallOptions1Dialog):
> >          self.fs_free_label =
> >          self.builder.get_object("options2_fs_free_label")
> >  
> >      def _set_free_space_labels(self, disk_free, fs_free):
> > -        disk_free_text = size_str(disk_free)
> > -        self.disk_free_label.set_text(disk_free_text)
> > -
> > -        fs_free_text = size_str(fs_free)
> > -        self.fs_free_label.set_text(fs_free_text)
> > +        self.disk_free_label.set_text(str(disk_free))
> > +        self.fs_free_label.set_text(str(fs_free))
> >  
> >      def refresh(self, required_space, auto_swap, disk_free, fs_free,
> >      autoPartType, encrypted):
> >          self.autoPartType = autoPartType
> > @@ -272,11 +269,8 @@ class InstallOptions3Dialog(InstallOptions1Dialog):
> >          self.fs_free_label =
> >          self.builder.get_object("options3_fs_free_label")
> >  
> >      def _set_free_space_labels(self, disk_free, fs_free):
> > -        disk_free_text = size_str(disk_free)
> > -        self.disk_free_label.set_text(disk_free_text)
> > -
> > -        fs_free_text = size_str(fs_free)
> > -        self.fs_free_label.set_text(fs_free_text)
> > +        self.disk_free_label.set_text(str(disk_free))
> > +        self.fs_free_label.set_text(str(fs_free))
> >  
> >      def refresh(self, required_space, auto_swap, disk_free, fs_free,
> >      autoPartType, encrypted):
> >          sw_text = self._get_sw_needs_text(required_space, auto_swap)
> > @@ -605,7 +599,6 @@ class StorageSpoke(NormalSpoke, StorageChecker):
> >          else:
> >              kind = "drive-harddisk"
> >  
> > -        size = size_str(disk.size)
> >          if disk.serial:
> >              popup_info = "%s" % disk.serial
> >          else:
> > @@ -623,8 +616,8 @@ class StorageSpoke(NormalSpoke, StorageChecker):
> >  
> >          overview = AnacondaWidgets.DiskOverview(description,
> >                                                  kind,
> > -                                                size,
> > -                                                _("%s free") %
> > size_str(free),
> > +                                                str(disk.size),
> > +                                                _("%s free") % free,
> >                                                  disk.name,
> >                                                  popup=popup_info)
> >          box.pack_start(overview, False, False, 0)
> > @@ -657,7 +650,7 @@ class StorageSpoke(NormalSpoke, StorageChecker):
> >      def _update_summary(self):
> >          """ Update the summary based on the UI. """
> >          count = 0
> > -        capacity = 0
> > +        capacity = Size(bytes=0)
> >          free = Size(bytes=0)
> >  
> >          # pass in our disk list so hidden disks' free space is available
> > @@ -672,7 +665,7 @@ class StorageSpoke(NormalSpoke, StorageChecker):
> >          summary = (P_("%(count)d disk selected; %(capacity)s capacity;
> >          %(free)s free",
> >                        "%(count)d disks selected; %(capacity)s capacity;
> >                        %(free)s free",
> >                        count) % {"count" : count,
> > -                                "capacity" : str(Size(en_spec="%f MB" %
> > capacity)),
> > +                                "capacity" : capacity,
> >                                  "free" : free})
> >          summary_label = self.builder.get_object("summary_label")
> >          summary_label.set_text(summary)
> > @@ -775,7 +768,7 @@ class StorageSpoke(NormalSpoke, StorageChecker):
> >  
> >          # show the installation options dialog
> >          disks = [d for d in self.disks if d.name in self.selected_disks]
> > -        disks_size = sum(Size(en_spec="%f MB" % d.size) for d in disks)
> > +        disks_size = Size(bytes=0) + sum(d.size for d in disks)
> >  
> >          # No disks selected?  The user wants to back out of the storage
> >          spoke.
> >          if not disks:
> > @@ -800,10 +793,9 @@ class StorageSpoke(NormalSpoke, StorageChecker):
> >              fs_free = sum(f[1] for f in free_space.itervalues())
> >  
> >          required_space = self.payload.spaceRequired
> > -        auto_swap = Size(bytes=0)
> > -        for autoreq in self.storage.autoPartitionRequests:
> > -            if autoreq.fstype == "swap":
> > -                auto_swap += Size(en_spec="%d MB" % autoreq.size)
> > +        auto_swap = (Size(bytes=0) +
> > +                     sum(r.size for r in self.storage.autoPartitionRequests
> > +                                    if r.fstype == "swap"))
> >  
> >          log.debug("disk free: %s  fs free: %s  sw needs: %s  auto swap: %s",
> >                    disk_free, fs_free, required_space, auto_swap)
> > diff --git a/pyanaconda/ui/lib/disks.py b/pyanaconda/ui/lib/disks.py
> > index 32389e0..5c9b40c 100644
> > --- a/pyanaconda/ui/lib/disks.py
> > +++ b/pyanaconda/ui/lib/disks.py
> > @@ -25,7 +25,7 @@ from blivet.size import Size
> >  
> >  from pyanaconda.flags import flags
> >  
> > -__all__ = ["FakeDiskLabel", "FakeDisk", "getDisks", "isLocalDisk",
> > "size_str"]
> > +__all__ = ["FakeDiskLabel", "FakeDisk", "getDisks", "isLocalDisk"]
> >  
> >  class FakeDiskLabel(object):
> >      def __init__(self, free=0):
> > @@ -72,11 +72,3 @@ def getDisks(devicetree, fake=False):
> >  
> >  def isLocalDisk(disk):
> >      return not isinstance(disk, MultipathDevice) and not isinstance(disk,
> >      iScsiDiskDevice)
> > -
> > -def size_str(mb):
> > -    if isinstance(mb, Size):
> > -        size = mb
> > -    else:
> > -        size = Size(en_spec="%f mb" % mb)
> > -
> > -    return str(size).upper()
> > diff --git a/pyanaconda/ui/tui/spokes/storage.py
> > b/pyanaconda/ui/tui/spokes/storage.py
> > index 6567fca..71e4796 100644
> > --- a/pyanaconda/ui/tui/spokes/storage.py
> > +++ b/pyanaconda/ui/tui/spokes/storage.py
> > @@ -22,7 +22,7 @@
> >  # which has the same license and authored by David Lehman
> >  <dlehman at redhat.com>
> >  #
> >  
> > -from pyanaconda.ui.lib.disks import getDisks, size_str
> > +from pyanaconda.ui.lib.disks import getDisks
> >  from pyanaconda.ui.tui.spokes import NormalTUISpoke
> >  from pyanaconda.ui.tui.simpleline import TextWidget, CheckboxWidget
> >  
> > @@ -189,9 +189,8 @@ class StorageSpoke(NormalTUISpoke):
> >  
> >          # loop through the disks and present them.
> >          for disk in self.disks:
> > -            size = size_str(disk.size)
> >              c = CheckboxWidget(title="%i) %s: %s (%s)" %
> >              (self.disks.index(disk) + 1,
> > -                                                 disk.model, size,
> > disk.name),
> > +                                                 disk.model, disk.size,
> > disk.name),
> >                                 completed=(disk.name in self.selected_disks))
> >              self._window += [c, ""]
> >  
> > --
> > 1.8.1.4
> > 
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > 
> 
> Some comments are: 
> 1) In pyanaconda/kickstart.py --- in the classes' execute methods you set size to None if it is not possible to construct a Size object from self.size. I think that self.size value is obtained from the kickstart file, so it might be better to have an exception there than to set size to None.

Agreed.

> 2) There are places where abbreviations like "mib" are used. I understand that they're normalized, but they tend to look like powers of a negative constant, due to the use of MB for megabytes and mB for millibytes. The newer decimal prefixes have the same letter for the negative and positive powers, except that the negative power is lowercase and the positive power is capitalized. At this point, I can find no prefixes for negative binary powers, but a similar idea might be used. I would lean toward being less permissive about capitalization.

It's easy enough to fix and makes things easier to read, so I'll fix
those.

> 3) I think it ought to be possible to make CustomPartitioningSpoke._currentTotalSpace() a one liner and might be worth it since
> all the lines get replaced anyway.

Yes, I can do that.

> 4) In general, rather than adding Size(bytes=0) on the left to a bunch of other things, I'ld rather trust to the fact that those other things are Size objects themselves as in the example: disks_size = Size(bytes=0) + sum(d.size for d in disks).

I put that there to ensure the expression evaluates to a Size even if
the sequence passed to sum() is empty. I'm converting to use the
optional start argument to sum instead -- thanks to bcl.

> 
> - mulhern  
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches




More information about the anaconda-patches mailing list