Skip to content

fix: change controllerKey and unitNumber to *int32#3880

Merged
tenthirtyam merged 1 commit intovmware:mainfrom
skogta:topic/skogta/key
Oct 8, 2025
Merged

fix: change controllerKey and unitNumber to *int32#3880
tenthirtyam merged 1 commit intovmware:mainfrom
skogta:topic/skogta/key

Conversation

@skogta
Copy link
Copy Markdown
Contributor

@skogta skogta commented Oct 4, 2025

Description

Change the type if controllerKey and unitNumber to *int32 in CnsVolumeAttachDetachSpec.

Closes: #3883

How Has This Been Tested?

Complete govmomi testing logs:
Testing.rtf

    client_test.go:1226: Running shared disk tests
    client_test.go:1243: Attaching volume using the spec: {DynamicData:{} VolumeId:{DynamicData:{} Id:3d993b1a-32ea-4dae-80d8-91b1b693b5f0} Vm:VirtualMachine:vm-1022 DiskMode:independent_persistent Sharing:sharingMultiWriter ControllerKey:0xc00031c1dc UnitNumber:0xc00031c1d8}
    client_test.go:1268: Volume attached sucessfully with shared disk params. Disk UUID: 6000C299-269c-70ea-7ea7-87dbac4d3045
    client_test.go:1279: Detaching volume using the spec: {DynamicData:{} VolumeId:{DynamicData:{} Id:3d993b1a-32ea-4dae-80d8-91b1b693b5f0} Vm:VirtualMachine:vm-1022 DiskMode: Sharing: ControllerKey:<nil> UnitNumber:<nil>}
    client_test.go:1303: Volume detached sucessfully

CNS logs for attaching disk with controllerKey and UnitNumber:

2025-10-06T06:58:06.193Z INFO vsanvcmgmtd 114601 [vc@4413 sub="CnsVolMgr" opId="457b15ed"] Attaching volume with spec: (vim.cns.VolumeAttachDetachSpec) [
-->    (vim.cns.VolumeAttachDetachSpec) {
-->       volumeId = (vim.cns.VolumeId) {
-->          id = "3d993b1a-32ea-4dae-80d8-91b1b693b5f0"
-->       }, 
-->       vm = 'vim.VirtualMachine:vm-1022',
-->       diskMode = "independent_persistent",
-->       sharing = "sharingMultiWriter",  
-->       controllerKey = 1002, 
-->       unitNumber = 23
-->    }
--> ]

CNS logs for attaching disk without shared disk params:

2025-10-06T06:58:04.590Z INFO vsanvcmgmtd 114892 [vc@4413 sub="CnsVolMgr" opId="457b15e8"] DetachVolume with spec: (vim.cns.VolumeAttachDetachSpec) [
-->    (vim.cns.VolumeAttachDetachSpec) { 
-->       volumeId = (vim.cns.VolumeId) {
-->          id = "3d993b1a-32ea-4dae-80d8-91b1b693b5f0"
-->       }, 
-->       vm = 'vim.VirtualMachine:vm-1022',
-->    }
--> ]

Guidelines

Please read and follow the CONTRIBUTION guidelines of this project.

@skogta skogta marked this pull request as draft October 4, 2025 09:26
@skogta skogta marked this pull request as ready for review October 6, 2025 07:04
@tenthirtyam tenthirtyam marked this pull request as draft October 6, 2025 13:27
@tenthirtyam
Copy link
Copy Markdown
Contributor

CI is faling on the DCO - commits need to be signed.

@tenthirtyam tenthirtyam changed the title Change controllerKey and unitNumber to int32 [wip] change controllerKey and unitNumber to int32 Oct 6, 2025
Copy link
Copy Markdown
Contributor

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

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

Can you open an issue and link the issue to this pull request for traceability?

@skogta skogta marked this pull request as ready for review October 6, 2025 16:22
@skogta skogta force-pushed the topic/skogta/key branch 2 times, most recently from 4edb815 to cf47f95 Compare October 6, 2025 16:32
@skogta skogta changed the title [wip] change controllerKey and unitNumber to int32 Change controllerKey and unitNumber to int32 Oct 6, 2025
@skogta
Copy link
Copy Markdown
Contributor Author

skogta commented Oct 6, 2025

@tenthirtyam I have addressed your comments.

divyenpatel
divyenpatel previously approved these changes Oct 6, 2025
Copy link
Copy Markdown
Contributor

@divyenpatel divyenpatel left a comment

Choose a reason for hiding this comment

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

/approve

@tenthirtyam tenthirtyam changed the title Change controllerKey and unitNumber to int32 fix: change controllerKey and unitNumber to int32 Oct 6, 2025
@tenthirtyam tenthirtyam changed the title fix: change controllerKey and unitNumber to int32 fix: change controllerKey and unitNumber to *int32 Oct 6, 2025
Copy link
Copy Markdown
Contributor

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

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

@skogta - please sign-off with your @broadcom.com email instead of legacy @vmware.com.

Signed-off-by: Saloni Kogta <saloni.kogta@broadcom.com>
@skogta
Copy link
Copy Markdown
Contributor Author

skogta commented Oct 7, 2025

@prziborowski @tenthirtyam can I please get an approval? I have addressed your comments.

@tenthirtyam tenthirtyam merged commit 5ef2d64 into vmware:main Oct 8, 2025
11 checks passed
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.

UnitNumber and ContollerKey for attaching volumes in int64

4 participants