From: Vratislav Podzimek vpodzime@redhat.com
Reverting changes in storage is really complicated and trying to do so lead us to many hard bugs like the one referenced above. So instead of playing this tricky game, let's just create a snapshot of unmodified on-disk storage and revert everything to it whenever we need to start over.
Big THANKS! goes to Vojtech Trefny vtrefny@redhat.com for his help with this patch.
(cherry picked from commit 0f267be6966374fd247d705bee003d1f7eca18fe)
Conflicts: pyanaconda/ui/gui/spokes/storage.py --- pyanaconda/storage_utils.py | 3 ++ pyanaconda/ui/gui/spokes/storage.py | 62 +++++++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 17 deletions(-)
diff --git a/pyanaconda/storage_utils.py b/pyanaconda/storage_utils.py index 7289bc5..b3774ec 100644 --- a/pyanaconda/storage_utils.py +++ b/pyanaconda/storage_utils.py @@ -444,3 +444,6 @@ def reset_to_snapshot(self, storage, dispose=False): if dispose: self.dispose_snapshot()
+# a snapshot of early storage as we got it from scanning disks without doing any +# changes +on_disk_storage = StorageSnapshot() diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py index d77e9d3..1d652e7 100644 --- a/pyanaconda/ui/gui/spokes/storage.py +++ b/pyanaconda/ui/gui/spokes/storage.py @@ -74,6 +74,7 @@ from pyanaconda.i18n import _, C_, CN_, P_ from pyanaconda import constants, iutil, isys from pyanaconda.bootloader import BootLoaderError +from pyanaconda.storage_utils import on_disk_storage
from pykickstart.constants import CLEARPART_TYPE_NONE, AUTOPART_TYPE_LVM from pykickstart.errors import KickstartValueError @@ -264,6 +265,7 @@ def __init__(self, *args, **kwargs): self.encrypted = False self.passphrase = "" self.selected_disks = self.data.ignoredisk.onlyuse[:] + self._last_selected_disks = None self._back_clicked = False self.autopart_missing_passphrase = False
@@ -484,15 +486,19 @@ def _on_disk_focus_in(self, overview, event): self._cur_clicked_overview = overview
def refresh(self): - self.disks = getDisks(self.storage.devicetree) - self._back_clicked = False
+ self.disks = getDisks(self.storage.devicetree) + # synchronize our local data store with the global ksdata disk_names = [d.name for d in self.disks] - # don't put disks with hidden formats in selected_disks self.selected_disks = [d for d in self.data.ignoredisk.onlyuse - if d in disk_names] + if d in disk_names] + + # unhide previously hidden disks so that they don't look like being + # empty (because of all child devices hidden) + self._unhide_disks() + self.autopart = self.data.autopart.autopart self.autoPartType = self.data.autopart.type if self.autoPartType is None: @@ -762,14 +768,18 @@ def _remove_nonexistant_partitions(self): partition in self.storage.partitions: self.storage.recursiveRemove(partition)
- def _hide_unhide_disks(self): + def _hide_disks(self): for disk in self.disks: if disk.name not in self.selected_disks and \ disk in self.storage.devices: self.storage.devicetree.hide(disk) - elif disk.name in self.selected_disks and \ - disk not in self.storage.devices: - self.storage.devicetree.unhide(disk) + + def _unhide_disks(self): + if self._last_selected_disks: + for disk in self.disks: + if disk.name not in self.selected_disks and \ + disk.name not in self._last_selected_disks: + self.storage.devicetree.unhide(disk)
def _check_dasd_formats(self): rc = DASD_FORMAT_NO_CHANGE @@ -853,25 +863,42 @@ def on_back_clicked(self, button): else: self._back_clicked = True
+ # make sure the snapshot of unmodified on-disk-storage model is created + if not on_disk_storage.created: + on_disk_storage.create_snapshot(self.storage) + if self.autopart_missing_passphrase: self._setup_passphrase() NormalSpoke.on_back_clicked(self, button) return
- disks = [d for d in self.disks if d.name in self.selected_disks] # No disks selected? The user wants to back out of the storage spoke. - if not disks: + if not self.selected_disks: NormalSpoke.on_back_clicked(self, button) return
- # Remove all non-existing devices if autopart was active when we last - # refreshed. - if self._previous_autopart: - self._previous_autopart = False - self._remove_nonexistant_partitions() + disk_selection_changed = False + if self._last_selected_disks: + disk_selection_changed = (self._last_selected_disks != set(self.selected_disks))
- # hide/unhide disks as requested - self._hide_unhide_disks() + # remember the disk selection for future decisions + self._last_selected_disks = set(self.selected_disks) + + if disk_selection_changed: + # Changing disk selection is really, really complicated and has + # always been causing numerous hard bugs. Let's not play the hero + # game and just revert everything and start over again. + on_disk_storage.reset_to_snapshot(self.storage) + self.disks = getDisks(self.storage.devicetree) + else: + # Remove all non-existing devices if autopart was active when we last + # refreshed. + if self._previous_autopart: + self._previous_autopart = False + self._remove_nonexistant_partitions() + + # hide disks as requested + self._hide_disks()
if arch.isS390(): # check for unformatted DASDs and launch dasdfmt if any discovered @@ -906,6 +933,7 @@ def on_back_clicked(self, button): # 3) we are just asked to do autopart => check free space and see if we need # user to do anything more self.autopart = not self._customPart.get_active() + disks = [d for d in self.disks if d.name in self.selected_disks] dialog = None if not self.autopart: self.skipTo = "CustomPartitioningSpoke"