Skip to content

Conversation

ydirson
Copy link
Contributor

@ydirson ydirson commented Jun 18, 2025

This replaces the existing disk fixtures, which rely on the assumption that we can take a dom0 device name on the pool master, and expect to find it across the whole pool with the same meaning. This was causing issues for GlusterFS and Linstor tests when some hosts in the test pool get their disk device names swapped after a reboot.

The old mechanism used a mandatory --sr_disk flag to control the behavior of various sr_disk* fixtures, with little code-sharing. The new one is based on a central disks fixture to properly introspect the pool's disks, which comes with its own --disks flag to filter which disks to expose to the tests, but by default exposes all of them. It then uses additional fixtures to filter disks using different criteria (for now: availability and blocksize of the disk), and then some pool-level fixtures allowing testing of distributed SR without risking to (try to) use the wrong disk.

Builds on:

@ydirson ydirson changed the title Diskfeatures rework Disk fixtures rework Jun 18, 2025
@ydirson ydirson force-pushed the disks-rework branch 6 times, most recently from ac95623 to 1f72326 Compare June 19, 2025 16:41
@ydirson
Copy link
Contributor Author

ydirson commented Jun 19, 2025

Last push:

  • adds a first --disk implementation
  • fixes unused_disks ignoring any filtering done in disks
  • extends a bit the use of the type aliases
  • notes in WIP that pool_with_unused_disk only applies to pool A, we'll need
    • to make that more explicit
    • something better for linstor pool-join

@ydirson ydirson force-pushed the disks-rework branch 4 times, most recently from 0c6c08c to 6642deb Compare June 24, 2025 12:58
@ydirson ydirson requested a review from Nambrok June 24, 2025 12:58
@ydirson ydirson force-pushed the disks-rework branch 3 times, most recently from 18ad9aa to c1c6bfb Compare June 26, 2025 08:03
@ydirson ydirson marked this pull request as ready for review June 26, 2025 08:09
@ydirson ydirson force-pushed the disks-rework branch 3 times, most recently from 21fe576 to 12f840e Compare August 25, 2025 08:58
@ydirson
Copy link
Contributor Author

ydirson commented Aug 25, 2025

Last push squashes fixups from fixing issues discovered during CI runs.

# 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"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have a function to get a device path from a BlockDeviceInfo since we do a lot for tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure .name() brings a log over ["name"]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't talking about only name but to get the device path too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I get what you mean, but that sounds like an improvement that can be done later?

ydirson added 18 commits August 28, 2025 10:34
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 <[email protected]>
This method was not used since 7435c99
(largeblock: add tests largeblock driver + fixture), this repurposes it
for a new life.

Signed-off-by: Yann Dirson <[email protected]>
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 <[email protected]>
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 <[email protected]>
Only used for everything ZFS

Signed-off-by: Yann Dirson <[email protected]>
Signed-off-by: Yann Dirson <[email protected]>
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 <[email protected]>
@ydirson ydirson merged commit 03bc212 into master Aug 29, 2025
9 checks passed
@ydirson ydirson deleted the disks-rework branch August 29, 2025 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants