Skip to content

Enhance block device management - #498

Merged
glehmann merged 3 commits into
masterfrom
gln/block-device-enhancement-and-lun-deduplication-rqtm
May 29, 2026
Merged

Enhance block device management#498
glehmann merged 3 commits into
masterfrom
gln/block-device-enhancement-and-lun-deduplication-rqtm

Conversation

@glehmann

@glehmann glehmann commented Apr 24, 2026

Copy link
Copy Markdown
Member

Some block devices may be used to build other devices (mdadm, lvm) or be accessible on multiple hosts, but only usable on a single one at a time (fc, iscsi).

This PR is part of a tree containing 19 PRs:

  1. master
  2. storage: avoid removing the xva/vdi before going in the debugger #436master
  3. storage: test large volumes #437storage: avoid removing the xva/vdi before going in the debugger #436
  4. host: include mdadm RAID devices in disk detection #447storage: test large volumes #437
  5. Update randstream to 0.5.0 #446host: include mdadm RAID devices in disk detection #447
  6. storage: Avoid writing the whole device in coalesce tests #449Update randstream to 0.5.0 #446
  7. storage: avoid writing the whole device in migration tests #450storage: Avoid writing the whole device in coalesce tests #449
  8. xva/vdi: only write a small data amount in large volumes for faster tests #452storage: avoid writing the whole device in migration tests #450
  9. storage: test full device write #453xva/vdi: only write a small data amount in large volumes for faster tests #452
  10. storage: test that we can't create a vdi over its max allowed size #454storage: test full device write #453
  11. storage: add jobs for large volume tests #461storage: test that we can't create a vdi over its max allowed size #454
  12. skip large volume tests for zvol and nfsv4 #464storage: add jobs for large volume tests #461
  13. storage: add comprehensive tests for lvmohba storage repositories #470skip large volume tests for zvol and nfsv4 #464
  14. storage: free space for XVA import by destroying source VM first #471storage: add comprehensive tests for lvmohba storage repositories #470
  15. storage: limit data written per VDI with --write-volume-cap #481storage: free space for XVA import by destroying source VM first #471
  16. Add Packer configuration to build a minimal Alpine 3.23 UEFI VM for XCP-ng tests #523storage: limit data written per VDI with --write-volume-cap #481
  17. migration: create a xfs sr on the second host for intra/cross-pool migration #497storage: limit data written per VDI with --write-volume-cap #481
  18. "Enhance block device management" (this PR) → migration: create a xfs sr on the second host for intra/cross-pool migration #497
  19. Add VHD_MAX and QCOW2_MAX symbolic size constants #500Enhance block device management #498
  20. partially_populate_device: align span positions to block size for better performance #509Add VHD_MAX and QCOW2_MAX symbolic size constants #500

@glehmann
glehmann requested review from a team as code owners April 24, 2026 14:57
@glehmann glehmann changed the title gln/block-device-enhancement-and-lun-deduplication-rqtm Enhance block device management Apr 24, 2026
@glehmann
glehmann force-pushed the gln/block-device-enhancement-and-lun-deduplication-rqtm branch 2 times, most recently from f101a3f to ede9108 Compare April 29, 2026 17:38
@glehmann
glehmann requested a review from a team as a code owner April 29, 2026 17:38
@glehmann
glehmann force-pushed the gln/migration-test-cleanup-and-setup-lvmv branch from e0dd6d1 to edb6dbf Compare April 29, 2026 17:39
@glehmann
glehmann force-pushed the gln/migration-test-cleanup-and-setup-lvmv branch from edb6dbf to 36bfb8c Compare May 11, 2026 15:49
@glehmann
glehmann force-pushed the gln/block-device-enhancement-and-lun-deduplication-rqtm branch from ede9108 to 483adb0 Compare May 11, 2026 15:49
@glehmann
glehmann force-pushed the gln/migration-test-cleanup-and-setup-lvmv branch from 36bfb8c to bc1eab6 Compare May 12, 2026 13:16
@glehmann
glehmann force-pushed the gln/block-device-enhancement-and-lun-deduplication-rqtm branch from 483adb0 to 1960a93 Compare May 12, 2026 13:16
@glehmann
glehmann force-pushed the gln/migration-test-cleanup-and-setup-lvmv branch from bc1eab6 to be2cd1f Compare May 12, 2026 13:23
@Lankou66
Lankou66 requested review from Lankou66 and removed request for Millefeuille42 May 12, 2026 13:47
@Millefeuille42
Millefeuille42 self-requested a review May 12, 2026 13:58
@glehmann
glehmann force-pushed the gln/block-device-enhancement-and-lun-deduplication-rqtm branch from 8f39146 to 2710827 Compare May 12, 2026 17:08
@glehmann
glehmann force-pushed the gln/block-device-enhancement-and-lun-deduplication-rqtm branch 2 times, most recently from 9f88dc5 to 7cc39d2 Compare May 18, 2026 16:22
Comment thread lib/host.py Outdated
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 _disk_is_available_local(self, disk: str) -> bool:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Declare it before the method that uses it?

Comment thread conftest.py
ret = {host: list(_host_disks(host, cli_disks.get(host.hostname_or_ip)))
for host in pools_hosts_by_name_or_ip.values()
}
# Cross-host deduplication: a LUN in use on any host (same WWN) is unavailable on all hosts.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment about the commit message: I read it but it didn't explain what problem it solves. Only how it solves it.

@glehmann
glehmann force-pushed the gln/block-device-enhancement-and-lun-deduplication-rqtm branch 2 times, most recently from 3845b8f to 8b8e409 Compare May 19, 2026 15:55
@dinhngtu
dinhngtu removed the request for review from a team May 20, 2026 07:24
@glehmann
glehmann force-pushed the gln/block-device-enhancement-and-lun-deduplication-rqtm branch 3 times, most recently from ce5225e to e196d05 Compare May 21, 2026 14:07
@glehmann
glehmann requested a review from Millefeuille42 May 21, 2026 14:11
@glehmann
glehmann force-pushed the gln/block-device-enhancement-and-lun-deduplication-rqtm branch from e196d05 to 291e27e Compare May 21, 2026 14:16
@glehmann

Copy link
Copy Markdown
Member Author

I've reworked the block device again to mark disks with unused partitions as available.

@glehmann
glehmann force-pushed the gln/block-device-enhancement-and-lun-deduplication-rqtm branch 2 times, most recently from 1130de5 to dc44617 Compare May 22, 2026 15:29
@glehmann
glehmann requested a review from vxgmichel May 22, 2026 17:40

@vxgmichel vxgmichel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One suggestion, LGTM otherwise 👍

Comment thread .github/workflows/unit.yml Outdated
dev: false
- name: Create a dummy data.py
run: cp data.py-dist data.py
- run: pytest tests/unit/*.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe follow the convention for python test discovery by renaming tests/unit/rescan_block_devices_info.py to tests/unit/test_rescan_block_devices_info.py?

Suggested change
- run: pytest tests/unit/*.py
- run: pytest tests/unit

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I voluntarily kept them without the test_ prefix, so they are not run with the other tests, to not add more to the output in jenkins or in our terminal.

Do you think that's a problem?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alternatively, we could put then under /unit so that they're easier to run from CLI directly without having to think about the *.py stuff.

@vxgmichel vxgmichel May 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Apparently passing a test directory explicitly takes priority over --ignore, so another possible approach would be add the following.

diff --git a/pytest.ini b/pytest.ini
index 3deaa2c..bf08f83 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -1,5 +1,5 @@
 [pytest]
-addopts = -ra --maxfail=1 --ignore=contrib/
+addopts = -ra --maxfail=1 --ignore=contrib/ --ignore=tests/unit
 markers =
     # *** Markers that change test behaviour ***
     default_vm: mark a test with a default VM in case no --vm parameter was given.

This effectively disables the unit tests by default.

But @stormi suggestion of using a unit directory outside of tests is probably more explicit. It would require defining testpaths though:

diff --git a/pytest.ini b/pytest.ini
index 3deaa2c..a94601d 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -1,5 +1,6 @@
 [pytest]
-addopts = -ra --maxfail=1 --ignore=contrib/
+testpaths = tests
+addopts = -ra --maxfail=1
 markers =

@glehmann glehmann May 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Apparently passing a test directory explicitly takes priority over --ignore

I did that. Running the unit tests is now done with just pytest tests/unit.

glehmann added 3 commits May 29, 2026 13:37
Replace the BlockDeviceInfo TypedDict with a typed dataclass that includes
availability as a field, computed once at scan time from lsblk output
(mountpoint and device type).
Extend enumeration beyond local disks to also cover mdadm arrays and multipath
devices, using a single lsblk call.
Mark disk with unused partitions as available.
Update all callers to use attribute access instead of dict subscript.
Add a unit test suite and a CI workflow to run it.

Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
zpool destroy removes the pool but leaves ZFS member signatures and the
partition table on disk, causing the device to appear in use on the next
test run.

Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
Use WWNs in the disk fixture to:
- Mark a LUN unavailable on all hosts if it is already in use on any host
  (cross-host deduplication for shared FC/iSCSI LUNs).
- Deprioritize LUNs listed in LVMOHBA_DEVICE_CONFIG or LVMOISCSI_DEVICE_CONFIG
  so they are only selected when no other disk is available.

Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>
@glehmann
glehmann force-pushed the gln/block-device-enhancement-and-lun-deduplication-rqtm branch from dc44617 to 2d3046e Compare May 29, 2026 11:37
@glehmann
glehmann merged commit adf9f6f into master May 29, 2026
11 checks passed
@glehmann
glehmann deleted the gln/block-device-enhancement-and-lun-deduplication-rqtm branch May 29, 2026 12:32
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