Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] VirtualDeviceList.AssignController may generate positive device key #2881

Closed
Syuparn opened this issue Jun 21, 2022 · 1 comment · Fixed by #2882
Closed

[BUG] VirtualDeviceList.AssignController may generate positive device key #2881

Syuparn opened this issue Jun 21, 2022 · 1 comment · Fixed by #2882

Comments

@Syuparn
Copy link
Contributor

Syuparn commented Jun 21, 2022

Describe the bug

VirtualDeviceList.AssignController should generate a negative random device key so that it will not collide with existing devices' keys. However, it sometimes generates positive key.

To Reproduce

  1. run code below
// key has not been set to disk yet (disk.Key == 0)
disk := &types.VirtualDisk{
	VirtualDevice: types.VirtualDevice{},
}

l := object.VirtualDeviceList{disk}
c, _ := l.CreateIDEController()

// AssignContoller gives a new random key to device if it does not have a key yet
l.AssignController(disk, c.(types.BaseVirtualController))

fmt.Printf("disk device key: %d\n", disk.Key)

Expected behavior
A clear and concise description of what you expected to happen.

AssignController always gives negative keys to the device.

Affected version
Please provide details on the version used, e.g. release tag, commit, module version, etc.

  • govmomi v0.28.0

Screenshots/Debug Output

AssignController sometimes gives a positive key to the device.

// key is positive
disk device key: 1697971134

Additional context
Add any other context about the problem here.

This is same issue as #2536, CreateEthernetCard sometimes generated a positive device key.

Generated key is expected to be negative (#2156), but type convertion from uint32 to int32 may change the key's sign.

@Syuparn
Copy link
Contributor Author

Syuparn commented Jun 21, 2022

Logic in #2569 can be used for this too. I will make a PR.

Syuparn added a commit to Syuparn/govmomi that referenced this issue Jun 21, 2022
Closes: vmware#2881
Signed-off-by: syuparn <s.hello.spagetti@gmail.com>
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 a pull request may close this issue.

1 participant