Show multiple bootloader devices on the Manual Partitioning screen #4271
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
poncovka
added
manual testing required
jkonecny12
reviewed
Aug 17, 2022
| mounts = { | ||
| d.format.mountpoint: d for d in devices | ||
| if getattr(d.format, "mountpoint", None) | ||
| } | ||
|
|
||
| for device in devices: | ||
| if device in bootloader_devices: | ||
| mounts[device.format.name] = device |
There was a problem hiding this comment.
I have a bad feeling about removing this part. Don't we really need it?
There was a problem hiding this comment.
Well, this is exactly that code that adds bootloader devices to a dictionary of mount points, so if you have two biosboot devices, you will end up with only one of them. Without this change, the rest of it is pointless.
Copy-paste the `copy` method of the `Blivet` class, so we can later change it. It manipulates with collected installation roots, but these objects are part of Anaconda, not Blivet. Blivet should remove this code. Then we can call `super().copy()` to create a Blivet copy.
Define the `roots` attribute in Anaconda, so Blivet can remove it.
Move the code for creating copies of installation roots to the `Root` class. Simplify the implementation, so it should be easier to change it later.
Don't keep bootloader devices in a dictionary of mount points. Otherwise, we can loose information about bootloader devices of the same type (like biosboot) and show only one of them on the Manual Partitioning screen in GUI. All devices used by an OS installation are now represented by the `devices` property of the `OSData` structure. The list of devices includes bootloader devices, mount point sources and swap devices. The `swap_devices` property was removed. Related: rhbz#2088113 Related: rhbz#2093793 Related: rhbz#1913035
Add unit tests for our redefinition of the Blivet.copy method.
poncovka
force-pushed
the
master-multiple_bootloader_devices
branch
from
345dd9e
to
8b00e62
Compare
|
Unfortunately, Blivet still contains some code that manipulates with our installation roots. The pull request uses some temporary workarounds to deal with that and can be merged without storaged-project/blivet#1069. |
|
/kickstart-test --testtype smoke |
M4rtinK
approved these changes
Aug 22, 2022
poncovka
removed
the
manual testing required
VladimirSlavik
approved these changes
Aug 24, 2022
This was referenced Nov 2, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Don't keep bootloader devices in a dictionary of mount points. Otherwise, we
can loose information about bootloader devices of the same type (like biosboot)
and show only one of them on the Manual Partitioning screen in GUI.
All devices used by an OS installation are now represented by the
devicesproperty of the
OSDatastructure. The list of devices includes bootloaderdevices, mount point sources and swap devices. The
swap_devicespropertywas removed.
Related: rhbz#2088113
Related: rhbz#2093793
Related: rhbz#1913035