Merge pull request #5121 from vojtechtrefny/master_boot-efi-backend

Get list of required mount points for mount point assignment from backend

pyanaconda/modules/common/structures/storage.py

@@ -453,3 +453,63 @@ def get_root_device(self):
         :return: a device name or None
         """
         return self.mount_points.get("/")
+
+
+class RequiredMountPointData(DBusData):
+    """Constrains (filesystem and device types allowed) for mount points required
+       for the installed system
+    """
+
+    def __init__(self):
+        self._mount_point = ""
+        self._required_filesystem_type = ""
+        self._encryption_allowed = False
+        self._logical_volume_allowed = False
+
+    @property
+    def mount_point(self) -> Str:
+        """Mount point value, e.g. /boot/efi
+
+        :return: a string with mount point
+        """
+        return self._mount_point
+
+    @mount_point.setter
+    def mount_point(self, mount_point: Str):
+        self._mount_point = mount_point
+
+    @property
+    def required_filesystem_type(self) -> Str:
+        """Filesystem type required for mount point
+
+        :return: a string with filesystem type required for this mount point
+        """
+        return self._required_filesystem_type
+
+    @required_filesystem_type.setter
+    def required_filesystem_type(self, required_filesystem_type: Str):
+        self._required_filesystem_type = required_filesystem_type
+
+    @property
+    def encryption_allowed(self) -> Bool:
+        """Whether this mount point can be encrypted or not
+
+        :return: bool
+        """
+        return self._encryption_allowed
+
+    @encryption_allowed.setter
+    def encryption_allowed(self, encryption_allowed: Bool):
+        self._encryption_allowed = encryption_allowed
+
+    @property
+    def logical_volume_allowed(self) -> Bool:
+        """Whether this mount point can be a LVM logical volume or not
+
+        :return: bool
+        """
+        return self._logical_volume_allowed
+
+    @logical_volume_allowed.setter
+    def logical_volume_allowed(self, logical_volume_allowed: Bool):
+        self._logical_volume_allowed = logical_volume_allowed

pyanaconda/modules/storage/devicetree/viewer.py

@@ -26,9 +26,11 @@
 from pyanaconda.core.i18n import _
 from pyanaconda.modules.common.errors.storage import UnknownDeviceError
 from pyanaconda.modules.common.structures.storage import DeviceData, DeviceActionData, \
-    DeviceFormatData, OSData
+    DeviceFormatData, OSData, RequiredMountPointData
 from pyanaconda.modules.storage.devicetree.utils import get_required_device_size, \
     get_supported_filesystems
+from pyanaconda.modules.storage.platform import platform
+from pyanaconda.modules.storage.partitioning.specification import PartSpec
 
 log = get_module_logger(__name__)
 
@@ -467,3 +469,30 @@ def _get_os_data(self, root):
             path: device.name for path, device in root.mounts.items()
         }
         return data
+
+    def _get_platform_mount_point_data(self, spec):
+        """Get the mount point data.
+
+        :param spec: an instance of PartSpec
+        :return: an instance of RequiredMountPointData
+        """
+        data = RequiredMountPointData()
+        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_required_mount_points(self):
+        """Get list of required mount points for the current platform
+
+        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
+        """
+        root_partition = PartSpec(mountpoint="/", lv=True, thin=True, encrypted=True)
+        ret = list(map(self._get_platform_mount_point_data,
+                       [p for p in platform.partitions if p.mountpoint] + [root_partition]))
+        return ret

pyanaconda/modules/storage/devicetree/viewer_interface.py

@@ -22,7 +22,7 @@
 from dasbus.typing import *  # pylint: disable=wildcard-import
 from pyanaconda.modules.common.constants.interfaces import DEVICE_TREE_VIEWER
 from pyanaconda.modules.common.structures.storage import DeviceData, DeviceActionData, \
-    DeviceFormatData, OSData
+    DeviceFormatData, OSData, RequiredMountPointData
 
 __all__ = ["DeviceTreeViewerInterface"]
 
@@ -192,3 +192,14 @@ def GetExistingSystems(self) -> List[Structure]:
         :return: a list of data about found installations
         """
         return OSData.to_structure_list(self.implementation.get_existing_systems())
+
+    def GetRequiredMountPoints(self) -> List[Structure]:
+        """Get list of required mount points for the current platform
+
+        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 RequiredMountPointData.to_structure_list(
+            self.implementation.get_required_mount_points())

tests/unit_tests/pyanaconda_tests/modules/storage/test_module_device_tree.py

@@ -36,7 +36,7 @@
 from dasbus.typing import *  # pylint: disable=wildcard-import
 from pyanaconda.core.kernel import KernelArguments
 from pyanaconda.modules.common.errors.storage import UnknownDeviceError, MountFilesystemError
-from pyanaconda.modules.common.structures.storage import DeviceFormatData
+from pyanaconda.modules.common.structures.storage import DeviceFormatData, RequiredMountPointData
 from pyanaconda.modules.storage.devicetree import DeviceTreeModule, create_storage, utils
 from pyanaconda.modules.storage.devicetree.devicetree_interface import DeviceTreeInterface
 from pyanaconda.modules.storage.devicetree.populate import FindDevicesTask
@@ -849,6 +849,25 @@ def test_set_device_mount_options(self):
         self.interface.SetDeviceMountOptions("dev1", "")
         assert dev1.format.options == "defaults"
 
+    def test_get_required_mount_points(self):
+        """Test GetRequiredMountPoints."""
+        result = self.interface.GetRequiredMountPoints()
+        assert isinstance(result, list)
+        assert len(result) != 0
+
+        result = RequiredMountPointData.from_structure_list(self.interface.GetRequiredMountPoints())
+        for mp in result:
+            assert mp.mount_point is not None
+            assert mp.required_filesystem_type is not None
+
+        # we are always adding / so it's a good candidate for testing
+        root = next(r for r in result if r.mount_point == "/")
+        assert root is not None
+        assert root.encryption_allowed is True
+        assert root.logical_volume_allowed is True
+        assert root.mount_point == "/"
+        assert root.required_filesystem_type == ""
+
 
 class DeviceTreeTasksTestCase(unittest.TestCase):
     """Test the storage tasks."""

ui/webui/src/apis/storage.js

@@ -153,6 +153,18 @@ export const getRequiredDeviceSize = ({ requiredSpace }) => {
             .then(res => res[0]);
 };
 
+/**
+ * @returns {Promise}           List of all mount points required on the platform
+ */
+export const getRequiredMountPoints = () => {
+    return new StorageClient().client.call(
+        "/org/fedoraproject/Anaconda/Modules/Storage/DeviceTree",
+        "org.fedoraproject.Anaconda.Modules.Storage.DeviceTree.Viewer",
+        "GetRequiredMountPoints", []
+    )
+            .then(res => res[0]);
+};
+
 /**
  * @param {Array[string]} diskNames A list of disk names
  *

ui/webui/src/components/AnacondaWizard.jsx

@@ -47,6 +47,7 @@ import { usePageLocation } from "hooks";
 import {
     applyStorage,
     resetPartitioning,
+    getRequiredMountPoints,
 } from "../apis/storage.js";
 
 const _ = cockpit.gettext;
@@ -59,11 +60,21 @@ export const AnacondaWizard = ({ dispatch, isBootIso, osRelease, storageData, lo
     const [storageEncryption, setStorageEncryption] = useState(getStorageEncryptionState());
     const [storageScenarioId, setStorageScenarioId] = useState(window.sessionStorage.getItem("storage-scenario-id") || getDefaultScenario().id);
     const [reusePartitioning, setReusePartitioning] = useState(false);
+    const [requiredMountPoints, setRequiredMountPoints] = useState();
 
     const availableDevices = useMemo(() => {
         return Object.keys(storageData.devices);
     }, [storageData.devices]);
 
+    useEffect(() => {
+        const updateRequiredMountPoints = async () => {
+            const requiredMountPoints = await getRequiredMountPoints().catch(console.error);
+
+            setRequiredMountPoints(requiredMountPoints);
+        };
+        updateRequiredMountPoints();
+    }, []);
+
     useEffect(() => {
         /*
          * When disk selection changes or the user re-scans the devices we need to re-create the partitioning.
@@ -102,7 +113,7 @@ export const AnacondaWizard = ({ dispatch, isBootIso, osRelease, storageData, lo
             label: _("Disk configuration"),
             steps: [{
                 component: MountPointMapping,
-                data: { deviceData: storageData.devices, diskSelection: storageData.diskSelection, partitioningData: storageData.partitioning, dispatch, reusePartitioning, setReusePartitioning },
+                data: { deviceData: storageData.devices, diskSelection: storageData.diskSelection, partitioningData: storageData.partitioning, requiredMountPoints, dispatch, reusePartitioning, setReusePartitioning },
                 id: "mount-point-mapping",
                 label: _("Manual disk configuration"),
                 isHidden: storageScenarioId !== "mount-point-mapping"

ui/webui/src/components/storage/MountPointMapping.jsx

@@ -74,17 +74,17 @@ const getInitialRequests = (usablePartitioningRequests, requiredMountPoints) =>
     const bootEfiOriginalRequest = usablePartitioningRequests.find(r => r["mount-point"] === "/boot/efi");
 
     const requests = requiredMountPoints.map((mountPoint, idx) => {
-        const request = ({ "mount-point": mountPoint.value, reformat: mountPoint.name === "root" });
+        const request = ({ "mount-point": mountPoint["mount-point"].v, reformat: mountPoint["mount-point"].v === "/" });
 
-        if (mountPoint.name === "boot" && bootOriginalRequest) {
+        if (mountPoint["mount-point"].v === "/boot" && bootOriginalRequest) {
             return { ...bootOriginalRequest, ...request };
         }
 
-        if (mountPoint.name === "root" && rootOriginalRequest) {
+        if (mountPoint["mount-point"].v === "/" && rootOriginalRequest) {
             return { ...rootOriginalRequest, ...request };
         }
 
-        if (mountPoint.name === "boot-efi" && bootEfiOriginalRequest) {
+        if (mountPoint["mount-point"].v === "/boot/efi" && bootEfiOriginalRequest) {
             return { ...bootEfiOriginalRequest, ...request };
         }
 
@@ -130,18 +130,20 @@ const isReformatInvalid = (deviceData, request, requests) => {
     }
 };
 
-const isDeviceMountPointInvalid = (deviceData, request) => {
+const isDeviceMountPointInvalid = (deviceData, requiredMountPoints, request) => {
     const device = request["device-spec"];
+    const requiredMountPointData = requiredMountPoints.find(val => val["mount-point"].v === request["mount-point"]);
 
-    if (!device || !request["mount-point"]) {
+    if (!device || !request["mount-point"] || !requiredMountPointData) {
         return [false, ""];
     }
 
-    // /boot/efi must be on EFI System Partition
-    if (request["mount-point"] === "/boot/efi") {
-        if (deviceData[device].formatData.type.v !== "efi") {
-            return [true, _("/boot/efi must be on a EFI System Partition device")];
-        }
+    // we have constraints for filesystem type for required mount points from the backend) {
+    if (requiredMountPointData && requiredMountPointData["required-filesystem-type"].v !== "" &&
+        deviceData[device].formatData.type.v !== requiredMountPointData["required-filesystem-type"].v) {
+        return [true,
+            cockpit.format(_("'$0' must be on a device formatted to '$1'"),
+                           request["mount-point"], requiredMountPointData["required-filesystem-type"].v)];
     }
 
     return [false, ""];
@@ -278,10 +280,10 @@ const DeviceColumnSelect = ({ deviceData, devices, idPrefix, lockedLUKSDevices,
     );
 };
 
-const DeviceColumn = ({ deviceData, devices, idPrefix, handleRequestChange, lockedLUKSDevices, request, requests }) => {
+const DeviceColumn = ({ deviceData, devices, requiredMountPoints, idPrefix, handleRequestChange, lockedLUKSDevices, request, requests }) => {
     const device = request["device-spec"];
     const duplicatedDevice = isDuplicateRequestField(requests, "device-spec", device);
-    const [deviceInvalid, errorMessage] = isDeviceMountPointInvalid(deviceData, request);
+    const [deviceInvalid, errorMessage] = isDeviceMountPointInvalid(deviceData, requiredMountPoints, request);
 
     return (
         <Flex direction={{ default: "column" }} spaceItems={{ default: "spaceItemsNone" }}>
@@ -378,7 +380,7 @@ const RequestsTable = ({
 }) => {
     const columnClassName = idPrefix + "__column";
     const getRequestRow = (request) => {
-        const isRequiredMountPoint = !!requiredMountPoints.find(val => val.value === request["mount-point"]);
+        const isRequiredMountPoint = !!requiredMountPoints.find(val => val["mount-point"].v === request["mount-point"]);
         const duplicatedMountPoint = isDuplicateRequestField(requests, "mount-point", request["mount-point"]);
         const rowId = idPrefix + "-row-" + request["request-id"];
 
@@ -402,6 +404,7 @@ const RequestsTable = ({
                         <DeviceColumn
                           deviceData={deviceData}
                           devices={allDevices}
+                          requiredMountPoints={requiredMountPoints}
                           handleRequestChange={handleRequestChange}
                           idPrefix={rowId + "-device"}
                           lockedLUKSDevices={lockedLUKSDevices}
@@ -590,19 +593,8 @@ const MountPointMappingContent = ({ deviceData, partitioningData, usablePartitio
     }
 };
 
-export const MountPointMapping = ({ deviceData, diskSelection, partitioningData, dispatch, idPrefix, setIsFormValid, onAddErrorNotification, reusePartitioning, setReusePartitioning, stepNotification }) => {
+export const MountPointMapping = ({ deviceData, diskSelection, partitioningData, requiredMountPoints, dispatch, idPrefix, setIsFormValid, onAddErrorNotification, reusePartitioning, setReusePartitioning, stepNotification }) => {
     const [usedPartitioning, setUsedPartitioning] = useState(partitioningData?.path);
-    const requiredMountPoints = useMemo(() => {
-        const mounts = [
-            { value: "/boot", name: "boot" },
-            { value: "/", name: "root" },
-        ];
-
-        cockpit.spawn(["ls", "/sys/firmware/efi"], { err: "ignore" })
-                .then(() => { mounts.push({ value: "/boot/efi", name: "boot-efi" }) });
-
-        return mounts;
-    }, []);
 
     const isUsableDevice = (devSpec, deviceData) => {
         const device = deviceData[devSpec];

ui/webui/test/check-storage

@@ -1001,17 +1001,17 @@ class TestStorageMountPointsEFI(anacondalib.VirtInstallMachineCase):
 
         # verify gathered requests
         # root partition is not auto-mapped
-        s.check_mountpoint_row(1, "/boot", "Select a device", False)
-        s.select_mountpoint_row_device(1, f"{dev}2")
-        s.check_mountpoint_row_format_type(1, "ext4")
+        s.check_mountpoint_row(1, "/boot/efi", "Select a device", False)
+        s.select_mountpoint_row_device(1, f"{dev}1")
+        s.check_mountpoint_row_format_type(1, "EFI System Partition")
 
-        s.check_mountpoint_row(2, "/", "Select a device", True)
-        s.select_mountpoint_row_device(2, f"{dev}3")
-        s.check_mountpoint_row_format_type(2, "xfs")
+        s.check_mountpoint_row(2, "/boot", "Select a device", False)
+        s.select_mountpoint_row_device(2, f"{dev}2")
+        s.check_mountpoint_row_format_type(2, "ext4")
 
-        s.check_mountpoint_row(3, "/boot/efi", "Select a device", False)
-        s.select_mountpoint_row_device(3, f"{dev}1")
-        s.check_mountpoint_row_format_type(3, "EFI System Partition")
+        s.check_mountpoint_row(3, "/", "Select a device", True)
+        s.select_mountpoint_row_device(3, f"{dev}3")
+        s.check_mountpoint_row_format_type(3, "xfs")
 
         i.next()