Allow having duplicate device names

Since we are addressing devices through their device-id it's now
possible to utilize for installation devices with the same name.

src/components/storage/Common.jsx

@@ -40,8 +40,6 @@ import {
     partitioningSetPassphrase,
 } from "../../apis/storage_partitioning.js";
 
-import { findDuplicatesInArray } from "../../helpers/utils.js";
-
 import { StorageContext } from "../Common.jsx";
 import { scenarios } from "./InstallationScenario.jsx";
 
@@ -75,21 +73,6 @@ export const useDiskFreeSpace = ({ devices, selectedDisks }) => {
     return diskFreeSpace;
 };
 
-export const useDuplicateDeviceNames = ({ devices }) => {
-    const [duplicateDeviceNames, setDuplicateDeviceNames] = useState([]);
-
-    useEffect(() => {
-        const update = async () => {
-            const _duplicateDeviceNames = findDuplicatesInArray(Object.keys(devices).map(device => devices[device].name.v));
-
-            setDuplicateDeviceNames(_duplicateDeviceNames);
-        };
-        update();
-    }, [devices]);
-
-    return duplicateDeviceNames;
-};
-
 export const useUsablePartitions = ({ devices, selectedDisks }) => {
     const [usablePartitions, setUsablePartitions] = useState();
 

src/components/storage/InstallationScenario.jsx

@@ -34,7 +34,6 @@ import { StorageReview } from "../review/StorageReview.jsx";
 import {
     useDiskFreeSpace,
     useDiskTotalSpace,
-    useDuplicateDeviceNames,
     useMountPointConstraints,
     useOriginalDeviceTree,
     useRequiredSize,
@@ -106,7 +105,7 @@ const getMissingNonmountablePartitions = (usablePartitions, mountPointConstraint
     return missingNonmountablePartitions;
 };
 
-const checkMountPointMapping = ({ duplicateDeviceNames, mountPointConstraints, selectedDisks, usablePartitions }) => {
+const checkMountPointMapping = ({ mountPointConstraints, selectedDisks, usablePartitions }) => {
     const availability = new AvailabilityState();
 
     availability.hidden = false;
@@ -122,10 +121,6 @@ const checkMountPointMapping = ({ duplicateDeviceNames, mountPointConstraints, s
     } else if (missingNMParts.length) {
         availability.available = false;
         availability.reason = cockpit.format(_("Some required partitions are missing: $0"), missingNMParts.join(", "));
-    } else if (duplicateDeviceNames.length) {
-        availability.available = false;
-        availability.reason = cockpit.format(_("Some devices use the same name: $0."), duplicateDeviceNames.join(", "));
-        availability.hint = _("To use this option, rename devices to have unique names.");
     }
     return availability;
 };
@@ -282,7 +277,6 @@ const InstallationScenarioSelector = ({
     ));
     const diskTotalSpace = useDiskTotalSpace({ devices, selectedDisks });
     const diskFreeSpace = useDiskFreeSpace({ devices, selectedDisks });
-    const duplicateDeviceNames = useDuplicateDeviceNames({ devices });
     const mountPointConstraints = useMountPointConstraints();
     const usablePartitions = useUsablePartitions({ devices, selectedDisks });
     const requiredSize = useRequiredSize();
@@ -301,7 +295,6 @@ const InstallationScenarioSelector = ({
                     devices,
                     diskFreeSpace,
                     diskTotalSpace,
-                    duplicateDeviceNames,
                     mountPointConstraints,
                     partitioning: partitioning.path,
                     requiredSize,
@@ -317,7 +310,6 @@ const InstallationScenarioSelector = ({
         devices,
         diskFreeSpace,
         diskTotalSpace,
-        duplicateDeviceNames,
         mountPointConstraints,
         partitioning.path,
         partitioning.storageScenarioId,

src/components/storage/MountPointMapping.jsx

@@ -261,6 +261,7 @@ const DeviceColumnSelect = ({ deviceData, devices, handleRequestChange, idPrefix
 
     const device = request["device-spec"];
     const deviceName = device && deviceData[device].name.v;
+    const size = cockpit.format_bytes(deviceData[device]?.total.v);
     const options = devices.map(device => {
         const deviceName = deviceData[device].name.v;
         const formatType = deviceData[device]?.formatData.type.v;
@@ -276,11 +277,17 @@ const DeviceColumnSelect = ({ deviceData, devices, handleRequestChange, idPrefix
 
         return (
             <SelectOption
-              data-value={deviceName}
+              data-device-name={deviceName}
+              data-device-id={device}
               isDisabled={isDisabled}
               description={description}
               key={device}
-              value={deviceName}
+              value={{
+                  compareTo: function (value) { return value.device === this.device },
+                  device,
+                  deviceName,
+                  toString: function () { return this.deviceName },
+              }}
             />
         );
     });
@@ -290,11 +297,22 @@ const DeviceColumnSelect = ({ deviceData, devices, handleRequestChange, idPrefix
           hasPlaceholderStyle
           isOpen={isOpen}
           placeholderText={_("Select a device")}
-          selections={deviceName ? [deviceName] : []}
+          selections={device
+              ? [{
+                  compareTo: function (value) { return value.device === this.device },
+                  device,
+                  deviceName,
+                  hasUniqueName: Object.keys(deviceData).filter(dev => deviceData[dev].name.v === deviceName).length > 1,
+                  size,
+                  toString: function () {
+                      if (!this.hasUniqueName) { return this.deviceName } else { return cockpit.format("$0 ($1)", this.deviceName, this.size) }
+                  },
+              }]
+              : []}
           variant={SelectVariant.single}
           onToggle={(_event, val) => setIsOpen(val)}
           onSelect={(_, selection) => {
-              const deviceSpec = devices.find(d => deviceData[d].name.v === selection);
+              const deviceSpec = devices.find(d => d === selection.device);
               handleRequestChange({ deviceSpec, mountPoint: request["mount-point"], requestIndex });
               setIsOpen(false);
           }}

src/helpers/utils.js

@@ -15,14 +15,6 @@
  * along with This program; If not, see <http://www.gnu.org/licenses/>.
  */
 
-/* Find duplicates in an array
- * @param {Array} array
- * @returns {Array} The duplicates
- */
-export const findDuplicatesInArray = (array) => {
-    return array.filter((item, index) => array.indexOf(item) !== index);
-};
-
 /* Check if the given arrays are equal
  * - works only with primitive values
  * @param {Array} array1

test/check-storage-mountpoints

@@ -597,38 +597,54 @@ class TestStorageMountPoints(VirtInstallMachineCase, StorageCase):
         i.check_next_disabled(False)
 
     @skipImage("btrfs support missing on fedora-eln image", "fedora-eln-boot")
-    def testUnsupportedStorageConfiguration(self):
-        # Prevent the user from performing an installation if multiple devices exist with the same name
+    def testDuplicateDeviceNames(self):
         # https://bugzilla.redhat.com/show_bug.cgi?id=2237878
-        # TODO: Make the test nondestructive
         b = self.browser
         m = self.machine
         i = Installer(b, m)
         s = Storage(b, m)
-        _r = Review(b, m)  # noqa: F841
+        r = Review(b, m)
 
         disk = "/dev/vda"
-        tmp_mount = "/tmp/btrfs-mount-test"
-        s.partition_disk(disk, [("1MiB", "biosboot"), ("1GB", "btrfs"), ("", "btrfs")])
-        m.execute(f"""
-        mkdir -p {tmp_mount}
-
-        mount {disk}2 {tmp_mount}
-        btrfs subvolume create {tmp_mount}/home
-        umount {tmp_mount}
-
-        mount {disk}3 {tmp_mount}
-        btrfs subvolume create {tmp_mount}/home
-        umount {tmp_mount}
-        rmdir {tmp_mount}
-        """)
-
+        btrfsname = "home"
+        s.partition_disk(
+            disk,
+            [
+                ("1MiB", "biosboot"),
+                ("1GB", "ext4"),
+                ("5GiB", "btrfs", "-f", "-L", "root"),
+                ("5GiB", "btrfs", "-f", "-L", btrfsname),
+                ("", "btrfs", "-f", "-L", btrfsname)
+            ]
+        )
         s.udevadm_settle()
+        btrfs_volume_ids = s.get_btrfs_volume_ids(btrfsname)
 
         i.open()
         i.reach(i.steps.INSTALLATION_METHOD)
         s.rescan_disks()
-        s.wait_scenario_available("mount-point-mapping", False)
+
+        s.select_mountpoint([("vda", True)])
+
+        s.select_mountpoint_row_device(1, "root")
+        s.select_mountpoint_row_device(2, "vda2")
+        s.add_mountpoint_row()
+        s.select_mountpoint_row_mountpoint(3, "/home/joe")
+        s.select_mountpoint_row_device(3, "home", device_id=btrfs_volume_ids[0])
+        s.add_mountpoint_row()
+        s.select_mountpoint_row_mountpoint(4, "/home/alan")
+        s.select_mountpoint_row_device(4, "home", device_id=btrfs_volume_ids[1])
+
+        i.reach(i.steps.REVIEW)
+
+        # verify review screen
+        disk = "vda"
+        r.check_disk(disk, "16.1 GB vda (0x1af4)")
+
+        r.check_disk_row(disk, "/boot", "vda2", "1.07 GB", False)
+        r.check_disk_row(disk, "/", "vda3", "5.37 GB", True, "btrfs")
+        r.check_disk_row(disk, "/home/joe", "vda4", "5.37 GB", False, "btrfs")
+        r.check_disk_row(disk, "/home/alan", "vda5", "4.29 GB", False, "btrfs")
 
     def testLVM(self):
         b = self.browser

test/helpers/storage.py

@@ -319,6 +319,14 @@ def dbus_set_initialization_mode(self, value):
             {DISK_INITIALIZATION_OBJECT_PATH} \
             {DISK_INITIALIZATION_INTERFACE} InitializationMode i -- {value}')
 
+    def get_btrfs_volume_ids(self, volume_name):
+        """Get device ids of all volumes with volume_name found."""
+        # The tool shows unspecified name as "none"
+        volume_name = volume_name or "none"
+        volume_ids = [f"BTRFS-{uuid.strip()}" for uuid in
+                      self.machine.execute(f"btrfs filesystem show | grep {volume_name} | cut -d ':' -f 3").split('\n')[:-1]]
+        return volume_ids
+
 
 class StorageScenario():
     def __init__(self, browser, machine):
@@ -504,11 +512,14 @@ def select_mountpoint(self, disks, encrypted=False):
     def select_mountpoint_row_mountpoint(self, row, mountpoint):
         self.browser.set_input_text(f"{self.table_row(row)} td[data-label='Mount point'] input", mountpoint)
 
-    def select_mountpoint_row_device(self, row, device):
+    def select_mountpoint_row_device(self, row, device, device_id=None):
         selector = f"{self.table_row(row)} .pf-v5-c-select__toggle"
 
         self.browser.click(f"{selector}:not([disabled]):not([aria-disabled=true])")
-        select_entry = f"{selector} + ul button[data-value='{device}']"
+        if device_id:
+            select_entry = f"{selector} + ul button[data-device-id='{device_id}']"
+        else:
+            select_entry = f"{selector} + ul button[data-device-name='{device}']"
         self.browser.click(select_entry)
         self.browser.wait_in_text(f"{selector} .pf-v5-c-select__toggle-text", device)