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

[David Lehman]

On 07/29/2014 09:23 AM, Vratislav Podzimek wrote:
> 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

Shouldn't this all be using Size? Even if not, it should probably be 
using MiB instead of 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
>
> +def refreshAutoSwapSize(storage):
>
> """
>
> Refresh size of the auto partitioning request for swap device accordingly to
>
> 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)

Looks good aside from the question of using Size and/or MiB -v- MB.

David