storage: Collect "available spaces" during page creation

Currently, get_available_spaces implements its own logic for finding
empty block devices and unused space in partition tables. This should
match the logic used during creation of the page tree, and currently
needs to be done with careful coding.

Instead, let's collect the available spaces as a side effect of
creating the page tree.

This has made two edge more consistent with the rule that only empty
block devices (aka "Unformatted data") should be offered as available:

 - LVM2 physical volumes that are not associated with any known volume
   group are no longer considered "available" since they are not
   unformatted.

 - Devices that can be accessed via multiple pathes but that don't
   have a "mpath" device to manage them, might now be offered as
   available via one path.

Author: mvollmer

pkg/storaged/block/create-pages.jsx

@@ -113,7 +113,9 @@ export function make_block_page(parent, block, card) {
         } else if (block_swap ||
                    (content_block.IdUsage == "other" && content_block.IdType == "swap")) {
             card = make_swap_card(card, block, content_block);
-        } else if (client.blocks_available[content_block.path]) {
+        } else if (!content_block.IdUsage) {
+            if (!block.HintIgnore)
+                register_available_block_space(client, content_block);
             card = make_unformatted_data_card(card, block, content_block);
         } else {
             card = make_unrecognized_data_card(card, block, content_block);

pkg/storaged/block/resize.jsx

@@ -188,7 +188,7 @@ function lvol_or_part_and_fsys_resize(client, lvol_or_part, size, offline, passp
                 return Promise.reject(_("Stratis blockdevs can not be made smaller")); // not-covered: safety check
             else
                 return Promise.resolve();
-        } else if (client.blocks_available[block.path]) {
+        } else if (!block.IdUsage) {
             // Growing or shrinking unformatted data, nothing to do
             return Promise.resolve();
         } else if (size < orig_size) {
@@ -293,7 +293,7 @@ export function get_resize_info(client, block, to_fit) {
                 can_grow: false,
             };
             shrink_excuse = grow_excuse = _("Swap can not be resized here");
-        } else if (client.blocks_available[block.path]) {
+        } else if (!block.IdUsage) {
             info = {
                 can_shrink: true,
                 can_grow: true,

pkg/storaged/client.js

@@ -825,13 +825,6 @@ function update_indices() {
         }
     }
 
-    client.blocks_available = { };
-    for (path in client.blocks) {
-        block = client.blocks[path];
-        if (utils.is_available_block(client, block))
-            client.blocks_available[path] = true;
-    }
-
     client.path_jobs = { };
     function enter_job(job) {
         if (!job.Objects || !job.Objects.length)

pkg/storaged/dialog.jsx

@@ -243,7 +243,7 @@ import { FormHelper } from "cockpit-components-form-helper";
 
 import {
     decode_filename, fmt_size, block_name, format_size_and_text, format_delay, for_each_async, get_byte_units,
-    is_available_block, BTRFS_TOOL_MOUNT_PATH
+    BTRFS_TOOL_MOUNT_PATH
 } from "./utils.js";
 import { fmt_to_fragments } from "utils.jsx";
 import client from "./client.js";
@@ -1391,8 +1391,7 @@ export function init_teardown_usage(client, usage, expect_single_unmount) {
                     } else if (u.block.IdUsage == "crypto" && !client.blocks_cleartext[u.block.path]) {
                         u.data_warning = _("Locked encrypted device might contain data");
                     } else if (!client.blocks_ptable[u.block.path] &&
-                               u.block.IdUsage != "raid" &&
-                               !is_available_block(client, u.block)) {
+                               u.block.IdUsage && u.block.IdUsage != "raid") {
                         u.data_warning = _("Device contains unrecognized data");
                     }
                     if (u.data_warning)

pkg/storaged/lvm2/create-dialog.jsx

@@ -56,7 +56,7 @@ export function create_vgroup() {
                                  if (disks.length === 0)
                                      return _("At least one disk is needed.");
                              },
-                             spaces: get_available_spaces(client)
+                             spaces: get_available_spaces()
                          })
         ],
         Action: {

pkg/storaged/lvm2/volume-group.jsx

@@ -204,7 +204,7 @@ function add_disk(vgroup) {
                                  if (disks.length === 0)
                                      return _("At least one disk is needed.");
                              },
-                             spaces: get_available_spaces(client).filter(filter_inside_vgroup)
+                             spaces: get_available_spaces().filter(filter_inside_vgroup)
                          })
         ],
         Action: {

pkg/storaged/mdraid/create-dialog.jsx

@@ -105,7 +105,7 @@ export function create_mdraid() {
                                      return cockpit.format(cockpit.ngettext("At least $0 disk is needed.", "At least $0 disks are needed.", disks_needed),
                                                            disks_needed);
                              },
-                             spaces: get_available_spaces(client)
+                             spaces: get_available_spaces()
                          })
         ],
         Action: {

pkg/storaged/mdraid/mdraid.jsx

@@ -167,7 +167,7 @@ function add_disk(mdraid) {
                                  if (disks.length === 0)
                                      return _("At least one disk is needed.");
                              },
-                             spaces: get_available_spaces(client).filter(filter_inside_mdraid)
+                             spaces: get_available_spaces().filter(filter_inside_mdraid)
                          })
         ],
         Action: {

pkg/storaged/pages.jsx

@@ -39,7 +39,10 @@ import { Spinner } from "@patternfly/react-core/dist/esm/components/Spinner/inde
 import { DescriptionListDescription, DescriptionListGroup, DescriptionListTerm } from "@patternfly/react-core/dist/esm/components/DescriptionList/index.js";
 import { Flex, FlexItem } from "@patternfly/react-core/dist/esm/layouts/Flex/index.js";
 
-import { decode_filename, block_short_name, fmt_size } from "./utils.js";
+import {
+    decode_filename, block_short_name, fmt_size,
+    reset_available_spaces
+} from "./utils.js";
 import { StorageButton, StorageBarMenu, StorageMenuItem, StorageSize } from "./storage-controls.jsx";
 import { MultipathAlert } from "./multipath.jsx";
 import { JobsPanel } from "./jobs-panel.jsx";
@@ -142,6 +145,7 @@ export const PAGE_CATEGORY_NETWORK = 3;
 export function reset_pages() {
     pages = new Map();
     crossrefs = new Map();
+    reset_available_spaces();
 }
 
 function name_from_card(card) {

pkg/storaged/partitions/partition-table.jsx

@@ -21,7 +21,7 @@ import cockpit from "cockpit";
 import React from "react";
 import client from "../client";
 
-import { get_partitions } from "../utils.js";
+import { get_partitions, register_available_free_space } from "../utils.js";
 import { StorageCard, ChildrenTable, new_page, new_card } from "../pages.jsx";
 import { format_dialog } from "../block/format-dialog.jsx";
 import { make_block_page } from "../block/create-pages.jsx";
@@ -68,9 +68,10 @@ function make_partition_pages(parent, block) {
         let p;
         for (i = 0; i < partitions.length; i++) {
             p = partitions[i];
-            if (p.type == 'free')
+            if (p.type == 'free') {
+                register_available_free_space(client, block, p);
                 make_free_space_page(parent, p.start, p.size, enable_dos_extended);
-            else if (p.type == 'container')
+            } else if (p.type == 'container')
                 make_extended_partition_page(parent, p);
             else {
                 const card = make_partition_card(null, p.block);

pkg/storaged/stratis/create-dialog.jsx

@@ -59,7 +59,7 @@ export function create_stratis_pool() {
                                  if (disks.length === 0)
                                      return _("At least one block device is needed.");
                              },
-                             spaces: get_available_spaces(client)
+                             spaces: get_available_spaces()
                          }),
             CheckBoxes("encrypt_pass", _("Options"),
                        {

pkg/storaged/stratis/pool.jsx

@@ -237,7 +237,7 @@ function add_disks(pool) {
                                      if (disks.length === 0)
                                          return _("At least one disk is needed.");
                                  },
-                                 spaces: get_available_spaces(client)
+                                 spaces: get_available_spaces()
                              })
             ],
             Action: {

pkg/storaged/utils.js

@@ -462,93 +462,32 @@ export function get_partitions(client, block) {
     return process_level(0, 0, block.Size);
 }
 
-export function is_available_block(client, block, honor_ignore_hint) {
-    const block_ptable = client.blocks_ptable[block.path];
-    const block_part = client.blocks_part[block.path];
-    const block_pvol = client.blocks_pvol[block.path];
+let available_spaces = [];
 
-    function has_fs_label() {
-        if (!block.IdUsage)
-            return false;
-        // Devices with a LVM2_member label need to actually be
-        // associated with a volume group.
-        if (block.IdType == 'LVM2_member' && (!block_pvol || !client.vgroups[block_pvol.VolumeGroup]))
-            return false;
-        return true;
-    }
-
-    function is_mpath_member() {
-        if (!client.drives[block.Drive])
-            return false;
-        if (!client.drives_block[block.Drive]) {
-            // Broken multipath drive
-            return true;
-        }
-        const members = client.drives_multipath_blocks[block.Drive];
-        for (let i = 0; i < members.length; i++) {
-            if (members[i] == block)
-                return true;
-        }
-        return false;
-    }
-
-    function is_vdo_backing_dev() {
-        return !!client.legacy_vdo_overlay.find_by_backing_block(block);
-    }
-
-    function is_swap() {
-        return !!block && client.blocks_swap[block.path];
-    }
-
-    return (!(block.HintIgnore && honor_ignore_hint) &&
-            block.Size > 0 &&
-            !has_fs_label() &&
-            !is_mpath_member() &&
-            !is_vdo_backing_dev() &&
-            !is_swap() &&
-            !block_ptable &&
-            !(block_part && block_part.IsContainer) &&
-            !should_ignore(client, block.path));
+export function get_available_spaces() {
+    return available_spaces.sort((a, b) => block_cmp(a.block, b.block));
 }
 
-export function get_available_spaces(client) {
-    function make(path) {
-        const block = client.blocks[path];
-        const parts = get_block_link_parts(client, path);
-        const text = cockpit.format(parts.format, parts.link);
-        return { type: 'block', block, size: block.Size, desc: text };
-    }
-
-    const spaces = Object.keys(client.blocks).filter(p => is_available_block(client, client.blocks[p], true))
-            .sort(make_block_path_cmp(client))
-            .map(make);
-
-    function add_free_spaces(block) {
-        const parts = get_partitions(client, block);
-        let i;
-        let p;
-        let link_parts;
-        let text;
-        for (i in parts) {
-            p = parts[i];
-            if (p.type == 'free') {
-                link_parts = get_block_link_parts(client, block.path);
-                text = cockpit.format(link_parts.format, link_parts.link);
-                spaces.push({
-                    type: 'free',
-                    block,
-                    start: p.start,
-                    size: p.size,
-                    desc: cockpit.format(_("unpartitioned space on $0"), text)
-                });
-            }
-        }
-    }
+export function reset_available_spaces() {
+    available_spaces = [];
+}
 
-    for (const p in client.blocks_ptable)
-        add_free_spaces(client.blocks[p]);
+export function register_available_block_space(client, block) {
+    const parts = get_block_link_parts(client, block.path);
+    const text = cockpit.format(parts.format, parts.link);
+    available_spaces.push({ type: 'block', block, size: block.Size, desc: text });
+}
 
-    return spaces;
+export function register_available_free_space(client, block, partition) {
+    const link_parts = get_block_link_parts(client, block.path);
+    const text = cockpit.format(link_parts.format, link_parts.link);
+    available_spaces.push({
+        type: 'free',
+        block,
+        start: partition.start,
+        size: partition.size,
+        desc: cockpit.format(_("unpartitioned space on $0"), text)
+    });
 }
 
 export function prepare_available_spaces(client, spcs) {

test/verify/check-storage-lvm2

@@ -86,15 +86,6 @@ class TestStorageLvm2(storagelib.StorageCase):
         b.wait_visible(self.dropdown_action("Remove") + "[disabled]")
         b.wait_text(self.dropdown_description("Remove"), "Last cannot be removed")
 
-        # Wipe the disk and make sure lvmetad forgets about it.  This
-        # might help with reusing it in the second half of this test.
-        #
-        # HACK - the pvscan is necessary because of
-        # https://bugzilla.redhat.com/show_bug.cgi?id=1063813
-        #
-        m.execute(f"wipefs -a {dev_2}")
-        m.execute(f"pvscan --cache {dev_2}")
-
         # Thin volumes
         m.execute("lvcreate TEST1 --thinpool pool -L 20m")
         b.wait_visible(self.card_row("LVM2 logical volumes", name="pool"))
@@ -114,6 +105,15 @@ class TestStorageLvm2(storagelib.StorageCase):
         b.wait_visible(self.card("Storage"))
         b.wait_not_present(self.card_row("Storage", name="TEST1"))
 
+        # Wipe the disks and make sure lvmetad forgets about them.  This
+        # might help with reusing it in the second half of this test.
+        #
+        # HACK - the pvscan is necessary because of
+        # https://bugzilla.redhat.com/show_bug.cgi?id=1063813
+        #
+        m.execute(f"wipefs -a {dev_1} {dev_2}")
+        m.execute(f"pvscan --cache {dev_1} {dev_2}")
+
         # create volume group in the UI
 
         self.dialog_with_retry(trigger=lambda: self.click_devices_dropdown("Create LVM2 volume group"),
@@ -194,6 +194,9 @@ class TestStorageLvm2(storagelib.StorageCase):
         b.wait_not_present(self.card_row("LVM2 volume group", name=dev_2))
         b.wait_in_text(self.card_desc("LVM2 volume group", "Capacity"), "50.3 MB")
 
+        # loopback needs a explicit trigger...
+        m.execute("udevadm trigger")
+
         # create thin pool and volume
         # the pool will be maximum size, 50.3 MB
         b.click(self.card_button("LVM2 logical volumes", "Create new logical volume"))

test/verify/check-storage-multipath

@@ -78,11 +78,11 @@ class TestStorageMultipath(storagelib.StorageCase):
         b.wait_in_text(self.card_desc("Hard Disk Drive", "Multipathed devices"), dev1)
         b.wait_in_text(self.card_desc("Hard Disk Drive", "Multipathed devices"), dev2)
 
-        # Check that neither is offered as a free block device
+        # Check that only one is offered as a free block device
         b.go("#/")
         self.click_dropdown(self.card_header("Storage"), "Create LVM2 volume group")
         self.dialog_wait_open()
-        check_free_block_devices([])
+        check_free_block_devices(["/dev/sda"])
         self.dialog_cancel()
         self.dialog_wait_close()