Add internal disk functions #272
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Vaidotas Bauzys <[email protected]>
vbauzys
requested review from
dataclouder,
lvirbalas and
Didainius
and removed request for
dataclouder and
lvirbalas
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Didainius
reviewed
Dec 5, 2019
govcd/vm.go
Outdated
| // GetInternalDisk returns a valid *types.DiskSettings if it exists. | ||
| // If it doesn't, returns nil and ErrorEntityNotFound or other err. | ||
| func (vm *VM) GetInternalDisk(diskId string) (*types.DiskSettings, error) { | ||
| if vm.VM.HREF == "" { |
There was a problem hiding this comment.
HREF is validated, but it is not used anywhere. Also what about the "refresh" parameter?
There was a problem hiding this comment.
Getting me to thinking, not to have such option at all. User won't know details and get's to risk to messup no refreshing when needed and deleting disks which he wasn't expecting
There was a problem hiding this comment.
I added refresh handling. I made it implicit due big risk leave for user and he would forget to pass correct value.
Signed-off-by: Vaidotas Bauzys <[email protected]>
lvirbalas
requested changes
Dec 5, 2019
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
lvirbalas
reviewed
Dec 5, 2019
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
dataclouder
suggested changes
Dec 9, 2019
govcd/vm.go
Outdated
| } | ||
| } | ||
|
|
||
| if diskId == "" { |
There was a problem hiding this comment.
We should check for valid ID before refreshing.
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Closed
Signed-off-by: Vaidotas Bauzys <[email protected]> # Conflicts: # CHANGELOG.md
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
dataclouder
suggested changes
Dec 18, 2019
govcd/vm.go
Outdated
| return nil | ||
| } | ||
|
|
||
| // GetInternalDiskById returns a valid *types.DiskSettings if it exists. |
There was a problem hiding this comment.
Suggested change
| // GetInternalDiskById returns a valid *types.DiskSettings if it exists. | |
| // GetInternalDiskById returns a *types.DiskSettings if one exists. |
govcd/vm.go
Outdated
Comment on lines
993
to
994
| // types.VmSpecSection requires consist of all disk entities which exist and not updated, | ||
| // as also new ones or changed ones. Returns new disk ID and error. |
There was a problem hiding this comment.
Please rephrase. I don't understand this comment.
govcd/vm.go
Outdated
Comment on lines
1017
to
1018
| // types.VmSpecSection requires consist of all disk entities which exist and not updated, | ||
| // as also new ones or changed ones. |
|
|
||
| check.Assert(disk.StorageProfile.HREF, Equals, storageProfile.HREF) |
There was a problem hiding this comment.
Please add a check for ID correspondence
govcd/vm_test.go
Outdated
| check.Assert(disk.AdapterType, Equals, diskSettings.AdapterType) | ||
| } | ||
|
|
||
| func (vcd *TestVCD) createInternalDisk(check *C, busNumber int) (*VM, types.Reference, *types.DiskSettings, string, error) { |
There was a problem hiding this comment.
Please add a comment with the function purpose and the description of the returned values
Signed-off-by: Vaidotas Bauzys <[email protected]>
Didainius
reviewed
Dec 20, 2019
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Didainius
reviewed
Jan 7, 2020
dataclouder
reviewed
Jan 7, 2020
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Didainius
approved these changes
Jan 8, 2020
Signed-off-by: Vaidotas Bauzys <[email protected]>
Signed-off-by: Vaidotas Bauzys <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes needed for https://github.com/terraform-providers/terraform-provider-vcd/pull/412
Implement functions which helps work with internal disk of VM.