Skip to content

Fix SCSI max disk limit to support PVSCSI controller capacity#2698

Draft
Dwayne75 wants to merge 3 commits intovmware:mainfrom
Dwayne75:paravirtual_max_disk_fix
Draft

Fix SCSI max disk limit to support PVSCSI controller capacity#2698
Dwayne75 wants to merge 3 commits intovmware:mainfrom
Dwayne75:paravirtual_max_disk_fix

Conversation

@Dwayne75
Copy link

@Dwayne75 Dwayne75 commented Mar 17, 2026

Summary

  • The SCSI disk unit number validation, bus/unit mapping in assignDisk(), and reverse mapping in findControllerInfo() all hardcoded 15 usable units per controller, which is only correct for LSI Logic / LSI Logic SAS controllers
  • PVSCSI (ParaVirtual) controllers support 64 devices per controller (63 usable after reserving one for the controller)
  • Added scsiUsableUnitsPerController() helper that returns the correct limit based on scsi_type, and updated DiffGeneral, assignDisk, and findControllerInfo to use it

Test plan

  • Verify existing LSI Logic / LSI Logic SAS configurations still enforce the 15 units-per-controller limit
  • Verify PVSCSI configurations now allow up to 63 units per controller
  • Verify disk assignment correctly maps unit numbers to controllers for both SCSI types
  • Confirm error messages include the SCSI controller type for easier debugging

🤖 Generated with Claude Code

The SCSI disk unit number validation, bus/unit mapping, and controller
info lookup all hardcoded 15 usable units per controller, which is only
correct for LSI Logic/SAS. PVSCSI controllers support 64 devices (63
usable). Added scsiUsableUnitsPerController() helper and updated
DiffGeneral, assignDisk, and findControllerInfo to use controller-type
aware limits.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Dwayne75 Dwayne75 requested a review from a team as a code owner March 17, 2026 23:02
Copilot AI review requested due to automatic review settings March 17, 2026 23:02
@github-actions github-actions bot added provider Provider needs-review Needs Review size/s Relative Sizing: Small labels Mar 17, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates SCSI disk unit-number validation and controller/unit mapping so that PVSCSI controllers can use their full supported device capacity (63 usable slots per controller), instead of being limited to the LSI Logic/SAS default (15 usable).

Changes:

  • Add scsiUsableUnitsPerController(scsi_type) helper to determine per-controller usable unit capacity.
  • Update DiffGeneral() SCSI unit-number maximum validation to use the computed usable-unit limit and include scsi_type in the error message.
  • Update SCSI bus/unit mapping in assignDisk() and reverse mapping in findControllerInfo() to use the computed usable-unit limit.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Dwayne75 Dwayne75 requested a review from Copilot March 17, 2026 23:24
@github-actions github-actions bot added the size/m Relative Sizing: Medium label Mar 17, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates SCSI disk unit-number validation and controller/unit mapping so the provider correctly supports PVSCSI’s larger device capacity (63 usable disk slots per controller vs 15 for LSI Logic / LSI Logic SAS).

Changes:

  • Add scsiUsableUnitsPerController(scsiType) helper to centralize per-controller usable slot limits.
  • Update DiffGeneral(), assignDisk(), and findControllerInfo() to use the new per-SCSI-type usable unit count instead of a hardcoded 15.
  • Add a unit test covering scsiUsableUnitsPerController() for PVSCSI and LSI controller types.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
vsphere/internal/virtualdevice/virtual_machine_disk_subresource.go Replaces hardcoded SCSI unit math with SCSI-type-aware limits across validation, assignment, and reverse mapping.
vsphere/internal/virtualdevice/virtual_machine_disk_subresource_test.go Adds unit test for the new helper that returns per-controller usable SCSI slots.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tenthirtyam tenthirtyam requested a review from spacegospod March 18, 2026 02:03
@spacegospod
Copy link
Contributor

Hey @Dwayne75 thanks for your contribution.

There's a couple of items in the backlog specifically about SCSI controllers and max devices.
#2086
#1427

Please make sure to run all acceptance tests for r/virtual_machine and add new ones that cover your functionality.
Be especially careful for cases such as VMs with a mix of controllers, a mix of scsi controllers only, cloning and importing.

Instead of relying on the global scsi_type setting, determine the
controller sub-type from the actual controller device object. This
handles mixed-controller VMs correctly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Dwayne75
Copy link
Author

hey @spacegospod, I fixed that issue mentioned by copilot, I ran all the tests I can, my vsphere is a very controlled environment and I do not currently have the ability to run the full acceptance tests.

@Dwayne75
Copy link
Author

#1427 my PR should address this one from what I can tell. #2086 this seems to be a different problem related to the id 7 reservation my PR does not fix or make this worse.

@Dwayne75
Copy link
Author

Dwayne75 commented Mar 18, 2026

hmm also the documentation I can find says it supports 64 addresses with 7 being reserved. However I can clearly set a disk with unit id 64 in the UI.....
image
image

So now I am not sure if it actually supports 63 or 64 total disks.

@Dwayne75 Dwayne75 marked this pull request as draft March 19, 2026 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review Needs Review provider Provider size/m Relative Sizing: Medium size/s Relative Sizing: Small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants