Simplify logic for possibly reusing partitioning

If the type matches, (MANUAL vs AUTOMATIC) and the devices also match
then we can reuse the partitioning.

src/components/AnacondaWizard.jsx

@@ -16,7 +16,7 @@
  */
 import cockpit from "cockpit";
 
-import React, { useContext, useEffect, useMemo, useState } from "react";
+import React, { useContext, useEffect, useState } from "react";
 import {
     PageSection,
     PageSectionTypes,
@@ -25,26 +25,21 @@ import {
     WizardStep,
 } from "@patternfly/react-core";
 
-import { resetPartitioning } from "../apis/storage_partitioning.js";
-
 import { AnacondaPage } from "./AnacondaPage.jsx";
 import { AnacondaWizardFooter } from "./AnacondaWizardFooter.jsx";
 import { FooterContext, StorageContext } from "./Common.jsx";
 import { InstallationProgress } from "./installation/InstallationProgress.jsx";
 import { getSteps } from "./steps.js";
 import { CockpitStorageIntegration } from "./storage/CockpitStorageIntegration.jsx";
-const N_ = cockpit.noop;
 
 export const AnacondaWizard = ({ dispatch, onCritFail, setShowStorage, showStorage }) => {
     const [isFormDisabled, setIsFormDisabled] = useState(false);
     const [isFormValid, setIsFormValid] = useState(false);
-    const [reusePartitioning, setReusePartitioning] = useState(false);
     const [stepNotification, setStepNotification] = useState();
     const [showWizard, setShowWizard] = useState(true);
     const [currentStepId, setCurrentStepId] = useState();
     const storageData = useContext(StorageContext);
     const storageScenarioId = storageData.storageScenarioId;
-    const selectedDisks = storageData.diskSelection.selectedDisks;
     const [scenarioPartitioningMapping, setScenarioPartitioningMapping] = useState({});
 
     useEffect(() => {
@@ -56,62 +51,26 @@ export const AnacondaWizard = ({ dispatch, onCritFail, setShowStorage, showStora
         }
     }, [storageData.partitioning.path, storageScenarioId]);
 
-    const availableDevices = useMemo(() => {
-        return Object.keys(storageData.devices);
-    }, [storageData.devices]);
-
     useEffect(() => {
         if (!currentStepId) {
             return;
         }
         cockpit.location.go([currentStepId]);
     }, [currentStepId]);
 
-    useEffect(() => {
-        /*
-         * When disk selection changes or the user re-scans the devices we need to re-create the partitioning.
-         * For Automatic partitioning we do it each time we go to review page,
-         * but for custom mount assignment we try to reuse the partitioning when possible.
-         */
-        setReusePartitioning(false);
-    }, [availableDevices, selectedDisks]);
-
     const componentProps = {
         dispatch,
         isFormDisabled,
         onCritFail,
-        reusePartitioning,
         scenarioPartitioningMapping,
         setIsFormDisabled,
         setIsFormValid,
-        setReusePartitioning,
         setShowStorage,
     };
 
-    const getFlattenedStepsIds = (steps) => {
-        const stepIds = [];
-        for (const step of steps) {
-            stepIds.push(step.id);
-            if (step.steps) {
-                for (const childStep of step.steps) {
-                    if (childStep?.isHidden !== true) {
-                        stepIds.push(childStep.id);
-                    }
-                }
-            }
-        }
-        return stepIds;
-    };
     const stepsOrder = getSteps();
-    const flattenedStepsIds = getFlattenedStepsIds(stepsOrder);
     const firstStepId = stepsOrder[0].id;
 
-    const isStepFollowedBy = (earlierStepId, laterStepId) => {
-        const earlierStepIdx = flattenedStepsIds.findIndex(s => s === earlierStepId);
-        const laterStepIdx = flattenedStepsIds.findIndex(s => s === laterStepId);
-        return earlierStepIdx < laterStepIdx;
-    };
-
     const createSteps = (stepsOrder, componentProps) => {
         return stepsOrder.map(s => {
             const isVisited = firstStepId === s.id || currentStepId === s.id;
@@ -156,20 +115,7 @@ export const AnacondaWizard = ({ dispatch, onCritFail, setShowStorage, showStora
             setIsFormValid(false);
         }
 
-        // Reset the applied partitioning when going back from a step after creating partitioning to a step
-        // before creating partitioning.
-        if ((prevStep.id === "accounts" || isStepFollowedBy("accounts", prevStep.id)) &&
-            isStepFollowedBy(newStep.id, "accounts")) {
-            setIsFormDisabled(true);
-            resetPartitioning()
-                    .then(
-                        () => setCurrentStepId(newStep.id),
-                        () => onCritFail({ context: cockpit.format(N_("Error was hit when going back from $0."), prevStep.prevName) })
-                    )
-                    .always(() => setIsFormDisabled(false));
-        } else {
-            setCurrentStepId(newStep.id);
-        }
+        setCurrentStepId(newStep.id);
     };
 
     if (!showWizard) {

src/components/storage/InstallationDestination.jsx

@@ -47,6 +47,7 @@ import {
     scanDevicesWithTask,
 } from "../../apis/storage.js";
 import { setSelectedDisks } from "../../apis/storage_disks_selection.js";
+import { resetPartitioning } from "../../apis/storage_partitioning.js";
 
 import { getDevicesAction, getDiskSelectionAction } from "../../actions/storage-actions.js";
 
@@ -335,6 +336,16 @@ export const InstallationDestination = ({
         }
     }, [isRescanningDisks, diskSelection.usableDisks]);
 
+    useEffect(() => {
+        // Always reset the partitioning when entering the installation destination page
+        const resetPartitioningAsync = async () => {
+            setIsFormDisabled(true);
+            await resetPartitioning();
+            setIsFormDisabled(false);
+        };
+        resetPartitioningAsync();
+    }, [setIsFormDisabled]);
+
     useEffect(() => {
         // Select default disks for the partitioning on component mount
         if (refUsableDisks.current !== undefined) {

src/components/storage/MountPointMapping.jsx

@@ -46,6 +46,7 @@ import {
 } from "../../apis/storage_partitioning.js";
 
 import {
+    getDeviceAncestors,
     getDeviceChildren,
     getLockedLUKSDevices,
     hasDuplicateFields,
@@ -611,13 +612,33 @@ const isUsableDevice = (devSpec, deviceData) => {
 const MountPointMapping = ({
     dispatch,
     idPrefix,
-    reusePartitioning,
     setIsFormValid,
-    setReusePartitioning,
     setStepNotification,
 }) => {
-    const { devices, partitioning } = useContext(StorageContext);
-    const [usedPartitioning, setUsedPartitioning] = useState(partitioning?.path);
+    const { devices, diskSelection, partitioning } = useContext(StorageContext);
+    const [usedPartitioning, setUsedPartitioning] = useState();
+    const canReusePartitioning = useMemo(() => {
+        // The partitioning can be reused if the devices in the partitioning requests and children
+        // of the selected disks are the same
+        if (usedPartitioning === partitioning?.path) {
+            return true;
+        }
+
+        const usableDevices = Object.keys(devices).filter(device => {
+            const children = devices[device].children.v;
+            const ancestors = getDeviceAncestors(devices, device);
+
+            return children.length === 0 && diskSelection.selectedDisks.some(disk => ancestors.includes(disk));
+        });
+
+        const usedDevices = partitioning?.requests?.map(r => r["device-spec"]) || [];
+
+        if (usedDevices.every(d => usableDevices.includes(d)) && usableDevices.every(d => usedDevices.includes(d))) {
+            return true;
+        }
+        return false;
+    }, [devices, diskSelection.selectedDisks, partitioning.path, partitioning.requests, usedPartitioning]);
+
     const mountPointConstraints = useMountPointConstraints();
     const [skipUnlock, setSkipUnlock] = useState(false);
     const lockedLUKSDevices = useMemo(
@@ -636,7 +657,13 @@ const MountPointMapping = ({
     useWizardFooter(getFooter);
 
     useEffect(() => {
-        if (!reusePartitioning || partitioning?.method !== "MANUAL") {
+        let recreatedPartitioning = false;
+
+        if (partitioning?.method !== "MANUAL" || !canReusePartitioning) {
+            recreatedPartitioning = true;
+        }
+
+        if (recreatedPartitioning) {
             /* Reset the bootloader drive before we schedule partitions
              * The bootloader drive is automatically set during the partitioning, so
              * make sure we always reset the previous value before we run another one,
@@ -647,12 +674,13 @@ const MountPointMapping = ({
                     .then(() => createPartitioning({ method: "MANUAL" }))
                     .then(path => {
                         setUsedPartitioning(path);
-                        setReusePartitioning(true);
                     });
+        } else {
+            setUsedPartitioning(partitioning.path);
         }
-    }, [reusePartitioning, setReusePartitioning, partitioning?.method, partitioning?.path]);
+    }, [canReusePartitioning, partitioning?.method, partitioning?.path]);
 
-    const isLoadingNewPartitioning = !reusePartitioning || usedPartitioning !== partitioning.path;
+    const isLoadingNewPartitioning = usedPartitioning !== partitioning.path;
     const showLuksUnlock = lockedLUKSDevices?.length > 0 && !skipUnlock;
 
     return (

src/helpers/storage.js

@@ -21,7 +21,7 @@
  * @param {string} device - The name of the device
  * @returns {Array}
  */
-const getDeviceAncestors = (deviceData, device) => {
+export const getDeviceAncestors = (deviceData, device) => {
     // device ancestors including the device itself
     const ancestors = [];
     const deviceParents = deviceData[device]?.parents?.v || [];

test/check-storage

@@ -1369,14 +1369,12 @@ class TestStorageCockpitIntegration(anacondalib.VirtInstallMachineCase, StorageC
 
         i.next(next_page=i.steps.CUSTOM_MOUNT_POINT)
 
-        s.select_mountpoint_row_device(1, "root")
-        s.select_mountpoint_row_device(2, "vda2")
-        s.select_mountpoint_row_reformat(2)
-        s.add_mountpoint_row()
+        s.check_mountpoint_row(1, "/", "root", True, "btrfs")
+        s.check_mountpoint_row(2, "/boot", "vda2", False, "ext4")
+        s.check_mountpoint_row(3, "/home", "home", False, "btrfs")
 
-        # Selecting first device, then mount point and then reformat
-        s.select_mountpoint_row_device(3, "home")
-        s.select_mountpoint_row_mountpoint(3, "/home")
+        s.select_mountpoint_row_reformat(1)
+        s.select_mountpoint_row_reformat(2)
         s.select_mountpoint_row_reformat(3, False)
 
         # Root should be preselected to reformat