From 33eec4c3a9fa5c3829421c8287bdc614b53130fe Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 8 Jul 2025 11:33:45 +0200 Subject: [PATCH 01/26] host.available_disks: don't overload local with different type ... by just avoiding the need for an intermediate variable. Co-authored-by: Ronan Abhamon Signed-off-by: Yann Dirson --- lib/host.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/host.py b/lib/host.py index be9d84363..ad7485973 100644 --- a/lib/host.py +++ b/lib/host.py @@ -582,9 +582,7 @@ def available_disks(self, blocksize=512): avail_disks = [] blk_output = self.ssh(['lsblk', '-nd', '-I', '8,259', '--output', 'NAME,LOG-SEC']).splitlines() for line in blk_output: - line = line.split() - disk = line[0] - sec_size = line[1] + disk, sec_size = line.split() if sec_size == str(blocksize): avail_disks.append(disk) return [disk for disk in avail_disks if self.disk_is_available(disk)] From 7ca01522889d7b03fb9868e172e06094a733026a Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 8 Jul 2025 11:52:06 +0200 Subject: [PATCH 02/26] host: fix comment as too restrictive Signed-off-by: Yann Dirson --- lib/host.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/host.py b/lib/host.py index ad7485973..e767bb3c1 100644 --- a/lib/host.py +++ b/lib/host.py @@ -547,7 +547,7 @@ def management_pif(self): return pif.PIF(uuid, self) def disks(self): - """ List of SCSI disks, e.g ['sda', 'sdb', 'nvme0n1']. """ + """ List of disks, e.g ['sda', 'sdb', 'nvme0n1']. """ disks = self.ssh(['lsblk', '-nd', '-I', '8,259', '--output', 'NAME']).splitlines() disks.sort() return disks From 4b867a54b4a0f0e9cc9c7cf5082d0e338312ee32 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 25 Jun 2025 18:09:15 +0200 Subject: [PATCH 03/26] host: clarify the lsblk invocations Signed-off-by: Yann Dirson --- lib/host.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/host.py b/lib/host.py index e767bb3c1..ed9e13988 100644 --- a/lib/host.py +++ b/lib/host.py @@ -548,7 +548,9 @@ def management_pif(self): def disks(self): """ List of disks, e.g ['sda', 'sdb', 'nvme0n1']. """ - disks = self.ssh(['lsblk', '-nd', '-I', '8,259', '--output', 'NAME']).splitlines() + disks = self.ssh(['lsblk', '--noheadings', '--nodeps', + '-I', '8,259', # limit to: sd, blkext + '--output', 'NAME']).splitlines() disks.sort() return disks @@ -570,7 +572,7 @@ def disk_is_available(self, disk: str) -> bool: 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(['lsblk', '-n', '-o', 'MOUNTPOINT', '/dev/' + disk]).strip()) == 0 + return len(self.ssh(['lsblk', '--noheadings', '-o', 'MOUNTPOINT', '/dev/' + disk]).strip()) == 0 def available_disks(self, blocksize=512): """ @@ -580,7 +582,9 @@ def available_disks(self, blocksize=512): the output of lsblk (including their children such as partitions or md RAID devices) """ avail_disks = [] - blk_output = self.ssh(['lsblk', '-nd', '-I', '8,259', '--output', 'NAME,LOG-SEC']).splitlines() + blk_output = self.ssh(['lsblk', '--noheadings', '--nodeps', + '-I', '8,259', # limit to: sd, blkext + '--output', 'NAME,LOG-SEC']).splitlines() for line in blk_output: disk, sec_size = line.split() if sec_size == str(blocksize): From 4a7640abdad696ecc5bc2935755fe1e0715d29a8 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 17 Jun 2025 15:34:51 +0200 Subject: [PATCH 04/26] host: drop raw_disk_is_available No user yet, and only adds a second level of safety net to disk_is_available. Signed-off-by: Yann Dirson --- lib/host.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/host.py b/lib/host.py index ed9e13988..6650f74d6 100644 --- a/lib/host.py +++ b/lib/host.py @@ -554,14 +554,6 @@ def disks(self): disks.sort() return disks - def raw_disk_is_available(self, disk: str) -> bool: - """ - Check if a raw disk (without any identifiable filesystem or partition label) is available. - - It suggests the disk is "raw" and likely unformatted thus available. - """ - return self.ssh_with_result(['blkid', '/dev/' + disk]).returncode == 2 - def disk_is_available(self, disk: str) -> bool: """ Check if a disk is unmounted and appears available for use. From 8f3993fdea2f9836d1ef621d9b33700094b48e6e Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 18 Jun 2025 16:07:35 +0200 Subject: [PATCH 05/26] glusterfs: remove duplicate fixtures sr_disk_for_all_hosts is pulled by glusterfs_sr, and those tests have no need of the underlying disks that are even not free any more by the time they get the SR. Signed-off-by: Yann Dirson --- tests/storage/glusterfs/test_glusterfs_sr.py | 1 - .../storage/glusterfs/test_glusterfs_sr_crosspool_migration.py | 2 +- .../storage/glusterfs/test_glusterfs_sr_intrapool_migration.py | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/storage/glusterfs/test_glusterfs_sr.py b/tests/storage/glusterfs/test_glusterfs_sr.py index 62d29427f..52fb919c2 100644 --- a/tests/storage/glusterfs/test_glusterfs_sr.py +++ b/tests/storage/glusterfs/test_glusterfs_sr.py @@ -40,7 +40,6 @@ def test_create_and_destroy_sr(self, host, glusterfs_device_config, pool_with_gl vm.destroy(verify=True) sr.destroy(verify=True) -@pytest.mark.usefixtures("sr_disk_for_all_hosts", "glusterfs_sr") class TestGlusterFSSR: @pytest.mark.quicktest def test_quicktest(self, glusterfs_sr): diff --git a/tests/storage/glusterfs/test_glusterfs_sr_crosspool_migration.py b/tests/storage/glusterfs/test_glusterfs_sr_crosspool_migration.py index 60af0e3b4..7bb66e244 100644 --- a/tests/storage/glusterfs/test_glusterfs_sr_crosspool_migration.py +++ b/tests/storage/glusterfs/test_glusterfs_sr_crosspool_migration.py @@ -13,7 +13,7 @@ @pytest.mark.small_vm # run with a small VM to test the features @pytest.mark.big_vm # and ideally with a big VM to test it scales -@pytest.mark.usefixtures("hostB1", "local_sr_on_hostB1", "sr_disk_for_all_hosts") +@pytest.mark.usefixtures("hostB1", "local_sr_on_hostB1") class Test: def test_cold_crosspool_migration(self, host, hostB1, vm_on_glusterfs_sr, local_sr_on_hostB1): cold_migration_then_come_back(vm_on_glusterfs_sr, host, hostB1, local_sr_on_hostB1) diff --git a/tests/storage/glusterfs/test_glusterfs_sr_intrapool_migration.py b/tests/storage/glusterfs/test_glusterfs_sr_intrapool_migration.py index 0cff132f9..4bd361620 100644 --- a/tests/storage/glusterfs/test_glusterfs_sr_intrapool_migration.py +++ b/tests/storage/glusterfs/test_glusterfs_sr_intrapool_migration.py @@ -13,7 +13,7 @@ @pytest.mark.small_vm # run with a small VM to test the features @pytest.mark.big_vm # and ideally with a big VM to test it scales -@pytest.mark.usefixtures("hostA2", "local_sr_on_hostA2", "sr_disk_for_all_hosts") +@pytest.mark.usefixtures("local_sr_on_hostA2") class Test: def test_live_intrapool_shared_migration(self, host, hostA2, vm_on_glusterfs_sr): sr = vm_on_glusterfs_sr.get_sr() From ee5f7aa1dc5ff085040a23537aa5ebc392269965 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 18 Jun 2025 16:56:53 +0200 Subject: [PATCH 06/26] sr_disk: document non-obvious behavior of features to be reworked Quite important to make sure we replace them properly (ie. either with identical behavior, or with a more correct one when applicable). Signed-off-by: Yann Dirson --- conftest.py | 26 ++++++++++++++++++++++++++ pkgfixtures.py | 2 ++ tests/storage/linstor/conftest.py | 10 ++++++++++ 3 files changed, 38 insertions(+) diff --git a/conftest.py b/conftest.py index ccfbe2c6f..9a731db25 100644 --- a/conftest.py +++ b/conftest.py @@ -336,6 +336,12 @@ def local_sr_on_hostB1(hostB1): @pytest.fixture(scope='session') def sr_disk(pytestconfig, host): + """ + Disk DEVICE NAME available on FIRST POOL MASTER. + + Abort if not exactly one --sr_disk. If --sr_disk=auto take any, else + return requested device (abort if not present). + """ disks = pytestconfig.getoption("sr_disk") if len(disks) != 1: pytest.fail("This test requires exactly one --sr-disk parameter") @@ -354,6 +360,12 @@ def sr_disk(pytestconfig, host): @pytest.fixture(scope='session') def sr_disk_4k(pytestconfig, host): + """ + Disk DEVICE NAME with 4KB blocksize available on FIRST POOL MASTER. + + Abort if not exactly one --sr_disk. If --sr_disk=auto take any, else + return requested device (abort if not present). + """ disks = pytestconfig.getoption("sr_disk_4k") if len(disks) != 1: pytest.fail("This test requires exactly one --sr-disks-4k parameter") @@ -372,6 +384,12 @@ def sr_disk_4k(pytestconfig, host): @pytest.fixture(scope='session') def sr_disk_for_all_hosts(pytestconfig, request, host): + """ + Disk DEVICE NAME available on all hosts of FIRST POOL. + + Abort if not exactly one --sr_disk. If --sr_disk=auto take any, else + return requested device (abort if not present). + """ disks = pytestconfig.getoption("sr_disk") if len(disks) != 1: pytest.fail("This test requires exactly one --sr-disk parameter") @@ -401,6 +419,14 @@ def sr_disk_for_all_hosts(pytestconfig, request, host): @pytest.fixture(scope='session') def sr_disks_for_all_hosts(pytestconfig, request, host): + """ + List of disk DEVICE NAMES available on all hosts of FIRST POOL. + + Abort if no --sr_disk. If one --sr_disk=auto given, return names of all + "available" disk device occuring on all hosts in the pool and + IGNORE ALL OTHER --sr_disk; else return all devices on commandline (abort + if any is not present or not "available"). + """ disks = pytestconfig.getoption("sr_disk") assert len(disks) > 0, "This test requires at least one --sr-disk parameter" # Fetch available disks on the master host diff --git a/pkgfixtures.py b/pkgfixtures.py index cbf889173..d22301c14 100644 --- a/pkgfixtures.py +++ b/pkgfixtures.py @@ -13,6 +13,7 @@ # package scope because previous test packages may have used the disk @pytest.fixture(scope='package') def sr_disk_wiped(host, sr_disk): + """A disk on MASTER HOST OF FIRST POOL which we wipe.""" logging.info(">> wipe disk %s" % sr_disk) host.ssh(['wipefs', '-a', '/dev/' + sr_disk]) yield sr_disk @@ -20,6 +21,7 @@ def sr_disk_wiped(host, sr_disk): # package scope so that the device is unmounted before tests from the next package is executed. @pytest.fixture(scope='package') def formatted_and_mounted_ext4_disk(host, sr_disk): + """Mountpoint for newly-formatted disk on MASTER HOST OF FIRST POOL.""" mountpoint = '/var/tmp/sr_disk_mountpoint' setup_formatted_and_mounted_disk(host, sr_disk, 'ext4', mountpoint) yield mountpoint diff --git a/tests/storage/linstor/conftest.py b/tests/storage/linstor/conftest.py index acc965862..971de88d0 100644 --- a/tests/storage/linstor/conftest.py +++ b/tests/storage/linstor/conftest.py @@ -14,6 +14,16 @@ @pytest.fixture(scope='package') def lvm_disks(host, sr_disks_for_all_hosts, provisioning_type): + """ + Common LVM PVs on which a LV is created on each host of the pool. + + On each host in the pool, create PV on each of those disks whose + DEVICE NAME exists ACROSS THE WHOLE POOL. Then make a VG out of + all those, then a LV taking up the whole VG space. + + Return the list of device node paths for that list of devices + used in all hosts. + """ devices = [f"/dev/{disk}" for disk in sr_disks_for_all_hosts] hosts = host.pool.hosts From efdf4f3089abcce9bfa873c77ecb8e1fd9a85b83 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 17 Jun 2025 18:06:02 +0200 Subject: [PATCH 07/26] common: introduce type aliases for HostAddress and DiskDevName Will help readability and improve typechecking. Signed-off-by: Yann Dirson --- lib/commands.py | 16 +++++++++++----- lib/common.py | 7 ++++++- lib/host.py | 7 ++++--- lib/pool.py | 4 ++-- 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/lib/commands.py b/lib/commands.py index 8c5d61d58..24ec1cb07 100644 --- a/lib/commands.py +++ b/lib/commands.py @@ -4,6 +4,7 @@ import subprocess import lib.config as config +from lib.common import HostAddress from lib.netutil import wrap_ip from typing import List, Literal, Union, overload @@ -138,27 +139,32 @@ def _ssh(hostname_or_ip, cmd, check, simple_output, suppress_fingerprint_warning # This function is kept short for shorter pytest traces upon SSH failures, which are common, # as pytest prints the whole function definition that raised the SSHCommandFailed exception @overload -def ssh(hostname_or_ip: str, cmd: Union[str, List[str]], *, check: bool = True, simple_output: Literal[True] = True, +def ssh(hostname_or_ip: HostAddress, cmd: Union[str, List[str]], *, check: bool = True, + simple_output: Literal[True] = True, suppress_fingerprint_warnings: bool = True, background: Literal[False] = False, decode: Literal[True] = True, options: List[str] = []) -> str: ... @overload -def ssh(hostname_or_ip: str, cmd: Union[str, List[str]], *, check: bool = True, simple_output: Literal[True] = True, +def ssh(hostname_or_ip: HostAddress, cmd: Union[str, List[str]], *, check: bool = True, + simple_output: Literal[True] = True, suppress_fingerprint_warnings: bool = True, background: Literal[False] = False, decode: Literal[False], options: List[str] = []) -> bytes: ... @overload -def ssh(hostname_or_ip: str, cmd: Union[str, List[str]], *, check: bool = True, simple_output: Literal[False], +def ssh(hostname_or_ip: HostAddress, cmd: Union[str, List[str]], *, check: bool = True, + simple_output: Literal[False], suppress_fingerprint_warnings: bool = True, background: Literal[False] = False, decode: bool = True, options: List[str] = []) -> SSHResult: ... @overload -def ssh(hostname_or_ip: str, cmd: Union[str, List[str]], *, check: bool = True, simple_output: Literal[False], +def ssh(hostname_or_ip: HostAddress, cmd: Union[str, List[str]], *, check: bool = True, + simple_output: Literal[False], suppress_fingerprint_warnings: bool = True, background: Literal[True], decode: bool = True, options: List[str] = []) -> None: ... @overload -def ssh(hostname_or_ip: str, cmd: Union[str, List[str]], *, check=True, simple_output: bool = True, +def ssh(hostname_or_ip: HostAddress, cmd: Union[str, List[str]], *, check=True, + simple_output: bool = True, suppress_fingerprint_warnings=True, background: bool = False, decode: bool = True, options: List[str] = []) \ -> Union[str, bytes, SSHResult, None]: diff --git a/lib/common.py b/lib/common.py index 85123faf0..101aa19a8 100644 --- a/lib/common.py +++ b/lib/common.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import pytest import getpass @@ -15,13 +17,16 @@ import lib.commands as commands -from typing import TYPE_CHECKING, Callable, Dict, Literal, Optional, TypeVar, Union, cast, overload +from typing import TYPE_CHECKING, Callable, Dict, Literal, Optional, TypeAlias, TypeVar, Union, cast, overload if TYPE_CHECKING: import lib.host T = TypeVar("T") +HostAddress: TypeAlias = str +DiskDevName: TypeAlias = str + class PackageManagerEnum(Enum): UNKNOWN = 1 RPM = 2 diff --git a/lib/host.py b/lib/host.py index 6650f74d6..1fdcbd366 100644 --- a/lib/host.py +++ b/lib/host.py @@ -17,6 +17,7 @@ import lib.pool from lib.common import ( + DiskDevName, _param_add, _param_clear, _param_get, @@ -546,7 +547,7 @@ def management_pif(self): uuid = self.xe('pif-list', {'management': True, 'host-uuid': self.uuid}, minimal=True) return pif.PIF(uuid, self) - def disks(self): + def disks(self) -> list[DiskDevName]: """ List of disks, e.g ['sda', 'sdb', 'nvme0n1']. """ disks = self.ssh(['lsblk', '--noheadings', '--nodeps', '-I', '8,259', # limit to: sd, blkext @@ -554,7 +555,7 @@ def disks(self): disks.sort() return disks - def disk_is_available(self, disk: str) -> bool: + def disk_is_available(self, disk: DiskDevName) -> bool: """ Check if a disk is unmounted and appears available for use. @@ -566,7 +567,7 @@ def disk_is_available(self, disk: str) -> bool: """ return len(self.ssh(['lsblk', '--noheadings', '-o', 'MOUNTPOINT', '/dev/' + disk]).strip()) == 0 - def available_disks(self, blocksize=512): + def available_disks(self, blocksize: int = 512) -> list[DiskDevName]: """ Return a list of available disks for formatting, creating SRs or such. diff --git a/lib/pool.py b/lib/pool.py index 904192534..5cc1051c8 100644 --- a/lib/pool.py +++ b/lib/pool.py @@ -5,7 +5,7 @@ from packaging import version import lib.commands as commands -from lib.common import _param_get, _param_set, safe_split, wait_for, wait_for_not +from lib.common import HostAddress, _param_get, _param_set, safe_split, wait_for, wait_for_not from lib.host import Host from lib.sr import SR @@ -14,7 +14,7 @@ class Pool: xe_prefix = "pool" - def __init__(self, master_hostname_or_ip: str) -> None: + def __init__(self, master_hostname_or_ip: HostAddress) -> None: master = Host(self, master_hostname_or_ip) assert master.is_master(), f"Host {master_hostname_or_ip} is not a master host. Aborting." self.master = master From 1f4daad428504c5786bccaf6decafbf2358cc315 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 17 Jun 2025 18:24:10 +0200 Subject: [PATCH 08/26] Add type hints necessary for the new disks fixtures Signed-off-by: Yann Dirson --- conftest.py | 12 ++++++++---- pkgfixtures.py | 16 ++++++++++++---- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/conftest.py b/conftest.py index 9a731db25..0d81720f0 100644 --- a/conftest.py +++ b/conftest.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import pytest import itertools @@ -12,6 +14,7 @@ from lib import pxe from lib.common import ( callable_marker, + DiskDevName, is_uuid, prefix_object_name, setup_formatted_and_mounted_disk, @@ -21,6 +24,7 @@ wait_for, ) from lib.netutil import is_ipv6 +from lib.host import Host from lib.pool import Pool from lib.sr import SR from lib.vm import VM, vm_cache_key_from_def @@ -31,7 +35,7 @@ # need to import them in the global conftest.py so that they are recognized as fixtures. from pkgfixtures import formatted_and_mounted_ext4_disk, sr_disk_wiped -from typing import Dict +from typing import Dict, Generator # Do we cache VMs? try: @@ -157,7 +161,7 @@ def pytest_runtest_makereport(item, call): # fixtures @pytest.fixture(scope='session') -def hosts(pytestconfig): +def hosts(pytestconfig) -> Generator[list[Host]]: nested_list = [] def setup_host(hostname_or_ip, *, config=None): @@ -335,7 +339,7 @@ def local_sr_on_hostB1(hostB1): yield sr @pytest.fixture(scope='session') -def sr_disk(pytestconfig, host): +def sr_disk(pytestconfig, host: Host) -> Generator[DiskDevName]: """ Disk DEVICE NAME available on FIRST POOL MASTER. @@ -359,7 +363,7 @@ def sr_disk(pytestconfig, host): yield disk @pytest.fixture(scope='session') -def sr_disk_4k(pytestconfig, host): +def sr_disk_4k(pytestconfig, host: Host) -> Generator[DiskDevName]: """ Disk DEVICE NAME with 4KB blocksize available on FIRST POOL MASTER. diff --git a/pkgfixtures.py b/pkgfixtures.py index d22301c14..feb021463 100644 --- a/pkgfixtures.py +++ b/pkgfixtures.py @@ -1,9 +1,17 @@ +from __future__ import annotations + import pytest +from typing import TYPE_CHECKING, Generator import logging from lib.common import setup_formatted_and_mounted_disk, teardown_formatted_and_mounted_disk +if TYPE_CHECKING: + from lib.common import DiskDevName + from lib.host import Host + from lib.pool import Pool + # Due to a bug in the way pytest handles the setup and teardown of package-scoped fixtures, # we moved the following fixtures out of the main conftest.py. # To workaround the bug, the fixture must be imported either in a package's own conftest.py, @@ -12,7 +20,7 @@ # package scope because previous test packages may have used the disk @pytest.fixture(scope='package') -def sr_disk_wiped(host, sr_disk): +def sr_disk_wiped(host: Host, sr_disk: DiskDevName) -> Generator[DiskDevName]: """A disk on MASTER HOST OF FIRST POOL which we wipe.""" logging.info(">> wipe disk %s" % sr_disk) host.ssh(['wipefs', '-a', '/dev/' + sr_disk]) @@ -20,7 +28,7 @@ def sr_disk_wiped(host, sr_disk): # package scope so that the device is unmounted before tests from the next package is executed. @pytest.fixture(scope='package') -def formatted_and_mounted_ext4_disk(host, sr_disk): +def formatted_and_mounted_ext4_disk(host: Host, sr_disk: DiskDevName) -> Generator[str]: """Mountpoint for newly-formatted disk on MASTER HOST OF FIRST POOL.""" mountpoint = '/var/tmp/sr_disk_mountpoint' setup_formatted_and_mounted_disk(host, sr_disk, 'ext4', mountpoint) @@ -28,13 +36,13 @@ def formatted_and_mounted_ext4_disk(host, sr_disk): teardown_formatted_and_mounted_disk(host, mountpoint) @pytest.fixture(scope='package') -def host_with_saved_yum_state(host): +def host_with_saved_yum_state(host: Host) -> Generator[Host]: host.yum_save_state() yield host host.yum_restore_saved_state() @pytest.fixture(scope='package') -def pool_with_saved_yum_state(host): +def pool_with_saved_yum_state(host: Host) -> Generator[Pool]: for h in host.pool.hosts: h.yum_save_state() yield host.pool From 35422b80f521fcea12f9930d126e1fa77626f5b3 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Thu, 31 Jul 2025 15:05:08 +0200 Subject: [PATCH 09/26] Host: use a single lsblk call to cache all static data about disks Those info typically don't change over the course of a test session, but Host.rescan_block_devices_info() is provided for when we'll have more sophisticated test cases. We use a custom TypedDict to be able to use this type more largely in upcoming commits. Reimplements disks() and available_disks() on top of this. Signed-off-by: Yann Dirson --- lib/host.py | 56 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/lib/host.py b/lib/host.py index 1fdcbd366..e4a2604a1 100644 --- a/lib/host.py +++ b/lib/host.py @@ -2,6 +2,7 @@ import logging import os +import re import shlex import tempfile import uuid @@ -11,7 +12,7 @@ import lib.commands as commands import lib.pif as pif -from typing import TYPE_CHECKING, Dict, List, Literal, Optional, Union, overload +from typing import TYPE_CHECKING, Dict, List, Literal, Optional, TypedDict, Union, overload if TYPE_CHECKING: import lib.pool @@ -52,6 +53,18 @@ def host_data(hostname_or_ip): class Host: xe_prefix = "host" + # 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__) + + block_devices_info: list[BlockDeviceInfo] + def __init__(self, pool: 'lib.pool.Pool', hostname_or_ip): self.pool = pool self.hostname_or_ip = hostname_or_ip @@ -69,6 +82,8 @@ def __init__(self, pool: 'lib.pool.Pool', hostname_or_ip): self.xcp_version = version.parse(self.inventory['PRODUCT_VERSION']) self.xcp_version_short = f"{self.xcp_version.major}.{self.xcp_version.minor}" + self.rescan_block_devices_info() + def __str__(self): return self.hostname_or_ip @@ -547,13 +562,29 @@ def management_pif(self): uuid = self.xe('pif-list', {'management': True, 'host-uuid': self.uuid}, minimal=True) return pif.PIF(uuid, self) + def rescan_block_devices_info(self) -> None: + """ + Initalize static informations about the disks. + + Despite those being 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(["lsblk", "--pairs", "--bytes", + '-I', '8,259', # limit to: sd, blkext + "--output", Host.BLOCK_DEVICES_FIELDS]) + + 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 disks(self) -> list[DiskDevName]: """ List of disks, e.g ['sda', 'sdb', 'nvme0n1']. """ - disks = self.ssh(['lsblk', '--noheadings', '--nodeps', - '-I', '8,259', # limit to: sd, blkext - '--output', 'NAME']).splitlines() - disks.sort() - return disks + # filter out partitions from block_devices + return sorted(disk["name"] for disk in self.block_devices_info if not disk["pkname"]) def disk_is_available(self, disk: DiskDevName) -> bool: """ @@ -574,15 +605,10 @@ def available_disks(self, blocksize: int = 512) -> list[DiskDevName]: Returns a list of disk names (eg.: ['sdb', 'sdc']) that don't have any mountpoint in the output of lsblk (including their children such as partitions or md RAID devices) """ - avail_disks = [] - blk_output = self.ssh(['lsblk', '--noheadings', '--nodeps', - '-I', '8,259', # limit to: sd, blkext - '--output', 'NAME,LOG-SEC']).splitlines() - for line in blk_output: - disk, sec_size = line.split() - if sec_size == str(blocksize): - avail_disks.append(disk) - return [disk for disk in avail_disks if self.disk_is_available(disk)] + blocksize_str = str(blocksize) + return [disk["name"] for disk in self.block_devices_info + if not disk["pkname"] and disk["log-sec"] == blocksize_str + and self.disk_is_available(disk["name"])] def file_exists(self, filepath, regular_file=True): option = '-f' if regular_file else '-e' From aa26982c47c459688756fd2bf52bd4bd67b61b7d Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Thu, 31 Jul 2025 17:44:28 +0200 Subject: [PATCH 10/26] Host.disk: change to return BlockDeviceInfo not DiskDevName This method was not used since 7435c995c6231eed80e6d4068a0b6b7a024955c9 (largeblock: add tests largeblock driver + fixture), this repurposes it for a new life. Signed-off-by: Yann Dirson --- lib/host.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/host.py b/lib/host.py index e4a2604a1..0bd20f40f 100644 --- a/lib/host.py +++ b/lib/host.py @@ -581,10 +581,11 @@ def rescan_block_devices_info(self) -> None: ] logging.debug("blockdevs found: %s", [disk["name"] for disk in self.block_devices_info]) - def disks(self) -> list[DiskDevName]: - """ List of disks, e.g ['sda', 'sdb', 'nvme0n1']. """ + def disks(self) -> list[Host.BlockDeviceInfo]: + """ List of BlockDeviceInfo for all disks. """ # filter out partitions from block_devices - return sorted(disk["name"] for disk in self.block_devices_info if not disk["pkname"]) + return sorted((disk for disk in self.block_devices_info if not disk["pkname"]), + key=lambda disk: disk["name"]) def disk_is_available(self, disk: DiskDevName) -> bool: """ From dd4c593f1d002f5a631c2bbf7ab171e506c617c8 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 17 Jun 2025 18:07:09 +0200 Subject: [PATCH 11/26] New fixtures to access disks without assuming uniform naming in first pool Signed-off-by: Yann Dirson --- conftest.py | 32 +++++++++++++++++++++++++++++++- pytest.ini | 1 + 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/conftest.py b/conftest.py index 0d81720f0..14ae295de 100644 --- a/conftest.py +++ b/conftest.py @@ -121,7 +121,8 @@ def pytest_collection_modifyitems(items, config): 'hostA2', 'hostB1', 'sr_disk', - 'sr_disk_4k' + 'sr_disk_4k', + 'unused_512B_disks', ] for item in items: @@ -338,6 +339,27 @@ def local_sr_on_hostB1(hostB1): logging.info(">> local SR on hostB1 present : %s" % sr.uuid) yield sr +@pytest.fixture(scope='session') +def disks(hosts: list[Host]) -> dict[Host, list[Host.BlockDeviceInfo]]: + """Dict identifying names of all disks for on all hosts of first pool.""" + ret = {host: host.disks() + for pool_master in hosts + for host in pool_master.pool.hosts + } + logging.debug("disks collected: %s", {host.hostname_or_ip: value for host, value in ret.items()}) + return ret + +@pytest.fixture(scope='session') +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"])] + for host, host_disks in disks.items() + } + logging.debug("available disks collected: %s", {host.hostname_or_ip: value for host, value in ret.items()}) + return ret + @pytest.fixture(scope='session') def sr_disk(pytestconfig, host: Host) -> Generator[DiskDevName]: """ @@ -421,6 +443,14 @@ def sr_disk_for_all_hosts(pytestconfig, request, host): logging.info(f">> Disk or block device {disk} is present and free on all pool members") yield candidates[0] +@pytest.fixture(scope='session') +def pool_with_unused_512B_disk(host: Host, unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> Pool: + """Returns the first pool, ensuring all hosts have at least one unused 512-bytes-blocks disk.""" + for h in host.pool.hosts: + assert h in unused_512B_disks + assert unused_512B_disks[h], f"host {h} does not have any unused 512B-block disk" + return host.pool + @pytest.fixture(scope='session') def sr_disks_for_all_hosts(pytestconfig, request, host): """ diff --git a/pytest.ini b/pytest.ini index 6886cce3e..aa1b9bd99 100644 --- a/pytest.ini +++ b/pytest.ini @@ -11,6 +11,7 @@ markers = hostB1: a second pool. sr_disk: the test needs a free disk or writable block device that it can erase. sr_disk_4k: the test needs a free 4KiB block device that it can erase. + unused_512B_disks: the test needs one or more free 512B-blocks disk or writable block device that it can erase. # * VM-related markers, automatically set based on fixtures no_vm: tests that do not require a VM to run. From fa8a42829468a116a08b487a1047d9fcca5bf833 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Thu, 31 Jul 2025 17:59:58 +0200 Subject: [PATCH 12/26] glusterfs: switch from sr_disk_for_all_hosts to pool_with_unused_512B_disk Signed-off-by: Yann Dirson --- conftest.py | 35 -------------------- tests/storage/glusterfs/conftest.py | 9 ++--- tests/storage/glusterfs/test_glusterfs_sr.py | 2 +- 3 files changed, 6 insertions(+), 40 deletions(-) diff --git a/conftest.py b/conftest.py index 14ae295de..24139d3c8 100644 --- a/conftest.py +++ b/conftest.py @@ -408,41 +408,6 @@ def sr_disk_4k(pytestconfig, host: Host) -> Generator[DiskDevName]: f"4KiB block device {disk} must be available for use on master host" yield disk -@pytest.fixture(scope='session') -def sr_disk_for_all_hosts(pytestconfig, request, host): - """ - Disk DEVICE NAME available on all hosts of FIRST POOL. - - Abort if not exactly one --sr_disk. If --sr_disk=auto take any, else - return requested device (abort if not present). - """ - disks = pytestconfig.getoption("sr_disk") - if len(disks) != 1: - pytest.fail("This test requires exactly one --sr-disk parameter") - disk = disks[0] - master_disks = host.available_disks() - assert len(master_disks) > 0, "a free disk device is required on the master host" - - if disk != "auto": - assert disk in master_disks, \ - f"disk or block device {disk} is either not present or already used on master host" - master_disks = [disk] - - candidates = list(master_disks) - for h in host.pool.hosts[1:]: - other_disks = h.available_disks() - candidates = [d for d in candidates if d in other_disks] - - if disk == "auto": - assert len(candidates) > 0, \ - f"a free disk device is required on all pool members. Pool master has: {' '.join(master_disks)}." - logging.info(f">> Found free disk device(s) on all pool hosts: {' '.join(candidates)}. Using {candidates[0]}.") - else: - assert len(candidates) > 0, \ - f"disk or block device {disk} was not found to be present and free on all hosts" - logging.info(f">> Disk or block device {disk} is present and free on all pool members") - yield candidates[0] - @pytest.fixture(scope='session') def pool_with_unused_512B_disk(host: Host, unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> Pool: """Returns the first pool, ensuring all hosts have at least one unused 512-bytes-blocks disk.""" diff --git a/tests/storage/glusterfs/conftest.py b/tests/storage/glusterfs/conftest.py index 6c0e721f7..e1cae9c84 100644 --- a/tests/storage/glusterfs/conftest.py +++ b/tests/storage/glusterfs/conftest.py @@ -72,13 +72,14 @@ def pool_with_glusterfs(pool_without_glusterfs, pool_with_saved_yum_state): pool.exec_on_hosts_on_error_continue(_teardown_host_with_glusterfs) @pytest.fixture(scope='package') -def gluster_disk(host, sr_disk_for_all_hosts): - sr_disk = sr_disk_for_all_hosts +def gluster_disk(pool_with_unused_512B_disk, unused_512B_disks): + pool = pool_with_unused_512B_disk mountpoint = '/mnt/sr_disk' - for h in host.pool.hosts: + for h in pool.hosts: + sr_disk = unused_512B_disks[h][0]["name"] setup_formatted_and_mounted_disk(h, sr_disk, 'xfs', mountpoint) yield - host.pool.exec_on_hosts_on_error_continue( + pool.exec_on_hosts_on_error_continue( lambda h: teardown_formatted_and_mounted_disk(h, mountpoint) ) diff --git a/tests/storage/glusterfs/test_glusterfs_sr.py b/tests/storage/glusterfs/test_glusterfs_sr.py index 52fb919c2..9188b753d 100644 --- a/tests/storage/glusterfs/test_glusterfs_sr.py +++ b/tests/storage/glusterfs/test_glusterfs_sr.py @@ -10,7 +10,7 @@ # - one XCP-ng host >= 8.2 with an additional unused disk for the SR # - access to XCP-ng RPM repository from the host -@pytest.mark.usefixtures("sr_disk_for_all_hosts") # don't even run the tests if there's no free disk +@pytest.mark.usefixtures("pool_with_unused_512B_disk") # don't even run the tests if there's no free disk class TestGlusterFSSRCreateDestroy: """ Tests that do not use fixtures that setup the SR or import VMs, From 3f7e61e763ddccd169a0fa629bad3c2ad8ccad3e Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Thu, 31 Jul 2025 18:43:30 +0200 Subject: [PATCH 13/26] disks: add per-host override using --disks=host:[device[,device]*] Fills the role of --sr_disk, except that: - it is per-host - the default is "all disks", use empty device list to forbid all disks on a host Signed-off-by: Yann Dirson --- conftest.py | 54 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/conftest.py b/conftest.py index 24139d3c8..d9f9558c4 100644 --- a/conftest.py +++ b/conftest.py @@ -15,6 +15,7 @@ from lib.common import ( callable_marker, DiskDevName, + HostAddress, is_uuid, prefix_object_name, setup_formatted_and_mounted_disk, @@ -35,7 +36,7 @@ # need to import them in the global conftest.py so that they are recognized as fixtures. from pkgfixtures import formatted_and_mounted_ext4_disk, sr_disk_wiped -from typing import Dict, Generator +from typing import Dict, Generator, Iterable # Do we cache VMs? try: @@ -83,6 +84,14 @@ def pytest_addoption(parser): default=20, help="Max lines to output in a ssh log (0 if no limit)" ) + parser.addoption( + "--disks", + action="append", + default=[], + help="HOST:DISKS to authorize for use by tests. " + "DISKS is a possibly-empty comma-separated list. " + "No mention of a given host authorizes use of all its disks." + ) parser.addoption( "--sr-disk", action="append", @@ -228,6 +237,14 @@ def cleanup_hosts(): cleanup_hosts() +@pytest.fixture(scope='session') +def pools_hosts_by_name_or_ip(hosts: list[Host]) -> Generator[dict[HostAddress, Host]]: + """All hosts of all pools, each indexed by their hostname_or_ip.""" + yield {host.hostname_or_ip: host + for pool_master in hosts + for host in pool_master.pool.hosts + } + @pytest.fixture(scope='session') def registered_xo_cli(): # The fixture is not responsible for establishing the connection. @@ -340,11 +357,38 @@ def local_sr_on_hostB1(hostB1): yield sr @pytest.fixture(scope='session') -def disks(hosts: list[Host]) -> dict[Host, list[Host.BlockDeviceInfo]]: +def disks(pytestconfig, pools_hosts_by_name_or_ip: dict[HostAddress, Host] + ) -> dict[Host, list[Host.BlockDeviceInfo]]: """Dict identifying names of all disks for on all hosts of first pool.""" - ret = {host: host.disks() - for pool_master in hosts - for host in pool_master.pool.hosts + def _parse_disk_option(option_text: str) -> tuple[HostAddress, list[DiskDevName]]: + parsed = option_text.split(sep=":", maxsplit=1) + assert len(parsed) == 2, f"--disks option {option_text!r} is not :[,]*" + host_address, disks_string = parsed + devices = disks_string.split(',') if disks_string else [] + return host_address, devices + + cli_disks = dict(_parse_disk_option(option_text) + for option_text in pytestconfig.getoption("disks")) + + def _host_disks(host: Host, hosts_cli_disks: list[DiskDevName] | None) -> Iterable[Host.BlockDeviceInfo]: + """Filter host disks according to list from `--cli` if given.""" + host_disks = host.disks() + # no disk specified = allow all + if hosts_cli_disks is None: + yield from host_disks + return + # check all disks in --disks=host:... exist + for cli_disk in hosts_cli_disks: + for disk in host_disks: + 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)}") + + ret = {host: list(_host_disks(host, cli_disks.get(host.hostname_or_ip))) + for host in pools_hosts_by_name_or_ip.values() } logging.debug("disks collected: %s", {host.hostname_or_ip: value for host, value in ret.items()}) return ret From 9e983a27556e95c65ff9eb9cc8ce8733e6344879 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 24 Jun 2025 14:24:49 +0200 Subject: [PATCH 14/26] host: explicitly type Host.pool For some reason pyright would not consider the type known otherwise. Also use unquoted type hint, since we have deferred annotations and we use it that way in all other places. Signed-off-by: Yann Dirson --- lib/host.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/host.py b/lib/host.py index 0bd20f40f..37c866261 100644 --- a/lib/host.py +++ b/lib/host.py @@ -15,7 +15,7 @@ from typing import TYPE_CHECKING, Dict, List, Literal, Optional, TypedDict, Union, overload if TYPE_CHECKING: - import lib.pool + from lib.pool import Pool from lib.common import ( DiskDevName, @@ -52,6 +52,7 @@ def host_data(hostname_or_ip): class Host: xe_prefix = "host" + pool: Pool # Data extraction is automatic, no conversion from str is done. BlockDeviceInfo = TypedDict('BlockDeviceInfo', {"name": str, @@ -65,7 +66,7 @@ class Host: block_devices_info: list[BlockDeviceInfo] - def __init__(self, pool: 'lib.pool.Pool', hostname_or_ip): + def __init__(self, pool: Pool, hostname_or_ip): self.pool = pool self.hostname_or_ip = hostname_or_ip self.xo_srv_id: Optional[str] = None From 31327cfac630f127a4fac9cbb170d2fe96597999 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Fri, 20 Jun 2025 16:40:07 +0200 Subject: [PATCH 15/26] linstor: switch from sr_disks_for_all_hosts to pool_with_unused_512B_disk Signed-off-by: Yann Dirson --- tests/storage/linstor/conftest.py | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/storage/linstor/conftest.py b/tests/storage/linstor/conftest.py index 971de88d0..256d4c1e6 100644 --- a/tests/storage/linstor/conftest.py +++ b/tests/storage/linstor/conftest.py @@ -1,19 +1,31 @@ +from __future__ import annotations + import pytest +import functools import logging +import os import lib.commands as commands # explicit import for package-scope fixtures from pkgfixtures import pool_with_saved_yum_state +from typing import TYPE_CHECKING, Generator + +if TYPE_CHECKING: + from lib.host import Host + from lib.pool import Pool + GROUP_NAME = 'linstor_group' STORAGE_POOL_NAME = f'{GROUP_NAME}/thin_device' LINSTOR_RELEASE_PACKAGE = 'xcp-ng-release-linstor' LINSTOR_PACKAGE = 'xcp-ng-linstor' @pytest.fixture(scope='package') -def lvm_disks(host, sr_disks_for_all_hosts, provisioning_type): +def lvm_disks(pool_with_unused_512B_disk: Pool, + unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]], + provisioning_type: str) -> Generator[None]: """ Common LVM PVs on which a LV is created on each host of the pool. @@ -24,10 +36,14 @@ def lvm_disks(host, sr_disks_for_all_hosts, provisioning_type): Return the list of device node paths for that list of devices used in all hosts. """ - devices = [f"/dev/{disk}" for disk in sr_disks_for_all_hosts] - hosts = host.pool.hosts + hosts = pool_with_unused_512B_disk.hosts + + @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]] for host in hosts: + devices = host_devices(host) for device in devices: try: host.ssh(['pvcreate', '-ff', '-y', device]) @@ -45,11 +61,12 @@ def lvm_disks(host, sr_disks_for_all_hosts, provisioning_type): if provisioning_type == 'thin': host.ssh(['lvcreate', '-l', '100%FREE', '-T', STORAGE_POOL_NAME]) - yield devices + # FIXME ought to provide storage_pool_name and get rid of that other fixture + yield None for host in hosts: host.ssh(['vgremove', '-f', GROUP_NAME]) - for device in devices: + for device in host_devices(host): host.ssh(['pvremove', device]) @pytest.fixture(scope="package") From b603608b4403d830f88e6275bb521ee1327d6249 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 24 Jun 2025 15:12:39 +0200 Subject: [PATCH 16/26] Drop now-unused sr_disks_for_all_hosts Signed-off-by: Yann Dirson --- conftest.py | 42 ------------------------------------------ 1 file changed, 42 deletions(-) diff --git a/conftest.py b/conftest.py index d9f9558c4..9734e22ef 100644 --- a/conftest.py +++ b/conftest.py @@ -460,48 +460,6 @@ def pool_with_unused_512B_disk(host: Host, unused_512B_disks: dict[Host, list[Ho assert unused_512B_disks[h], f"host {h} does not have any unused 512B-block disk" return host.pool -@pytest.fixture(scope='session') -def sr_disks_for_all_hosts(pytestconfig, request, host): - """ - List of disk DEVICE NAMES available on all hosts of FIRST POOL. - - Abort if no --sr_disk. If one --sr_disk=auto given, return names of all - "available" disk device occuring on all hosts in the pool and - IGNORE ALL OTHER --sr_disk; else return all devices on commandline (abort - if any is not present or not "available"). - """ - disks = pytestconfig.getoption("sr_disk") - assert len(disks) > 0, "This test requires at least one --sr-disk parameter" - # Fetch available disks on the master host - master_disks = host.available_disks() - assert len(master_disks) > 0, "a free disk device is required on the master host" - - if "auto" in disks: - candidates = list(master_disks) - else: - # Validate that all specified disks exist on the master host - for disk in disks: - assert disk in master_disks, \ - f"Disk or block device {disk} is either not present or already used on the master host" - candidates = list(disks) - - # Check if all disks are available on all hosts in the pool - for h in host.pool.hosts[1:]: - other_disks = h.available_disks() - candidates = [d for d in candidates if d in other_disks] - - if "auto" in disks: - # Automatically select disks if "auto" is passed - assert len(candidates) > 0, \ - f"Free disk devices are required on all pool members. Pool master has: {' '.join(master_disks)}." - logging.info(">> Using free disk device(s) on all pool hosts: %s.", candidates) - else: - # Ensure specified disks are free on all hosts - assert len(candidates) == len(disks), \ - f"Some specified disks ({', '.join(disks)}) are not free or available on all hosts." - logging.info(">> Disk(s) %s are present and free on all pool members", candidates) - yield candidates - @pytest.fixture(scope='module') def vm_ref(request): ref = request.param From 7963173ba75d762fadb34d45575f71c80fe8de3b Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 25 Jun 2025 16:22:49 +0200 Subject: [PATCH 17/26] Use unused_512B_disks in formatted_and_mounted_ext4_disk Signed-off-by: Yann Dirson --- pkgfixtures.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkgfixtures.py b/pkgfixtures.py index feb021463..a02fc519f 100644 --- a/pkgfixtures.py +++ b/pkgfixtures.py @@ -28,9 +28,11 @@ def sr_disk_wiped(host: Host, sr_disk: DiskDevName) -> Generator[DiskDevName]: # package scope so that the device is unmounted before tests from the next package is executed. @pytest.fixture(scope='package') -def formatted_and_mounted_ext4_disk(host: Host, sr_disk: DiskDevName) -> Generator[str]: +def formatted_and_mounted_ext4_disk(host: Host, unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]] + ) -> Generator[str]: """Mountpoint for newly-formatted disk on MASTER HOST OF FIRST POOL.""" mountpoint = '/var/tmp/sr_disk_mountpoint' + sr_disk = unused_512B_disks[host][0]["name"] setup_formatted_and_mounted_disk(host, sr_disk, 'ext4', mountpoint) yield mountpoint teardown_formatted_and_mounted_disk(host, mountpoint) From b3afb310fda4bea7f7b701d0d7b80a428224472c Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 25 Jun 2025 17:21:55 +0200 Subject: [PATCH 18/26] Use unused_512B_disks in ext tests Signed-off-by: Yann Dirson --- tests/storage/ext/conftest.py | 11 ++++++++++- tests/storage/ext/test_ext_sr.py | 10 +++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/storage/ext/conftest.py b/tests/storage/ext/conftest.py index d4a8ac531..d0fb452a9 100644 --- a/tests/storage/ext/conftest.py +++ b/tests/storage/ext/conftest.py @@ -1,10 +1,19 @@ +from __future__ import annotations + import pytest import logging +from typing import TYPE_CHECKING, Generator + +if TYPE_CHECKING: + from lib.host import Host + from lib.sr import SR + @pytest.fixture(scope='package') -def ext_sr(host, sr_disk): +def ext_sr(host: Host, unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> Generator[SR]: """ An EXT SR on first host. """ + sr_disk = unused_512B_disks[host][0]["name"] sr = host.sr_create('ext', "EXT-local-SR-test", {'device': '/dev/' + sr_disk}) yield sr # teardown diff --git a/tests/storage/ext/test_ext_sr.py b/tests/storage/ext/test_ext_sr.py index 2de05e461..bb6bf33a5 100644 --- a/tests/storage/ext/test_ext_sr.py +++ b/tests/storage/ext/test_ext_sr.py @@ -1,8 +1,15 @@ +from __future__ import annotations + import pytest from lib.common import vm_image, wait_for from tests.storage import try_to_create_sr_with_missing_device, vdi_is_open +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from lib.host import Host + # Requirements: # - one XCP-ng host with an additional unused disk for the SR @@ -16,8 +23,9 @@ class TestEXTSRCreateDestroy: def test_create_sr_with_missing_device(self, host): try_to_create_sr_with_missing_device('ext', 'EXT-local-SR-test', host) - def test_create_and_destroy_sr(self, host, sr_disk): + def test_create_and_destroy_sr(self, host: Host, unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> 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 = host.sr_create('ext', "EXT-local-SR-test", {'device': '/dev/' + sr_disk}, verify=True) # import a VM in order to detect vm import issues here rather than in the vm_on_xfs_fixture used in # the next tests, because errors in fixtures break teardown From 36e46e64883517a76bd7dc41b58e00cb518284b5 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Fri, 1 Aug 2025 14:24:42 +0200 Subject: [PATCH 19/26] Use unused_512B_disks in sr_disk_wiped Only used for everything ZFS Signed-off-by: Yann Dirson --- pkgfixtures.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkgfixtures.py b/pkgfixtures.py index a02fc519f..08cbd75e8 100644 --- a/pkgfixtures.py +++ b/pkgfixtures.py @@ -20,8 +20,9 @@ # package scope because previous test packages may have used the disk @pytest.fixture(scope='package') -def sr_disk_wiped(host: Host, sr_disk: DiskDevName) -> Generator[DiskDevName]: +def sr_disk_wiped(host: Host, unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> Generator[DiskDevName]: """A disk on MASTER HOST OF FIRST POOL which we wipe.""" + sr_disk = unused_512B_disks[host][0]["name"] logging.info(">> wipe disk %s" % sr_disk) host.ssh(['wipefs', '-a', '/dev/' + sr_disk]) yield sr_disk From bf63daf476253c5812656af5ca3edbb210f4cf36 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 25 Jun 2025 17:30:18 +0200 Subject: [PATCH 20/26] Use unused_512B_disks in lvm tests Signed-off-by: Yann Dirson --- tests/storage/lvm/conftest.py | 11 ++++++++++- tests/storage/lvm/test_lvm_sr.py | 10 +++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/storage/lvm/conftest.py b/tests/storage/lvm/conftest.py index ab2ffdfaa..fb6845f88 100644 --- a/tests/storage/lvm/conftest.py +++ b/tests/storage/lvm/conftest.py @@ -1,10 +1,19 @@ +from __future__ import annotations + import pytest import logging +from typing import TYPE_CHECKING, Generator + +if TYPE_CHECKING: + from lib.host import Host + from lib.sr import SR + @pytest.fixture(scope='package') -def lvm_sr(host, sr_disk): +def lvm_sr(host: Host, unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> Generator[SR]: """ An LVM SR on first host. """ + sr_disk = unused_512B_disks[host][0]["name"] sr = host.sr_create('lvm', "LVM-local-SR-test", {'device': '/dev/' + sr_disk}) yield sr # teardown diff --git a/tests/storage/lvm/test_lvm_sr.py b/tests/storage/lvm/test_lvm_sr.py index 8877d26ef..7bbf0a38a 100644 --- a/tests/storage/lvm/test_lvm_sr.py +++ b/tests/storage/lvm/test_lvm_sr.py @@ -1,8 +1,15 @@ +from __future__ import annotations + import pytest from lib.common import vm_image, wait_for from tests.storage import try_to_create_sr_with_missing_device, vdi_is_open +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from lib.host import Host + # Requirements: # - one XCP-ng host with an additional unused disk for the SR @@ -16,7 +23,8 @@ class TestLVMSRCreateDestroy: def test_create_sr_with_missing_device(self, host): try_to_create_sr_with_missing_device('lvm', 'LVM-local-SR-test', host) - def test_create_and_destroy_sr(self, host, sr_disk): + def test_create_and_destroy_sr(self, host: Host, unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> None: + sr_disk = unused_512B_disks[host][0]["name"] # Create and destroy tested in the same test to leave the host as unchanged as possible sr = host.sr_create('lvm', "LVM-local-SR-test", {'device': '/dev/' + sr_disk}, verify=True) # import a VM in order to detect vm import issues here rather than in the vm_on_xfs_fixture used in From 08be1664916da5da987bd7023f0bc28ca6bdb1e0 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 25 Jun 2025 17:32:41 +0200 Subject: [PATCH 21/26] Use unused_512B_disks in xfs tests Signed-off-by: Yann Dirson --- tests/storage/xfs/conftest.py | 12 +++++++++++- tests/storage/xfs/test_xfs_sr.py | 19 +++++++++++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/tests/storage/xfs/conftest.py b/tests/storage/xfs/conftest.py index c6f73b574..806a5a8cb 100644 --- a/tests/storage/xfs/conftest.py +++ b/tests/storage/xfs/conftest.py @@ -1,7 +1,15 @@ +from __future__ import annotations + import pytest import logging +from typing import TYPE_CHECKING, Generator + +if TYPE_CHECKING: + from lib.host import Host + from lib.sr import SR + @pytest.fixture(scope='package') def host_with_xfsprogs(host): assert not host.file_exists('/usr/sbin/mkfs.xfs'), \ @@ -13,8 +21,10 @@ def host_with_xfsprogs(host): host.yum_restore_saved_state() @pytest.fixture(scope='package') -def xfs_sr(sr_disk, host_with_xfsprogs): +def xfs_sr(unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]], host_with_xfsprogs: Host + ) -> Generator[SR]: """ A XFS SR on first host. """ + sr_disk = unused_512B_disks[host_with_xfsprogs][0]["name"] sr = host_with_xfsprogs.sr_create('xfs', "XFS-local-SR-test", {'device': '/dev/' + sr_disk}) yield sr # teardown diff --git a/tests/storage/xfs/test_xfs_sr.py b/tests/storage/xfs/test_xfs_sr.py index a591319dc..2d567bee7 100644 --- a/tests/storage/xfs/test_xfs_sr.py +++ b/tests/storage/xfs/test_xfs_sr.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import pytest import logging @@ -7,6 +9,11 @@ from lib.common import vm_image, wait_for from tests.storage import vdi_is_open +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from lib.host import Host + # Requirements: # - one XCP-ng host >= 8.2 with an additional unused disk for the SR # - access to XCP-ng RPM repository from the host @@ -18,10 +25,14 @@ class TestXFSSRCreateDestroy: and VM import. """ - def test_create_xfs_sr_without_xfsprogs(self, host, sr_disk): + def test_create_xfs_sr_without_xfsprogs(self, + host: Host, + unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]] + ) -> None: # This test must be the first in the series in this module assert not host.file_exists('/usr/sbin/mkfs.xfs'), \ "xfsprogs must not be installed on the host at the beginning of the tests" + sr_disk = unused_512B_disks[host][0]["name"] sr = None try: sr = host.sr_create('xfs', "XFS-local-SR-test", {'device': '/dev/' + sr_disk}) @@ -31,9 +42,13 @@ def test_create_xfs_sr_without_xfsprogs(self, host, sr_disk): sr.destroy() assert False, "SR creation should not have succeeded!" - def test_create_and_destroy_sr(self, sr_disk, host_with_xfsprogs): + def test_create_and_destroy_sr(self, + unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]], + host_with_xfsprogs: Host + ) -> None: # Create and destroy tested in the same test to leave the host as unchanged as possible host = host_with_xfsprogs + sr_disk = unused_512B_disks[host][0]["name"] sr = host.sr_create('xfs', "XFS-local-SR-test", {'device': '/dev/' + sr_disk}, verify=True) # import a VM in order to detect vm import issues here rather than in the vm_on_xfs fixture used in # the next tests, because errors in fixtures break teardown From 6ac9204ee6aab78bd5418fabb02d26f94c0f0c52 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 25 Jun 2025 17:39:24 +0200 Subject: [PATCH 22/26] Drop now-unused sr_disk fixture and flag Signed-off-by: Yann Dirson --- conftest.py | 25 ------------------------- pytest.ini | 1 - 2 files changed, 26 deletions(-) diff --git a/conftest.py b/conftest.py index 9734e22ef..01696859c 100644 --- a/conftest.py +++ b/conftest.py @@ -129,7 +129,6 @@ def pytest_collection_modifyitems(items, config): 'windows_vm', 'hostA2', 'hostB1', - 'sr_disk', 'sr_disk_4k', 'unused_512B_disks', ] @@ -404,30 +403,6 @@ def unused_512B_disks(disks: dict[Host, list[Host.BlockDeviceInfo]] logging.debug("available disks collected: %s", {host.hostname_or_ip: value for host, value in ret.items()}) return ret -@pytest.fixture(scope='session') -def sr_disk(pytestconfig, host: Host) -> Generator[DiskDevName]: - """ - Disk DEVICE NAME available on FIRST POOL MASTER. - - Abort if not exactly one --sr_disk. If --sr_disk=auto take any, else - return requested device (abort if not present). - """ - disks = pytestconfig.getoption("sr_disk") - if len(disks) != 1: - pytest.fail("This test requires exactly one --sr-disk parameter") - disk = disks[0] - if disk == "auto": - logging.info(">> Check for the presence of a free disk device on the master host") - disks = host.available_disks() - assert len(disks) > 0, "a free disk device is required on the master host" - disk = disks[0] - logging.info(f">> Found free disk device(s) on hostA1: {' '.join(disks)}. Using {disk}.") - else: - logging.info(f">> Check that disk or block device {disk} is available on the master host") - assert disk in host.available_disks(), \ - f"disk or block device {disk} is either not present or already used on master host" - yield disk - @pytest.fixture(scope='session') def sr_disk_4k(pytestconfig, host: Host) -> Generator[DiskDevName]: """ diff --git a/pytest.ini b/pytest.ini index aa1b9bd99..9e179f157 100644 --- a/pytest.ini +++ b/pytest.ini @@ -9,7 +9,6 @@ markers = # * Host-related markers, automatically set based on fixtures hostA2: a second member in the first pool. hostB1: a second pool. - sr_disk: the test needs a free disk or writable block device that it can erase. sr_disk_4k: the test needs a free 4KiB block device that it can erase. unused_512B_disks: the test needs one or more free 512B-blocks disk or writable block device that it can erase. From a6f83ea01f5fdbb6d1cf254adef00eaeb070d037 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 25 Jun 2025 18:15:38 +0200 Subject: [PATCH 23/26] New fixture unused_4k_disks for largeblocks tests Signed-off-by: Yann Dirson --- conftest.py | 12 ++++++++++++ jobs.py | 16 ++++++++-------- pytest.ini | 1 + tests/storage/largeblock/conftest.py | 13 +++++++++++-- tests/storage/largeblock/test_largeblock_sr.py | 12 ++++++++++-- 5 files changed, 42 insertions(+), 12 deletions(-) diff --git a/conftest.py b/conftest.py index 01696859c..bff6f88a8 100644 --- a/conftest.py +++ b/conftest.py @@ -131,6 +131,7 @@ def pytest_collection_modifyitems(items, config): 'hostB1', 'sr_disk_4k', 'unused_512B_disks', + 'unused_4k_disks', ] for item in items: @@ -403,6 +404,17 @@ def unused_512B_disks(disks: dict[Host, list[Host.BlockDeviceInfo]] logging.debug("available disks collected: %s", {host.hostname_or_ip: value for host, value in ret.items()}) return ret +@pytest.fixture(scope='session') +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"])] + 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()}) + return ret + @pytest.fixture(scope='session') def sr_disk_4k(pytestconfig, host: Host) -> Generator[DiskDevName]: """ diff --git a/jobs.py b/jobs.py index 2f418687f..1d021432f 100755 --- a/jobs.py +++ b/jobs.py @@ -91,7 +91,7 @@ "--sr-disk": "auto", }, "paths": ["tests/storage"], - "markers": "(small_vm or no_vm) and not reboot and not quicktest and not sr_disk_4k", + "markers": "(small_vm or no_vm) and not reboot and not quicktest and not unused_4k_disks", "name_filter": "not migration and not linstor", }, "storage-migrations": { @@ -109,7 +109,7 @@ "--sr-disk": "auto", }, "paths": ["tests/storage"], - "markers": "not sr_disk_4k", + "markers": "not unused_4k_disks", "name_filter": "migration and not linstor", }, "storage-reboots": { @@ -126,7 +126,7 @@ "--sr-disk": "auto", }, "paths": ["tests/storage"], - "markers": "reboot and not flaky and not sr_disk_4k", + "markers": "reboot and not flaky and not unused_4k_disks", "name_filter": "not linstor", }, "storage-quicktest": { @@ -141,7 +141,7 @@ "--sr-disk": "auto", }, "paths": ["tests/storage"], - "markers": "quicktest and not sr_disk_4k", + "markers": "quicktest and not unused_4k_disks", "name_filter": "not linstor and not zfsvol", }, "linstor-main": { @@ -218,7 +218,7 @@ "--sr-disk-4k": "auto", }, "paths": ["tests/storage"], - "markers": "(small_vm or no_vm) and sr_disk_4k and not reboot and not quicktest", + "markers": "(small_vm or no_vm) and unused_4k_disks and not reboot and not quicktest", "name_filter": "not migration", }, "largeblock-migrations": { @@ -235,7 +235,7 @@ "--sr-disk-4k": "auto", }, "paths": ["tests/storage"], - "markers": "sr_disk_4k", + "markers": "unused_4k_disks", "name_filter": "migration", }, "largeblock-reboots": { @@ -251,7 +251,7 @@ "--sr-disk-4k": "auto", }, "paths": ["tests/storage"], - "markers": "sr_disk_4k and reboot", + "markers": "unused_4k_disks and reboot", }, "largeblock-quicktest": { "description": "runs `quicktest` on the largeblock storage driver", @@ -264,7 +264,7 @@ "--sr-disk-4k": "auto", }, "paths": ["tests/storage"], - "markers": "sr_disk_4k and quicktest", + "markers": "unused_4k_disks and quicktest", }, "sb-main": { "description": "tests uefistored/varstored and SecureBoot using a small unix VM (or no VM when none needed)", diff --git a/pytest.ini b/pytest.ini index 9e179f157..1c734e64e 100644 --- a/pytest.ini +++ b/pytest.ini @@ -11,6 +11,7 @@ markers = hostB1: a second pool. sr_disk_4k: the test needs a free 4KiB block device that it can erase. unused_512B_disks: the test needs one or more free 512B-blocks disk or writable block device that it can erase. + unused_4k_disks: the test needs a free 4KiB block device that it can erase. # * VM-related markers, automatically set based on fixtures no_vm: tests that do not require a VM to run. diff --git a/tests/storage/largeblock/conftest.py b/tests/storage/largeblock/conftest.py index 17ea67174..58702a757 100644 --- a/tests/storage/largeblock/conftest.py +++ b/tests/storage/largeblock/conftest.py @@ -1,11 +1,20 @@ +from __future__ import annotations + import pytest import logging +from typing import TYPE_CHECKING, Generator + +if TYPE_CHECKING: + from lib.host import Host + from lib.sr import SR + @pytest.fixture(scope='package') -def largeblock_sr(host, sr_disk_4k): +def largeblock_sr(host: Host, unused_4k_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> Generator[SR]: """ A LARGEBLOCK SR on first host. """ - sr = host.sr_create('largeblock', "LARGEBLOCK-local-SR-test", {'device': '/dev/' + sr_disk_4k}) + sr_disk = unused_4k_disks[host][0]["name"] + sr = host.sr_create('largeblock', "LARGEBLOCK-local-SR-test", {'device': '/dev/' + sr_disk}) yield sr # teardown sr.destroy() diff --git a/tests/storage/largeblock/test_largeblock_sr.py b/tests/storage/largeblock/test_largeblock_sr.py index 97ec815af..c2683283e 100644 --- a/tests/storage/largeblock/test_largeblock_sr.py +++ b/tests/storage/largeblock/test_largeblock_sr.py @@ -1,8 +1,15 @@ +from __future__ import annotations + import pytest from lib.common import vm_image, wait_for from tests.storage import try_to_create_sr_with_missing_device, vdi_is_open +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from lib.host import Host + # Requirements: # - one XCP-ng host with an additional unused 4KiB disk for the SR @@ -16,9 +23,10 @@ class TestLARGEBLOCKSRCreateDestroy: def test_create_sr_with_missing_device(self, host): try_to_create_sr_with_missing_device('largeblock', 'LARGEBLOCK-local-SR-test', host) - def test_create_and_destroy_sr(self, host, sr_disk_4k): + def test_create_and_destroy_sr(self, host: Host, unused_4k_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> None: # Create and destroy tested in the same test to leave the host as unchanged as possible - sr = host.sr_create('largeblock', "LARGEBLOCK-local-SR-test", {'device': '/dev/' + sr_disk_4k}, verify=True) + sr_disk = unused_4k_disks[host][0]["name"] + sr = host.sr_create('largeblock', "LARGEBLOCK-local-SR-test", {'device': '/dev/' + sr_disk}, verify=True) # import a VM in order to detect vm import issues here rather than in the vm_on_xfs_fixture used in # the next tests, because errors in fixtures break teardown vm = host.import_vm(vm_image('mini-linux-x86_64-bios'), sr_uuid=sr.uuid) From 407777011fea2cf307855b6d921257c110cfae3f Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Thu, 31 Jul 2025 18:47:10 +0200 Subject: [PATCH 24/26] Drop now-unused sr_disk_4k Signed-off-by: Yann Dirson --- conftest.py | 25 ------------------------- pytest.ini | 1 - 2 files changed, 26 deletions(-) diff --git a/conftest.py b/conftest.py index bff6f88a8..c436654dd 100644 --- a/conftest.py +++ b/conftest.py @@ -129,7 +129,6 @@ def pytest_collection_modifyitems(items, config): 'windows_vm', 'hostA2', 'hostB1', - 'sr_disk_4k', 'unused_512B_disks', 'unused_4k_disks', ] @@ -415,30 +414,6 @@ def unused_4k_disks(disks: dict[Host, list[Host.BlockDeviceInfo]] logging.debug("available 4k disks collected: %s", {host.hostname_or_ip: value for host, value in ret.items()}) return ret -@pytest.fixture(scope='session') -def sr_disk_4k(pytestconfig, host: Host) -> Generator[DiskDevName]: - """ - Disk DEVICE NAME with 4KB blocksize available on FIRST POOL MASTER. - - Abort if not exactly one --sr_disk. If --sr_disk=auto take any, else - return requested device (abort if not present). - """ - disks = pytestconfig.getoption("sr_disk_4k") - if len(disks) != 1: - pytest.fail("This test requires exactly one --sr-disks-4k parameter") - disk = disks[0] - if disk == "auto": - logging.info(">> Check for the presence of a free 4KiB block device on the master host") - disks = host.available_disks(4096) - assert len(disks) > 0, "a free 4KiB block device is required on the master host" - disk = disks[0] - logging.info(f">> Found free 4KiB block device(s) on hostA1: {' '.join(disks)}. Using {disk}.") - else: - logging.info(f">> Check that 4KiB block device {disk} is available on the master host") - assert disk in host.available_disks(4096), \ - f"4KiB block device {disk} must be available for use on master host" - yield disk - @pytest.fixture(scope='session') def pool_with_unused_512B_disk(host: Host, unused_512B_disks: dict[Host, list[Host.BlockDeviceInfo]]) -> Pool: """Returns the first pool, ensuring all hosts have at least one unused 512-bytes-blocks disk.""" diff --git a/pytest.ini b/pytest.ini index 1c734e64e..8e78251d4 100644 --- a/pytest.ini +++ b/pytest.ini @@ -9,7 +9,6 @@ markers = # * Host-related markers, automatically set based on fixtures hostA2: a second member in the first pool. hostB1: a second pool. - sr_disk_4k: the test needs a free 4KiB block device that it can erase. unused_512B_disks: the test needs one or more free 512B-blocks disk or writable block device that it can erase. unused_4k_disks: the test needs a free 4KiB block device that it can erase. From f0a42d4f547765c8c08ae5154e85830904cbcd37 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 8 Jul 2025 16:55:16 +0200 Subject: [PATCH 25/26] Drop now-unused Host.available_disks Signed-off-by: Yann Dirson --- lib/host.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/host.py b/lib/host.py index 37c866261..123aae1cc 100644 --- a/lib/host.py +++ b/lib/host.py @@ -600,18 +600,6 @@ def disk_is_available(self, disk: DiskDevName) -> bool: """ return len(self.ssh(['lsblk', '--noheadings', '-o', 'MOUNTPOINT', '/dev/' + disk]).strip()) == 0 - def available_disks(self, blocksize: int = 512) -> list[DiskDevName]: - """ - Return a list of available disks for formatting, creating SRs or such. - - Returns a list of disk names (eg.: ['sdb', 'sdc']) that don't have any mountpoint in - the output of lsblk (including their children such as partitions or md RAID devices) - """ - blocksize_str = str(blocksize) - return [disk["name"] for disk in self.block_devices_info - if not disk["pkname"] and disk["log-sec"] == blocksize_str - and self.disk_is_available(disk["name"])] - def file_exists(self, filepath, regular_file=True): option = '-f' if regular_file else '-e' return self.ssh_with_result(['test', option, filepath]).returncode == 0 From e309b8f2eb34cbbace2736a43b319b2b28fc067a Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 20 Aug 2025 11:16:01 +0200 Subject: [PATCH 26/26] Drop obsolete --sr-disk* options and their use in jobs and examples The old options had been missed and kept as no-op, which should not be a problem for any caller using "auto" (which was anyway the only sensible choice for a generic script). Signed-off-by: Yann Dirson --- README.md | 2 +- conftest.py | 15 --------------- jobs.py | 17 ----------------- tests/misc/test_export.py | 4 +--- tests/storage/iso/test_local_iso_sr.py | 6 +++--- tests/storage/iso/test_local_iso_sr_reboot.py | 6 +++--- tests/uefi_sb/test_varstored_cert_flow.py | 3 +-- 7 files changed, 9 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 69708bc61..63eab7be2 100644 --- a/README.md +++ b/README.md @@ -205,7 +205,7 @@ Here's an example of selection we can do thanks to the markers: ``` # Run storage driver tests that either need no VM at all, or advise to use a small VM. Exclude tests that reboot the hosts. -pytest tests/storage -m "(small_vm or no_vm) and not reboot" --hosts=ip_of_poolmaster1,ip_of_poolmaster2 --vm=http://path/to/a_small_vm.xva --sr-disk=auto +pytest tests/storage -m "(small_vm or no_vm) and not reboot" --hosts=ip_of_poolmaster1,ip_of_poolmaster2 --vm=http://path/to/a_small_vm.xva ``` Another example: diff --git a/conftest.py b/conftest.py index c436654dd..d4874d315 100644 --- a/conftest.py +++ b/conftest.py @@ -92,21 +92,6 @@ def pytest_addoption(parser): "DISKS is a possibly-empty comma-separated list. " "No mention of a given host authorizes use of all its disks." ) - parser.addoption( - "--sr-disk", - action="append", - default=[], - help="Name of an available disk (sdb) or partition device (sdb2) to be formatted and used in storage tests. " - "Set it to 'auto' to let the fixtures auto-detect available disks." - ) - parser.addoption( - "--sr-disk-4k", - action="append", - default=[], - help="Name of an available disk (sdb) or partition device (sdb2) with " - "4KiB blocksize to be formatted and used in storage tests. " - "Set it to 'auto' to let the fixtures auto-detect available disks." - ) def pytest_configure(config): global_config.ignore_ssh_banner = config.getoption('--ignore-ssh-banner') diff --git a/jobs.py b/jobs.py index 1d021432f..2c9f16405 100755 --- a/jobs.py +++ b/jobs.py @@ -20,7 +20,6 @@ "nb_pools": 2, "params": { "--vm": "single/small_vm", - "--sr-disk": "auto", }, "paths": [ "tests/misc", @@ -46,7 +45,6 @@ "nb_pools": 2, "params": { "--vm[]": "multi/all_unix", - "--sr-disk": "auto", }, "paths": ["tests/misc", "tests/migration"], "markers": "multi_vms and not flaky and not reboot", @@ -62,7 +60,6 @@ "nb_pools": 2, "params": { "--vm[]": "multi/all_windows", - "--sr-disk": "auto", }, "paths": ["tests/misc", "tests/migration"], "markers": "multi_vms and not flaky and not reboot", @@ -88,7 +85,6 @@ "nb_pools": 1, "params": { "--vm": "single/small_vm", - "--sr-disk": "auto", }, "paths": ["tests/storage"], "markers": "(small_vm or no_vm) and not reboot and not quicktest and not unused_4k_disks", @@ -106,7 +102,6 @@ "nb_pools": 2, "params": { "--vm": "single/small_vm", - "--sr-disk": "auto", }, "paths": ["tests/storage"], "markers": "not unused_4k_disks", @@ -123,7 +118,6 @@ "nb_pools": 1, "params": { "--vm": "single/small_vm", - "--sr-disk": "auto", }, "paths": ["tests/storage"], "markers": "reboot and not flaky and not unused_4k_disks", @@ -138,7 +132,6 @@ ], "nb_pools": 1, "params": { - "--sr-disk": "auto", }, "paths": ["tests/storage"], "markers": "quicktest and not unused_4k_disks", @@ -154,7 +147,6 @@ "nb_pools": 1, "params": { "--vm": "single/small_vm", - "--sr-disk": "auto", }, "paths": ["tests/storage/linstor"], "markers": "(small_vm or no_vm) and not reboot and not quicktest", @@ -171,7 +163,6 @@ "nb_pools": 2, "params": { "--vm": "single/small_vm", - "--sr-disk": "auto", }, "paths": ["tests/storage/linstor"], "markers": "", @@ -187,7 +178,6 @@ "nb_pools": 1, "params": { "--vm": "single/small_vm", - "--sr-disk": "auto", }, "paths": ["tests/storage/linstor"], "markers": "reboot", @@ -200,7 +190,6 @@ ], "nb_pools": 1, "params": { - "--sr-disk": "auto", }, "paths": ["tests/storage/linstor"], "markers": "quicktest", @@ -215,7 +204,6 @@ "nb_pools": 1, "params": { "--vm": "single/small_vm", - "--sr-disk-4k": "auto", }, "paths": ["tests/storage"], "markers": "(small_vm or no_vm) and unused_4k_disks and not reboot and not quicktest", @@ -232,7 +220,6 @@ "nb_pools": 2, "params": { "--vm": "single/small_vm", - "--sr-disk-4k": "auto", }, "paths": ["tests/storage"], "markers": "unused_4k_disks", @@ -248,7 +235,6 @@ "nb_pools": 1, "params": { "--vm": "single/small_vm", - "--sr-disk-4k": "auto", }, "paths": ["tests/storage"], "markers": "unused_4k_disks and reboot", @@ -261,7 +247,6 @@ ], "nb_pools": 1, "params": { - "--sr-disk-4k": "auto", }, "paths": ["tests/storage"], "markers": "unused_4k_disks and quicktest", @@ -297,7 +282,6 @@ # nb_pools left to 1 so that the job can run on XCP-ng 8.2 with just one pool, but 2 are required in 8.3+ "nb_pools": 1, "params": { - "--sr-disk": "auto", "--vm": "single/small_vm_efitools", }, "paths": ["tests/uefi_sb/test_uefistored_cert_flow.py", "tests/uefi_sb/test_varstored_cert_flow.py"], @@ -420,7 +404,6 @@ "nb_pools": 1, "params": { "--vm": "single/small_vm", - "--sr-disk": "auto", }, "paths": ["tests"], "markers": "flaky", diff --git a/tests/misc/test_export.py b/tests/misc/test_export.py index b5054eda4..5b007f6f5 100644 --- a/tests/misc/test_export.py +++ b/tests/misc/test_export.py @@ -4,9 +4,7 @@ # Requirements: # From --hosts parameter: -# - host: a XCP-ng host -# From --sr-disk parameter: -# - an additional unused disk to store the exported VM +# - host: a XCP-ng host with an unused disk to store the exported VM # From --vm parameter: # - A VM to import and export diff --git a/tests/storage/iso/test_local_iso_sr.py b/tests/storage/iso/test_local_iso_sr.py index 96013876a..827a7bcca 100644 --- a/tests/storage/iso/test_local_iso_sr.py +++ b/tests/storage/iso/test_local_iso_sr.py @@ -12,9 +12,9 @@ # Requirements: # From --hosts parameter: -# - host: a XCP-ng host, with the default SR being either a shared SR, or a local SR on the master host -# From --sr-disk parameter: -# - an additional unused disk for the SR +# - host: a XCP-ng host, with: +# - the default SR being either a shared SR, or a local SR on the master host +# - an additional unused disk for the SR # From --vm parameter: # - A VM to import diff --git a/tests/storage/iso/test_local_iso_sr_reboot.py b/tests/storage/iso/test_local_iso_sr_reboot.py index 12db19cac..e896600f5 100644 --- a/tests/storage/iso/test_local_iso_sr_reboot.py +++ b/tests/storage/iso/test_local_iso_sr_reboot.py @@ -8,9 +8,9 @@ # Requirements: # From --hosts parameter: -# - host: a XCP-ng host, with the default SR being either a shared SR, or a local SR on the master host -# From --sr-disk parameter: -# - an additional unused disk for the SR +# - host: a XCP-ng host, with: +# - the default SR being either a shared SR, or a local SR on the master host +# - an additional unused disk for the SR # From --vm parameter: # - A VM to import diff --git a/tests/uefi_sb/test_varstored_cert_flow.py b/tests/uefi_sb/test_varstored_cert_flow.py index 384402a01..6c067287c 100644 --- a/tests/uefi_sb/test_varstored_cert_flow.py +++ b/tests/uefi_sb/test_varstored_cert_flow.py @@ -15,10 +15,9 @@ # From --hosts parameter: # - host: XCP-ng host >= 8.3 # Master of a, at least, 2 hosts pool +# With a free disk # - hostB1: XCP-ng host >= 8.3 # This host will be joined and ejected from pool A, it means its state will be completely reinitialized from scratch -# From --sr-disk parameter: -# - a free disk on the first host. pytestmark = pytest.mark.default_vm('mini-linux-x86_64-uefi')