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 vpodzime@redhat.com --- 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 + +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)
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 vpodzime@redhat.com
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.sizereturn 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 disksfor request in self.storage.autoPartitionRequests:if request.fstype == "swap":disk_space = getAvailableDiskSpace(self.storage)request.size = swap_lib.swapSuggestion(disk_space=disk_space)breakdef 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
On Tue, 2014-07-29 at 11:26 -0500, David Lehman wrote:
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 vpodzime@redhat.com
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.
Even on the rhel7-branch? To be honest, I've just resent a patch that I've sent months ago and which stayed unnoticed just rebasing it to the current rhel7-branch's HEAD without looking at it much. 'MiB' is definitely a right thing to do I will fix it locally. But I'm not aware of current state of the "everything to Size" changes in the rhel7-branch. Any update?
On Wed, Jul 30, 2014 at 08:54:21AM +0200, Vratislav Podzimek wrote:
On Tue, 2014-07-29 at 11:26 -0500, David Lehman wrote:
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 vpodzime@redhat.com
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.
Even on the rhel7-branch? To be honest, I've just resent a patch that I've sent months ago and which stayed unnoticed just rebasing it to the current rhel7-branch's HEAD without looking at it much. 'MiB' is definitely a right thing to do I will fix it locally. But I'm not aware of current state of the "everything to Size" changes in the rhel7-branch. Any update?
getFreeSpace uses Size in the dict and swapSuggestion expects an int so we should stick with using Size for this and convert to int when necessary. But certainly not float.
----- Original Message -----
From: "Vratislav Podzimek" vpodzime@redhat.com To: anaconda-patches@lists.fedorahosted.org 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 vpodzime@redhat.com
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.sizereturn disk_space
- free_space = storage.getFreeSpace()
- if free_space:
return sum(float(disk_free.convertTo("MB")) for disk_free, fs_freein 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 selecteddisks
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 anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
Just a few inline comments.
- mulhern
On Wed, 2014-07-30 at 04:08 -0400, Anne Mulhern wrote:
----- Original Message -----
From: "Vratislav Podzimek" vpodzime@redhat.com To: anaconda-patches@lists.fedorahosted.org 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 vpodzime@redhat.com
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.sizereturn disk_space
- free_space = storage.getFreeSpace()
- if free_space:
return sum(float(disk_free.convertTo("MB")) for disk_free, fs_freein free_space.values())
- else:
return 0I 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 selecteddisks
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 anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
Just a few inline comments.
Good catches, thanks! Fixing locally.
anaconda-patches@lists.fedorahosted.org