-
Notifications
You must be signed in to change notification settings - Fork 941
govc: add device.sata.add
#3936
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
Conversation
42c817e to
24ae76c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a new device.sata.add command to govc for adding SATA controllers to virtual machines.
- Implements the
device.sata.addcommand with support for adding SATA (AHCI) controllers - Registers the new command in the govc CLI and updates documentation
- Adds test coverage for the new SATA controller functionality
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cli/device/sata/add.go | New file implementing the SATA controller add command |
| govc/main.go | Registers the new SATA device package |
| govc/USAGE.md | Documents the new device.sata.add command |
| govc/test/device.bats | Adds test case for SATA controller functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9db8ed8 to
19d4f7f
Compare
spacegospod
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
- Added 'device.sata.add' command to add SATA controllers to virtual machines. - Registered 'device.sata.add' in the 'govc' CLI entry point. - Updated documentation and test suite to cover the new command. Signed-off-by: Ryan Johnson <[email protected]>
19d4f7f to
d6507f0
Compare
| err = vm.AddDevice(ctx, d) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: consider changing this to if err := vm.AddDevice(ctx, d); err != nil { ... }?
Description
Closes: #3780
How Has This Been Tested?
make govc-testand manually.