Re: [master/rhel7-branch][PATCH] Use blivet's getFreeSpace for limitting automatic swap size

[Vratislav Podzimek]

On Wed, 2014-07-30 at 04:08 -0400, Anne Mulhern wrote:
> ----- Original Message -----
> > From: "Vratislav Podzimek" 
> > To: [e-mail]
> > Sent: Tuesday, July 29, 2014 4:23:04 PM
> > Subject: [master/rhel7-branch][PATCH] Use blivet's getFreeSpace for limitting	automatic swap size
> >
> > We should limit swap size to 10 % of available free space not overall disk
> > space. Blivet's getFreeSpace can help us a lot with this.
> >
> > Resolves: rhbz#1053462
> > Signed-off-by: Vratislav Podzimek
> >
> > pyanaconda/kickstart.py             | 30 ++++++++++++++++++++++--------
> >  pyanaconda/ui/gui/spokes/custom.py  |  3 +++
> >  pyanaconda/ui/gui/spokes/storage.py | 12 +++---------
> >  3 files changed, 28 insertions(+), 17 deletions(-)
> >
> > diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
> > index c8ac223..8a3a942 100644
> > --- a/pyanaconda/kickstart.py
> > +++ b/pyanaconda/kickstart.py
> > @@ -200,8 +200,7 @@ def removeExistingFormat(device, storage):
> >
> > def getAvailableDiskSpace(storage):
> >      """
> > Get overall disk space available on disks we may use (not free space on
> >
> > the
> > disks, but overall space on the disks).
> >
> > Get overall disk space available on disks we may use.
> >
> > :param storage: blivet.Blivet instance
> > :return: overall disk space available in MB
> >
> > @@ -209,13 +208,28 @@ def getAvailableDiskSpace(storage):
> >  """
> >
> > disk_space = 0
> >
> > for disk in storage.disks:
> >    if not storage.config.clearPartDisks or \
> >            disk.name in storage.config.clearPartDisks:
> >        disk_space += disk.size
> >
> > return disk_space
> > free_space = storage.getFreeSpace()
> > if free_space:
> >    return sum(float(disk_free.convertTo("MB")) for disk_free, fs_free
> >
> > in free_space.values())
> > else:
> >    return 0
>
> I believe you can get rid of if condition here entirely, since getFreeSpace() always
> returns a dict, never None.
> > +def refreshAutoSwapSize(storage):
> > """
> > Refresh size of the auto partitioning request for swap device
> >
> > accordingly to
>
> Should be "according".
> > the current state of the storage configuration.
> > :param storage: blivet.Blivet instance
> > """
> > for request in storage.autoPartitionRequests:
> >    if request.fstype == "swap":
> >        disk_space = getAvailableDiskSpace(storage)
> >        request.size = swap_lib.swapSuggestion(disk_space=disk_space)
> >        break
> >
> > ###
> >  ### SUBCLASSES OF PYKICKSTART COMMAND HANDLERS
> > diff --git a/pyanaconda/ui/gui/spokes/custom.py
> > b/pyanaconda/ui/gui/spokes/custom.py
> > index 61506fc..7372e47 100644
> > --- a/pyanaconda/ui/gui/spokes/custom.py
> > +++ b/pyanaconda/ui/gui/spokes/custom.py
> > @@ -82,6 +82,8 @@ from pyanaconda.ui.gui.utils import setViewportBackground,
> > gtk_action_wait, enli
> >          really_hide, really_show
> >  from pyanaconda.ui.gui.categories.system import SystemCategory
> >  from pyanaconda.ui.lib.disks import size_str
> > +from pyanaconda.kickstart import refreshAutoSwapSize
> >
> > # pylint: disable-msg=E0611
> >  from gi.repository import Gdk, Gtk
> > @@ -2647,6 +2649,7 @@ class CustomPartitioningSpoke(NormalSpoke,
> > StorageChecker):
> >          self.clear_errors()
> >          with ui_storage_logger():
> >              try:
> >            refreshAutoSwapSize(self.__storage)
> >            doAutoPartition(self.__storage, self.data)
> >        except NoDisksError as e:
> >            # No handling should be required for this.
> >
> > diff --git a/pyanaconda/ui/gui/spokes/storage.py
> > b/pyanaconda/ui/gui/spokes/storage.py
> > index bdd48ef..db4a5c3 100644
> > --- a/pyanaconda/ui/gui/spokes/storage.py
> > +++ b/pyanaconda/ui/gui/spokes/storage.py
> > @@ -53,14 +53,13 @@ from pyanaconda.ui.gui.spokes.lib.resize import
> > ResizeDialog
> >  from pyanaconda.ui.gui.categories.system import SystemCategory
> >  from pyanaconda.ui.gui.utils import enlightbox, gtk_call_once,
> >  gtk_action_wait
> >
> > -from pyanaconda.kickstart import doKickstartStorage, getAvailableDiskSpace
> > +from pyanaconda.kickstart import doKickstartStorage, refreshAutoSwapSize
> >  from blivet.size import Size
> >  from blivet.devices import MultipathDevice
> >  from blivet.errors import StorageError
> >  from blivet.errors import SanityError
> >  from blivet.errors import SanityWarning
> >  from blivet.platform import platform
> > -from blivet.devicelibs import swap as swap_lib
> >  from pyanaconda.threads import threadMgr, AnacondaThread
> >  from pyanaconda.product import productName
> >  from pyanaconda.flags import flags
> > @@ -340,13 +339,6 @@ class StorageSpoke(NormalSpoke, StorageChecker):
> >          # user may have set up before now.
> >          self.storage.config.clearNonExistent = self.data.autopart.autopart
> >    # refresh the autopart swap size suggestion with currently selected
> >
> > disks
> >    for request in self.storage.autoPartitionRequests:
> >        if request.fstype == "swap":
> >            disk_space = getAvailableDiskSpace(self.storage)
> >            request.size =
> >
> > swap_lib.swapSuggestion(disk_space=disk_space)
> >            break
> > def execute(self):
> >        # Spawn storage execution as a separate thread so there's no big
> >        delay
> >        # going back from this spoke to the hub while StorageChecker.run
> >        runs.
> >
> > @@ -872,6 +864,8 @@ class StorageSpoke(NormalSpoke, StorageChecker):
> >              # catch-all.  Just stay on this spoke.
> >              return
> >    if self.autopart:
> >        refreshAutoSwapSize(self.storage)
> >    self.applyOnSkip = True
> >    NormalSpoke.on_back_clicked(self, button)
> >
> > --
> > 1.9.3
> >
> > anaconda-patches mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
> Just a few inline comments.

Good catches, thanks! Fixing locally.