Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DONTMERGE: Unmerged code required for the c-storage integration in the anaconda-webui (multiple PRs) #5403

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions pyanaconda/modules/storage/devicetree/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ def is_supported(disk):

return list(filter(is_supported, disks))

def reset(self, cleanup_only=False):
def reset(self, cleanup_only=False, deep=True):
""" Reset storage configuration to reflect actual system state.

This will cancel any queued actions and rescan from scratch but not
Expand All @@ -250,9 +250,16 @@ def reset(self, cleanup_only=False):
:keyword cleanup_only: prepare the tree only to deactivate devices
:type cleanup_only: bool

:keyward deep: perform a deep scan and re-read existing installations

See :meth:`devicetree.Devicetree.populate` for more information
about the cleanup_only keyword argument.
"""
if deep:
log.debug("resetting storage configuration deep")
else:
log.debug("resetting storage configuration plain")

# set up the disk images
if conf.target.is_image:
self.setup_disk_images()
Expand All @@ -266,15 +273,17 @@ def reset(self, cleanup_only=False):

# Protect devices from teardown.
self._mark_protected_devices()
self.devicetree.teardown_all()
if deep:
self.devicetree.teardown_all()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw. we also call teardown_all during the installation, after we unlock a device and after we discover new devices. If I understand it correctly, Anaconda can never tear down any device if Cockpit Storage was used for the partitioning, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that teardown_all should be disabled globally via a flag or something if it is requested. It will be safer, more predictable and cleaner. You can start with using devicetree.teardown_all = lambda: log.debug("Tear down is disabled.") as a temporary workaround for testing.


self.fsset = FSSet(self.devicetree)

# Clear out attributes that refer to devices that are no longer in the tree.
self.bootloader.reset()

self.roots = []
self.roots = find_existing_installations(self.devicetree)
if deep:
self.roots = find_existing_installations(self.devicetree)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I talked to Vojta and we think this can be removed from the reset method. It can be called directly via the FindExistingSystemsWithTask DBus method. We can call it in the reset_storage function by default to handle GUI, TUI and the rescue mode and Web UI can handle this on its own. What do you think?

self.dump_state("initial")

def _mark_protected_devices(self):
Expand Down
5 changes: 3 additions & 2 deletions pyanaconda/modules/storage/reset.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,14 @@ class ScanDevicesTask(Task):
This task will reset the given instance of Blivet.
"""

def __init__(self, storage):
def __init__(self, storage, deep):
"""Create a new task.

:param storage: an instance of Blivet
"""
super().__init__()
self._storage = storage
self._deep = deep

@property
def name(self):
Expand Down Expand Up @@ -81,4 +82,4 @@ def _reload_modules(self):

def _reset_storage(self, storage):
"""Reset the storage."""
storage.reset()
storage.reset(False, self._deep)
9 changes: 7 additions & 2 deletions pyanaconda/modules/storage/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,14 +229,19 @@ def on_protected_devices_changed(self, protected_devices):

self.storage.protect_devices(protected_devices)

def scan_devices_with_task(self):
def scan_devices_with_task(self, deep_scan: bool = True):
"""Scan all devices with a task.

We will reset a copy of the current storage model
and switch the models if the reset is successful.

:return: a task
"""
if not deep_scan:
log.debug("Scanning devices without unmounting and deactivating.")
else:
log.debug("Scanning devices with unmounting and deactivating.")

# Copy the storage.
storage = self.storage.copy()

Expand All @@ -247,7 +252,7 @@ def scan_devices_with_task(self):
storage.disk_images = self._disk_selection_module.disk_images

# Create the task.
task = ScanDevicesTask(storage)
task = ScanDevicesTask(storage, deep_scan)
task.succeeded_signal.connect(lambda: self._set_storage(storage))
return task

Expand Down
4 changes: 2 additions & 2 deletions pyanaconda/modules/storage/storage_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def connect_signals(self):
"AppliedPartitioning", self.implementation.applied_partitioning_changed
)

def ScanDevicesWithTask(self) -> ObjPath:
def ScanDevicesWithTask(self, deep_scan: bool = True) -> ObjPath:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer a new method for this that can be marked as a temporary workaround and eventually dropped.

"""Scan all devices with a task.

Create a model of the current storage. This model will be used
Expand All @@ -50,7 +50,7 @@ def ScanDevicesWithTask(self) -> ObjPath:
:return: a path to a task
"""
return TaskContainer.to_object_path(
self.implementation.scan_devices_with_task()
self.implementation.scan_devices_with_task(deep_scan)
)

@emits_properties_changed
Expand Down
2 changes: 1 addition & 1 deletion pyanaconda/ui/lib/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def reset_storage(scan_all=False, retry=True):

while True:
try:
task_path = storage_proxy.ScanDevicesWithTask()
task_path = storage_proxy.ScanDevicesWithTask(True)
task_proxy = STORAGE.get_proxy(task_path)
sync_run_task(task_proxy)
except DBusError as e:
Expand Down
Loading