Improve block device enumeration and availability detection

Replace the BlockDeviceInfo TypedDict and the per-call disk_is_available()
check with a dataclass that includes availability as a field, determined once
at scan time using mountpoint, mdadm, LVM, and ZFS checks. Extend enumeration
to cover mdadm arrays and multipath devices in addition to local disks. Update
all callers to use attribute access instead of dict subscript.

Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>

Author: glehmann

conftest.py

@@ -425,12 +425,12 @@ def _host_disks(host: Host, hosts_cli_disks: list[DiskDevName] | None) -> Iterab
         # check all disks in --disks=host:... exist
         for cli_disk in hosts_cli_disks:
             for disk in host_disks:
-                if disk['name'] == cli_disk:
+                if disk.name == cli_disk:
                     yield disk
                     break # names are unique, don't expect another one
             else:
                 raise Exception(f"no {cli_disk!r} disk on host {host.hostname_or_ip}, "
-                                f"has {','.join(disk['name'] for disk in host_disks)}")
+                                f"has {','.join(disk.name for disk in host_disks)}")
 
     ret = {host: list(_host_disks(host, cli_disks.get(host.hostname_or_ip)))
            for host in pools_hosts_by_name_or_ip.values()
@@ -443,7 +443,7 @@ def unused_512B_disks(disks: dict[Host, list[Host.BlockDeviceInfo]]
                       ) -> dict[Host, list[Host.BlockDeviceInfo]]:
     """Dict identifying names of all 512-bytes-blocks disks for on all hosts of first pool."""
     ret = {host: [disk for disk in host_disks
-                  if disk["log-sec"] == "512" and host.disk_is_available(disk["name"])]
+                  if disk.log_sec == 512 and disk.available]
            for host, host_disks in disks.items()
            }
     logging.debug("available disks collected: %s", {host.hostname_or_ip: value for host, value in ret.items()})
@@ -454,7 +454,7 @@ def unused_4k_disks(disks: dict[Host, list[Host.BlockDeviceInfo]]
                     ) -> dict[Host, list[Host.BlockDeviceInfo]]:
     """Dict identifying names of all 4K-blocks disks for on all hosts of first pool."""
     ret = {host: [disk for disk in host_disks
-                  if disk["log-sec"] == "4096" and host.disk_is_available(disk["name"])]
+                  if disk.log_sec == 4096 and disk.available]
            for host, host_disks in disks.items()
            }
     logging.debug("available 4k disks collected: %s", {host.hostname_or_ip: value for host, value in ret.items()})

lib/host.py

@@ -7,12 +7,12 @@
 import subprocess
 import tempfile
 import uuid
+from dataclasses import dataclass
 
 from packaging import version
 
 import lib.commands as commands
 from lib.common import (
-    DiskDevName,
     _param_add,
     _param_clear,
     _param_get,
@@ -31,7 +31,7 @@
 from lib.vm import VM
 from lib.xo import xo_cli, xo_object_exists
 
-from typing import TYPE_CHECKING, Literal, TypedDict, overload
+from typing import TYPE_CHECKING, Literal, overload
 
 if TYPE_CHECKING:
     from lib.pool import Pool
@@ -55,14 +55,14 @@ class Host:
     pool: "Pool"
 
     # Data extraction is automatic, no conversion from str is done.
-    BlockDeviceInfo = TypedDict('BlockDeviceInfo', {"name": str,
-                                                    "kname": str,
-                                                    "pkname": str,
-                                                    "size": str,
-                                                    "log-sec": str,
-                                                    "type": str,
-                                                    })
-    BLOCK_DEVICES_FIELDS = ','.join(k.upper() for k in BlockDeviceInfo.__annotations__)
+    @dataclass
+    class BlockDeviceInfo:
+        name: str       # short kernel name: "sda", "md0", "dm-3"
+        path: str       # full device path: "/dev/sda", "/dev/md/myarray", "/dev/mapper/mpathb"
+        size: int       # bytes
+        log_sec: int    # logical sector size; 0 for md/mpath
+        type: str       # "disk", "md", "mpath"
+        available: bool # not mounted, not member of md/lvm/mpath/zfs
 
     block_devices_info: list[BlockDeviceInfo]
 
@@ -669,48 +669,99 @@ def management_pif(self) -> PIF:
 
     def rescan_block_devices_info(self) -> None:
         """
-        Initalize static informations about the disks.
+        Initialize information about block devices: local disks, mdadm arrays, and multipath devices.
 
-        Despite those being static, it can be necessary to rescan,
+        Despite those being mostly static, it can be necessary to rescan,
         when we test how XCP-ng reacts to changes of hardware (or
         reconfiguration of device blocksize), or after a reboot.
         """
-        output_string = self.ssh(
-            f'lsblk --pairs --bytes -I 8,259 --output {Host.BLOCK_DEVICES_FIELDS}'
-        )  # limit to: sd, blkext
+        RAID_TYPES = {'raid0', 'raid1', 'raid4', 'raid5', 'raid6', 'raid10', 'linear'}
+        USED_TYPES = RAID_TYPES | {'lvm', 'mpath', 'crypt'}
+        LSBLK_FIELDS = 'NAME,KNAME,PKNAME,SIZE,LOG-SEC,TYPE,MOUNTPOINT'
+
+        devices: list[Host.BlockDeviceInfo] = []
+
+        raw = self.ssh(f'lsblk --pairs --bytes --output {LSBLK_FIELDS}')
+        rows = [
+            {key.lower(): val.strip('"')
+             for key, val in re.findall(r'(\S+)=(".*?"|\S+)', line)}
+            for line in raw.strip().splitlines()
+        ] if raw.strip() else []
+
+        # build children map: kname -> list of child knames
+        children: dict[str, list[str]] = {}
+        for r in rows:
+            if r.get('pkname'):
+                children.setdefault(r['pkname'], []).append(r['kname'])
+
+        # availability from lsblk fields: no mountpoint, not a "used" type, all descendants also free
+        def _row_by_kname(kname: str) -> dict[str, str] | None:
+            for r in rows:
+                if r['kname'] == kname:
+                    return r
+            return None
 
-        self.block_devices_info = [
-            Host.BlockDeviceInfo({key.lower(): value.strip('"') # type: ignore[misc]
-                                  for key, value in re.findall(r'(\S+)=(".*?"|\S+)', line)})
-            for line in output_string.strip().splitlines()
-        ]
-        logging.debug("blockdevs found: %s", [disk["name"] for disk in self.block_devices_info])
+        def _all_available(kname: str) -> bool:
+            r = _row_by_kname(kname)
+            if r is None:
+                return False
+            if r.get('mountpoint') or r.get('type') in USED_TYPES:
+                return False
+            return all(_all_available(c) for c in children.get(kname, []))
+
+        seen_knames: set[str] = set()
+
+        for r in rows:
+            kname = r['kname']
+            typ = r.get('type', '')
+
+            # --- local disks ---
+            if typ == 'disk' and not r.get('pkname'):
+                devices.append(Host.BlockDeviceInfo(
+                    name=r['name'],
+                    path=f'/dev/{r["name"]}',
+                    size=int(r['size']),
+                    log_sec=int(r['log-sec']),
+                    type='disk',
+                    available=_all_available(kname) and not r['mountpoint'].strip(),
+                ))
+
+            # --- mdadm arrays (may appear once per member, deduplicate) ---
+            elif typ in RAID_TYPES:
+                if kname in seen_knames:
+                    continue
+                seen_knames.add(kname)
+                available = not r['mountpoint'].strip() and all(_all_available(c) for c in children.get(kname, []))
+                devices.append(Host.BlockDeviceInfo(
+                    name=r['name'],
+                    path=f'/dev/{r["name"]}',
+                    size=int(r['size']),
+                    log_sec=int(r.get('log-sec', '0')),
+                    type='md',
+                    available=available,
+                ))
+
+            # --- multipath devices (may appear once per path, deduplicate) ---
+            elif typ == 'mpath':
+                if kname in seen_knames:
+                    continue
+                seen_knames.add(kname)
+                available = not r['mountpoint'].strip() and all(_all_available(c) for c in children.get(kname, []))
+                devices.append(Host.BlockDeviceInfo(
+                    name=kname,
+                    path=f'/dev/mapper/{r["name"]}',
+                    size=int(r['size']),
+                    log_sec=int(r.get('log-sec', '0')),
+                    type='mpath',
+                    available=available,
+                ))
+
+        self.block_devices_info = sorted(devices, key=lambda d: d.name)
+        logging.debug("blockdevs found: %s", [d.name for d in self.block_devices_info])
 
     def disks(self) -> list[Host.BlockDeviceInfo]:
-        """ List of BlockDeviceInfo for all disks. """
-        # store the names of the parent devices to filter out the devices with children
-        pknames = set(disk['pkname'] for disk in self.block_devices_info if disk['pkname'])
-        # filter out partitions from block_devices
-        return sorted(
-            (
-                disk
-                for disk in self.block_devices_info
-                if (not disk["pkname"] or disk['type'] == 'raid0') and disk['kname'] not in pknames
-            ),
-            key=lambda disk: disk["name"],
-        )
-
-    def disk_is_available(self, disk: DiskDevName) -> bool:
-        """
-        Check if a disk is unmounted and appears available for use.
-
-        It may or may not contain identifiable filesystem or partition label.
-        If there are no mountpoints, it is assumed that the disk is not in use.
-
-        Warn: This function may misclassify LVM_member disks (e.g. in XOSTOR, RAID, ZFS) as "available".
-        Such disks may not have mountpoints but still be in use.
-        """
-        return len(self.ssh(f'lsblk --noheadings -o MOUNTPOINT /dev/{disk}').strip()) == 0
+        """ List of all block devices (local disks, mdadm arrays, multipath devices). """
+        return list(self.block_devices_info)
 
     def file_exists(self, filepath: str, regular_file: bool = True) -> bool:
         option = '-f' if regular_file else '-e'

pkgfixtures.py

@@ -25,7 +25,7 @@ def sr_disk_wiped(host: Host, unused_512B_disks: dict[Host, list[Host.BlockDevic
     """A disk on MASTER HOST OF FIRST POOL which we wipe."""
     host_disks = unused_512B_disks[host]
     assert host_disks, f"No 512B disk available on host {host}"
-    sr_disk = host_disks[0]["name"]
+    sr_disk = host_disks[0].name
     logging.info(">> wipe disk %s" % sr_disk)
     host.ssh(f'wipefs -a /dev/{sr_disk}')
     yield sr_disk
@@ -38,7 +38,7 @@ def formatted_and_mounted_ext4_disk(host: Host, unused_512B_disks: dict[Host, li
     mountpoint = '/var/tmp/sr_disk_mountpoint'
     host_disks = unused_512B_disks[host]
     assert host_disks, f"No 512B disk available on host {host}"
-    sr_disk = host_disks[0]["name"]
+    sr_disk = host_disks[0].name
     setup_formatted_and_mounted_disk(host, sr_disk, 'ext4', mountpoint)
     yield mountpoint
     teardown_formatted_and_mounted_disk(host, mountpoint)

tests/storage/conftest.py

@@ -83,7 +83,7 @@ def xfs_sr_on_hostA2(
     _xfs_config_on_hostA2: XfsConfig,
 ) -> Generator[SR, None, None]:
     """ A XFS SR on first host. """
-    sr_disk = unused_512B_disks[hostA2_with_xfsprogs][0]["name"]
+    sr_disk = unused_512B_disks[hostA2_with_xfsprogs][0].name
     sr = hostA2_with_xfsprogs.sr_create('xfs', "XFS-local-SR-test",
                                         {'device': '/dev/' + sr_disk,
                                          'preferred-image-formats': image_format})
@@ -123,7 +123,7 @@ def xfs_sr_on_hostB1(
     _xfs_config_on_hostB1: XfsConfig,
 ) -> Generator[SR, None, None]:
     """ A XFS SR on first host. """
-    sr_disk = unused_512B_disks[hostB1_with_xfsprogs][0]["name"]
+    sr_disk = unused_512B_disks[hostB1_with_xfsprogs][0].name
     sr = hostB1_with_xfsprogs.sr_create('xfs', "XFS-local-SR-test",
                                         {'device': '/dev/' + sr_disk,
                                          'preferred-image-formats': image_format})

tests/storage/ext/conftest.py

@@ -18,7 +18,7 @@ def ext_sr(host: Host,
            image_format: ImageFormat
            ) -> Generator[SR, None, None]:
     """ An EXT SR on first host. """
-    sr_disk = unused_512B_disks[host][0]["name"]
+    sr_disk = unused_512B_disks[host][0].name
     sr = host.sr_create('ext', "EXT-local-SR-test",
                         {'device': '/dev/' + sr_disk,
                          'preferred-image-formats': image_format})

tests/storage/ext/test_ext_sr.py

@@ -42,7 +42,7 @@ def test_create_and_destroy_sr(self, host: Host,
                                    image_format: ImageFormat
                                    ) -> None:
         # Create and destroy tested in the same test to leave the host as unchanged as possible
-        sr_disk = unused_512B_disks[host][0]["name"]
+        sr_disk = unused_512B_disks[host][0].name
         sr = host.sr_create('ext', "EXT-local-SR-test",
                             {'device': '/dev/' + sr_disk,
                              'preferred-image-formats': image_format}, verify=True)

tests/storage/glusterfs/conftest.py

@@ -122,7 +122,7 @@ def gluster_disk(
     pool = pool_with_unused_512B_disk
     mountpoint = '/mnt/sr_disk'
     for h in pool.hosts:
-        sr_disk = unused_512B_disks[h][0]["name"]
+        sr_disk = unused_512B_disks[h][0].name
         setup_formatted_and_mounted_disk(h, sr_disk, 'xfs', mountpoint)
 
     yield

tests/storage/largeblock/conftest.py

@@ -20,7 +20,7 @@ def largeblock_sr(host: Host,
                   unused_4k_disks: dict[Host, list[Host.BlockDeviceInfo]],
                   image_format: ImageFormat) -> Generator[SR, None, None]:
     """ A LARGEBLOCK SR on first host. """
-    sr_disk = unused_4k_disks[host][0]["name"]
+    sr_disk = unused_4k_disks[host][0].name
     sr = host.sr_create('largeblock', "LARGEBLOCK-local-SR-test",
                         {'device': '/dev/' + sr_disk,
                          'preferred-image-formats': image_format})

tests/storage/largeblock/test_largeblock_sr.py

@@ -31,7 +31,7 @@ def test_create_and_destroy_sr(self, host: Host,
                                    unused_4k_disks: dict[Host, list[Host.BlockDeviceInfo]],
                                    image_format: ImageFormat) -> None:
         # Create and destroy tested in the same test to leave the host as unchanged as possible
-        sr_disk = unused_4k_disks[host][0]["name"]
+        sr_disk = unused_4k_disks[host][0].name
         sr = host.sr_create('largeblock', "LARGEBLOCK-local-SR-test",
                             {'device': '/dev/' + sr_disk,
                              'preferred-image-formats': image_format}, verify=True)

tests/storage/linstor/conftest.py

@@ -60,7 +60,7 @@ def lvm_disks(
 
     @functools.cache
     def host_devices(host: Host) -> list[str]:
-        return [os.path.join("/dev", disk["name"]) for disk in unused_512B_disks[host][0:1]]
+        return [disk.path for disk in unused_512B_disks[host][0:1]]
 
     for host in hosts:
         devices = host_devices(host)