DONTMERGE: Unmerged code required for the c-storage integration in the anaconda-webui (multiple PRs) #5403
Conversation
KKoukiou
temporarily deployed
to
gh-cockpituous
GitHub Actions
Inactive
| and root.format.exists | ||
| and root.format.mountable | ||
| and root.format.mountpoint == "/" | ||
| and (root.format.free != root.format.total)): |
There was a problem hiding this comment.
I am not sure this will work, even newly created filesystems are not "empty", for example ext4 reserves 5 % of free space on the filesystem by default so filesystem free space will always be less than filesystem size. This is a freshly created 1 GiB Ext4 with just 973.14 MiB of free blocks:
# dumpe2fs /dev/sdd2
...
Block count: 262144
Reserved block count: 13107
Overhead clusters: 12949
Free blocks: 249125
Free inodes: 65524
First block: 0
Block size: 4096
...
There was a problem hiding this comment.
I see, yes I just wanted to try it out and pushed a PR instead of building locally.
KKoukiou
changed the title
KKoukiou
temporarily deployed
to
gh-cockpituous
GitHub Actions
Inactive
KKoukiou
force-pushed
the
check-empty-root-force-reformat
branch
from
e8d614d
to
9050f22
Compare
KKoukiou
had a problem deploying
to
gh-cockpituous
GitHub Actions
Failure
KKoukiou
force-pushed
the
check-empty-root-force-reformat
branch
from
9050f22
to
41c48a3
Compare
KKoukiou
temporarily deployed
to
gh-cockpituous
GitHub Actions
Inactive
KKoukiou
changed the title
KKoukiou
changed the title
KKoukiou
force-pushed
the
check-empty-root-force-reformat
branch
from
41c48a3
to
8d82a71
Compare
KKoukiou
temporarily deployed
to
gh-cockpituous
GitHub Actions
Inactive
…e the world This will make the integration with cockpit-storage more seemless and less hacky.
KKoukiou
temporarily deployed
to
gh-cockpituous
GitHub Actions
Inactive
| @@ -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: | |||
There was a problem hiding this comment.
I would prefer a new method for this that can be marked as a temporary workaround and eventually dropped.
|
|
||
| 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) |
There was a problem hiding this comment.
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?
| @@ -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() | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
No description provided.