Skip to content

fix: change controllerKey and unitNumber to *int32#3880

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

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

Conversation

@skogta

@skogta skogta commented Oct 4, 2025

Copy link
Copy Markdown
Contributor

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

@tenthirtyam tenthirtyam 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.

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

skogta commented Oct 6, 2025

Copy link
Copy Markdown
Contributor Author

@tenthirtyam I have addressed your comments.

divyenpatel
divyenpatel previously approved these changes Oct 6, 2025

@divyenpatel divyenpatel 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.

/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

@tenthirtyam tenthirtyam 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.

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

Comment thread cns/client_test.go Outdated
Signed-off-by: Saloni Kogta <saloni.kogta@broadcom.com>
@skogta

skogta commented Oct 7, 2025

Copy link
Copy Markdown
Contributor Author

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

Comment thread cns/client_test.go
@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