-
Notifications
You must be signed in to change notification settings - Fork 355
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
Make boot not required 2 #5381
Make boot not required 2 #5381
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,7 @@ | |
from pyanaconda.core.i18n import _ | ||
from pyanaconda.modules.common.errors.storage import UnknownDeviceError | ||
from pyanaconda.modules.common.structures.storage import DeviceData, DeviceActionData, \ | ||
DeviceFormatData, OSData, RequiredMountPointData | ||
DeviceFormatData, OSData, MountPointConstraintsData | ||
from pyanaconda.modules.storage.devicetree.utils import get_required_device_size, \ | ||
get_supported_filesystems | ||
from pyanaconda.modules.storage.platform import platform | ||
|
@@ -462,13 +462,61 @@ def _get_os_data(self, root): | |
} | ||
return data | ||
|
||
def _get_mount_point_constraints_data(self, spec): | ||
"""Get the mount point data. | ||
|
||
:param spec: an instance of PartSpec | ||
:return: an instance of MountPointConstraintsData | ||
""" | ||
data = MountPointConstraintsData() | ||
data.mount_point = spec.mountpoint or "" | ||
data.required_filesystem_type = spec.fstype or "" | ||
data.encryption_allowed = spec.encrypted | ||
data.logical_volume_allowed = spec.lv | ||
|
||
return data | ||
|
||
def get_mount_point_constraints(self): | ||
"""Get list of constraints on mountpoints for the current platform | ||
|
||
Also provides hints if the partition is required or recommended. | ||
|
||
This includes mount points required to boot (e.g. /boot/efi, /boot) | ||
and the / partition which is always considered to be required. | ||
|
||
/boot is not required in general but can be required in some cases, | ||
depending on the filesystem on the root partition (ie crypted root). | ||
|
||
:return: a list of mount points with its constraints | ||
""" | ||
|
||
constraints = [] | ||
|
||
# Root partition is required | ||
root_partition = PartSpec(mountpoint="/", lv=True, thin=True, encrypted=True) | ||
root_constraint = self._get_mount_point_constraints_data(root_partition) | ||
root_constraint.required = True | ||
constraints.append(root_constraint) | ||
|
||
# Platform partitions are required except for /boot partiotion which is recommended | ||
for p in platform.partitions: | ||
if p.mountpoint: | ||
constraint = self._get_mount_point_constraints_data(p) | ||
if p.mountpoint == "/boot": | ||
constraint.recommended = True | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this shouldn't be actually defined in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes sense to me, but I hesitated to use configuration files perhaps because of the outlook of more dynamic API (like /boot being required depending on fs on /). Nevertheless the static information (/boot (a mount point) required or recommended in general) can still be in the configuration file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something like
or even (not sure if it makes sense in this form) ?
Well, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @poncovka would you be OK with handling it as a follow-up? We are dealing with test flakiness on webui part and it could be handy to have this merged. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you want an issue about considering this followup? If this can be handled this year, I would not create one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, it is definitely just a suggestion. I didn't want to block this pull request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, please create the issue. |
||
else: | ||
constraint.required = True | ||
constraints.append(constraint) | ||
|
||
return constraints | ||
|
||
def _get_platform_mount_point_data(self, spec): | ||
"""Get the mount point data. | ||
|
||
:param spec: an instance of PartSpec | ||
:return: an instance of RequiredMountPointData | ||
:return: an instance of MountPointConstraintsData | ||
""" | ||
data = RequiredMountPointData() | ||
data = MountPointConstraintsData() | ||
data.mount_point = spec.mountpoint or "" | ||
data.required_filesystem_type = spec.fstype or "" | ||
data.encryption_allowed = spec.encrypted | ||
|
@@ -482,7 +530,7 @@ def get_required_mount_points(self): | |
This includes mount points required to boot (e.g. /boot and /boot/efi) | ||
and the / partition which is always considered to be required. | ||
|
||
:return: a list of mount points | ||
:return: a list of mount points with its constraints | ||
""" | ||
root_partition = PartSpec(mountpoint="/", lv=True, thin=True, encrypted=True) | ||
ret = list(map(self._get_platform_mount_point_data, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the new name!