If there is new root added in the custom partitioning, we automatically add all existing swaps under it. However, since the existing swaps are from different roots, they cannot be removed. This confuses users.
I believe that adding all swaps under the newly created root confuses users in general. So these three anaconda patches and one related blivet patch change the behaviour to show only the newly created or reformated swaps under the new root and also write only those swaps to the fstab.
Vratislav Podzimek (3): Make code to get new devices reusable as property Put only newly created or reformated swaps to the new root Tell blivet which swaps should appear in the fstab (#1011391)
pyanaconda/kickstart.py | 13 ++++++++++ pyanaconda/ui/gui/spokes/custom.py | 53 ++++++++++++++++++-------------------- 2 files changed, 38 insertions(+), 28 deletions(-)
Related: rhbz#1011391
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- pyanaconda/ui/gui/spokes/custom.py | 41 ++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index 2d784d1..170f0b2 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -897,20 +897,8 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self._current_selector.set_chosen(False) self._current_selector = None
- def _do_refresh(self, mountpointToShow=None): - # block mountpoint selector signal handler for now - self._initialized = False - self._clear_current_selector() - - # Make sure we start with a clean slate. - self._accordion.removeAllPages() - - # Start with buttons disabled, since nothing is selected. - self._removeButton.set_sensitive(False) - self._configButton.set_sensitive(False) - - # Now it's time to populate the accordion. - + @property + def new_devices(self): # A device scheduled for formatting only belongs in the new root. new_devices = [d for d in self._devices if d.isleaf and not d.format.exists and @@ -927,15 +915,30 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker):
new_devices = list(set(new_devices))
+ return new_devices + + def _do_refresh(self, mountpointToShow=None): + # block mountpoint selector signal handler for now + self._initialized = False + self._clear_current_selector() + + # Make sure we start with a clean slate. + self._accordion.removeAllPages() + + # Start with buttons disabled, since nothing is selected. + self._removeButton.set_sensitive(False) + self._configButton.set_sensitive(False) + + # Now it's time to populate the accordion. log.debug("ui: devices=%s", [d.name for d in self._devices]) log.debug("ui: unused=%s", [d.name for d in self.unusedDevices]) - log.debug("ui: new_devices=%s", [d.name for d in new_devices]) + log.debug("ui: new_devices=%s", [d.name for d in self.new_devices])
ui_roots = self.__storage.roots[:]
# If we've not yet run autopart, add an instance of CreateNewPage. This # ensures it's only added once. - if not new_devices: + if not self.new_devices: page = CreateNewPage(self.translated_new_install_name, self.on_create_clicked, partitionsToReuse=bool(ui_roots)) @@ -946,11 +949,11 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): _("When you create mount points for your %(name)s %(version)s installation, you'll be able to view their details here.") %\ {"name" : productName, "version" : productVersion}) else: - swaps = [d for d in new_devices if d.format.type == "swap"] - mounts = dict((d.format.mountpoint, d) for d in new_devices + swaps = [d for d in self.new_devices if d.format.type == "swap"] + mounts = dict((d.format.mountpoint, d) for d in self.new_devices if getattr(d.format, "mountpoint", None))
- for device in new_devices: + for device in self.new_devices: if device in self.bootLoaderDevices: mounts[device.format.type] = device
Related: rhbz#1011391
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- pyanaconda/ui/gui/spokes/custom.py | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index 170f0b2..16b73ba 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -790,11 +790,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): return unused_devices
@property - def existingSwaps(self): - return [d for d in self._devices - if d.format.type == "swap" and d.format.exists] - - @property def bootLoaderDevices(self): devices = [] format_types = ["biosboot", "prepboot"] @@ -910,7 +905,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): new_mounts = [d for d in self.__storage.mountpoints.values() if d.exists] if new_mounts or new_devices: new_devices.extend(self.__storage.mountpoints.values()) - new_devices.extend(self.existingSwaps) new_devices.extend(self.bootLoaderDevices)
new_devices = list(set(new_devices)) @@ -1049,9 +1043,6 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): page = Page(self.translated_new_install_name) expander.add(page)
- # pull in all the existing swap devices - devices.extend(self.existingSwaps) - # also pull in biosboot and prepboot that are on our boot disk devices.extend(self.bootLoaderDevices)
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- pyanaconda/kickstart.py | 13 +++++++++++++ pyanaconda/ui/gui/spokes/custom.py | 3 +++ 2 files changed, 16 insertions(+)
diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py index 12797a6..b376728 100644 --- a/pyanaconda/kickstart.py +++ b/pyanaconda/kickstart.py @@ -353,6 +353,9 @@ class BTRFSData(commands.btrfs.F17_BTRFSData):
if not device: raise KickstartValueError(formatErrorMsg(self.lineno, msg="Specified nonexistent BTRFS volume %s in btrfs command" % self.name)) + + if ty == "swap": + storage.add_fstab_swap(device.format) else: # If a previous device has claimed this mount point, delete the # old one. @@ -371,6 +374,8 @@ class BTRFSData(commands.btrfs.F17_BTRFSData): parents=members)
storage.createDevice(request) + if ty == "swap": + storage.add_fstab_swap(request.format)
class Realm(commands.realm.F19_Realm): @@ -756,6 +761,8 @@ class LogVolData(commands.logvol.F20_LogVolData): msg="Invalid target size (%d) for device %s" % (self.size, device.name)))
devicetree.registerAction(ActionCreateFormat(device, fmt)) + if ty == "swap": + storage.add_fstab_swap(device.format) else: # If a previous device has claimed this mount point, delete the # old one. @@ -789,6 +796,8 @@ class LogVolData(commands.logvol.F20_LogVolData): **pool_args)
storage.createDevice(request) + if ty == "swap": + storage.add_fstab_swap(request.format)
if self.encrypted: if self.passphrase and not storage.encryptionPassphrase: @@ -1026,6 +1035,8 @@ class PartitionData(commands.partition.F18_PartData): msg="Invalid target size (%d) for device %s" % (self.size, device.name)))
devicetree.registerAction(ActionCreateFormat(device, kwargs["format"])) + if ty == "swap": + storage.add_fstab_swap(device.format) else: # If a previous device has claimed this mount point, delete the # old one. @@ -1038,6 +1049,8 @@ class PartitionData(commands.partition.F18_PartData):
request = storage.newPartition(**kwargs) storage.createDevice(request) + if ty == "swap": + storage.add_fstab_swap(request.format)
if self.encrypted: if self.passphrase and not storage.encryptionPassphrase: diff --git a/pyanaconda/ui/gui/spokes/custom.py b/pyanaconda/ui/gui/spokes/custom.py index 16b73ba..710f33b 100644 --- a/pyanaconda/ui/gui/spokes/custom.py +++ b/pyanaconda/ui/gui/spokes/custom.py @@ -666,6 +666,9 @@ class CustomPartitioningSpoke(NormalSpoke, StorageChecker): self.storage.devicetree.names = self.__storage.devicetree.names self.storage.roots = self.__storage.roots
+ new_swaps = (dev.format for dev in self.new_devices if dev.format.type == "swap") + self.storage.set_fstab_swaps(new_swaps) + # update the global passphrase self.data.autopart.passphrase = self.passphrase
Not all swaps should be used by the system in some cases.
Related: rhbz#1011391
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- blivet/__init__.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ blivet/partitioning.py | 4 ++++ 2 files changed, 60 insertions(+)
diff --git a/blivet/__init__.py b/blivet/__init__.py index 8f6e82a..2c39552 100644 --- a/blivet/__init__.py +++ b/blivet/__init__.py @@ -2120,6 +2120,30 @@ class Blivet(object): parent = getattr(self.ksdata, list_attr) parent.dataList().append(data)
+ def add_fstab_swap(self, fmt): + """ + Add swap format to the list of swaps that should appear in the fstab. + + """ + + self.fsset.add_fstab_swap(fmt) + + def remove_fstab_swap(self, fmt): + """ + Remove swap format from the list of swaps that should appear in the fstab. + + """ + + self.fsset.remove_fstab_swap(fmt) + + def set_fstab_swaps(self, fmts): + """ + Set swap formats that should appear in the fstab. + + """ + + self.fsset.set_fstab_swaps(fmts) + def mountExistingSystem(fsset, rootDevice, allowDirty=None, dirtyCB=None, readOnly=None): @@ -2328,6 +2352,7 @@ class FSSet(object): self._usb = None self._selinux = None self._run = None + self._fstab_lines = set() self.preserveLines = [] # lines we just ignore and preserve
@property @@ -2855,6 +2880,10 @@ class FSSet(object): if fstype == "swap": mountpoint = "swap" options = device.format.options + if device.format not in self._fstab_swaps: + # swap that shouldn't appear in fstab + # XXX: do this only in the installer mode? + continue else: mountpoint = device.format.mountpoint options = device.format.options @@ -2890,6 +2919,33 @@ class FSSet(object):
return fstab
+ def add_fstab_swap(self, fmt): + """ + Add swap format to the list of swaps that should appear in the fstab. + + """ + + self._fstab_swaps.add(fmt) + + def remove_fstab_swap(self, fmt): + """ + Remove swap format from the list of swaps that should appear in the fstab. + + """ + + try: + self._fstab_swaps.remove(fmt) + except KeyError: + pass + + def set_fstab_swaps(self, fmts): + """ + Set swap formats that should appear in the fstab. + + """ + + self._fstab_swaps = set(fmts) + def getReleaseString(): relName = None relVer = None diff --git a/blivet/partitioning.py b/blivet/partitioning.py index abaf1b3..338a31a 100644 --- a/blivet/partitioning.py +++ b/blivet/partitioning.py @@ -325,6 +325,10 @@ def doAutoPartition(storage, data):
storage.setUpBootLoader()
+ new_swaps = (action.format for action in storage.devicetree.findActions(type="create") + if action.format.type == "swap") + storage.set_fstab_swaps(new_swaps) + # now do a full check of the requests (errors, warnings) = storage.sanityCheck() for error in errors:
On Wed, 2013-10-02 at 15:21 +0200, Vratislav Podzimek wrote:
Not all swaps should be used by the system in some cases.
Related: rhbz#1011391
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com
blivet/__init__.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ blivet/partitioning.py | 4 ++++ 2 files changed, 60 insertions(+)
diff --git a/blivet/__init__.py b/blivet/__init__.py index 8f6e82a..2c39552 100644 --- a/blivet/__init__.py +++ b/blivet/__init__.py @@ -2120,6 +2120,30 @@ class Blivet(object): parent = getattr(self.ksdata, list_attr) parent.dataList().append(data)
- def add_fstab_swap(self, fmt):
"""Add swap format to the list of swaps that should appear in the fstab."""self.fsset.add_fstab_swap(fmt)- def remove_fstab_swap(self, fmt):
"""Remove swap format from the list of swaps that should appear in the fstab."""self.fsset.remove_fstab_swap(fmt)- def set_fstab_swaps(self, fmts):
"""Set swap formats that should appear in the fstab."""self.fsset.set_fstab_swaps(fmts)def mountExistingSystem(fsset, rootDevice, allowDirty=None, dirtyCB=None, readOnly=None): @@ -2328,6 +2352,7 @@ class FSSet(object): self._usb = None self._selinux = None self._run = None
self._fstab_lines = set()
This should be fstab_swaps, of course. Fixed locally in the version I'm testing (forgot to ammend it).
On Wed, 2013-10-02 at 15:21 +0200, Vratislav Podzimek wrote:
Not all swaps should be used by the system in some cases.
I'd rather if you specified the swaps as devices instead of formats. Also, it would be nice if you could conform the new methods to the (admittedly ugly) style of the surrounding code.
David
Related: rhbz#1011391
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com
blivet/__init__.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ blivet/partitioning.py | 4 ++++ 2 files changed, 60 insertions(+)
diff --git a/blivet/__init__.py b/blivet/__init__.py index 8f6e82a..2c39552 100644 --- a/blivet/__init__.py +++ b/blivet/__init__.py @@ -2120,6 +2120,30 @@ class Blivet(object): parent = getattr(self.ksdata, list_attr) parent.dataList().append(data)
- def add_fstab_swap(self, fmt):
"""Add swap format to the list of swaps that should appear in the fstab."""self.fsset.add_fstab_swap(fmt)- def remove_fstab_swap(self, fmt):
"""Remove swap format from the list of swaps that should appear in the fstab."""self.fsset.remove_fstab_swap(fmt)- def set_fstab_swaps(self, fmts):
"""Set swap formats that should appear in the fstab."""self.fsset.set_fstab_swaps(fmts)def mountExistingSystem(fsset, rootDevice, allowDirty=None, dirtyCB=None, readOnly=None): @@ -2328,6 +2352,7 @@ class FSSet(object): self._usb = None self._selinux = None self._run = None
self._fstab_lines = set() self.preserveLines = [] # lines we just ignore and preserve@property
@@ -2855,6 +2880,10 @@ class FSSet(object): if fstype == "swap": mountpoint = "swap" options = device.format.options
if device.format not in self._fstab_swaps:# swap that shouldn't appear in fstab# XXX: do this only in the installer mode?continue else: mountpoint = device.format.mountpoint options = device.format.options@@ -2890,6 +2919,33 @@ class FSSet(object):
return fstab
- def add_fstab_swap(self, fmt):
"""Add swap format to the list of swaps that should appear in the fstab."""self._fstab_swaps.add(fmt)- def remove_fstab_swap(self, fmt):
"""Remove swap format from the list of swaps that should appear in the fstab."""try:self._fstab_swaps.remove(fmt)except KeyError:pass- def set_fstab_swaps(self, fmts):
"""Set swap formats that should appear in the fstab."""self._fstab_swaps = set(fmts)def getReleaseString(): relName = None relVer = None diff --git a/blivet/partitioning.py b/blivet/partitioning.py index abaf1b3..338a31a 100644 --- a/blivet/partitioning.py +++ b/blivet/partitioning.py @@ -325,6 +325,10 @@ def doAutoPartition(storage, data):
storage.setUpBootLoader()
- new_swaps = (action.format for action in storage.devicetree.findActions(type="create")
if action.format.type == "swap")- storage.set_fstab_swaps(new_swaps)
- # now do a full check of the requests (errors, warnings) = storage.sanityCheck() for error in errors:
On Wed, 2013-10-02 at 08:31 -0500, David Lehman wrote:
On Wed, 2013-10-02 at 15:21 +0200, Vratislav Podzimek wrote:
Not all swaps should be used by the system in some cases.
I'd rather if you specified the swaps as devices instead of formats.
That was my first approach as well, but there was a problem with autoparititioning that doesn't have devices in the end. Or can I get them somehow from the results of findActions?
Also, it would be nice if you could conform the new methods to the (admittedly ugly) style of the surrounding code.
No problem. :)
On Wed, 2013-10-02 at 15:35 +0200, Vratislav Podzimek wrote:
On Wed, 2013-10-02 at 08:31 -0500, David Lehman wrote:
On Wed, 2013-10-02 at 15:21 +0200, Vratislav Podzimek wrote:
Not all swaps should be used by the system in some cases.
I'd rather if you specified the swaps as devices instead of formats.
That was my first approach as well, but there was a problem with autoparititioning that doesn't have devices in the end. Or can I get them somehow from the results of findActions?
The devices should be in the tree after autopart. It just has to be at the end of doAutoPartition or later. findActions would work, but shouldn't be necessary -- I'd rather solve the problem you were seeing than resort to findActions for this purpose.
Also, it would be nice if you could conform the new methods to the (admittedly ugly) style of the surrounding code.
No problem. :)
anaconda-patches@lists.fedorahosted.org
-
David Lehman -
Vratislav Podzimek