This repository has been archived by the owner on Sep 10, 2024. It is now read-only.
Add support for matrix parameters to update memory #72
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
The oVirt API supports matrix parameters that alter the meaning of some operations, including updates. For example, the operation to update a virtual machine can receive an optional 'next_run' parameter that controls if the update is applied to the state of the current running virtual machine (the default) or only to the next execution of the virtual machine, after rebooting it. For example, to update the memory of for the next boot of the virtual machine the following request should be used: PUT /ovirt-engine/api/vms/123;next_run=true This mechanism isn't currently supported by this gem. That patch adds support for it. Signed-off-by: Juan Hernandez <[email protected]>
|
LGTM 👍 |
|
Looks good to me 👍 |
durandom
approved these changes
Nov 14, 2016
| # @param memory [Integer] The virtual memory assigned to the virtual machine, in bytes. If it is `nil` then | ||
| # the virtual memory won't be updated. | ||
| # | ||
| # @param guaranteed [Integer] The ammount of physical memory reserved for the virtual machine, in bytes. If |
There was a problem hiding this comment.
Fixed in the next version.
bdunne
suggested changes
Nov 17, 2016
| # | ||
| def update_memory(memory, guaranteed, matrix = {}) | ||
| update!(matrix) do |xml| | ||
| unless memory.nil? |
There was a problem hiding this comment.
if memory would be easier to follow
| unless memory.nil? | ||
| xml.memory memory | ||
| end | ||
| unless guaranteed.nil? |
| @@ -194,6 +219,110 @@ def expected_data(element) | |||
| end | |||
| end | |||
|
|
|||
| context '#update_memory' do | |||
| it 'updates only `memory` if `guaranteed` is nil' do | |||
There was a problem hiding this comment.
Please only use single and double quotes.
| vm.update_memory(memory, guaranteed) | ||
| end | ||
|
|
||
| it 'updates only `guaranteed` if `memory` is nil' do |
| vm.update_memory(memory, guaranteed) | ||
| end | ||
|
|
||
| it "adds the `next_run=true` matrix parameter correctly" do |
Currently the methods 'memory=' and 'memory_reserve=' of the 'Vm' class don't support adding matrix parameters, and thus they don't support adding the 'next_run' parameter which is very relevant when updating virtual machines. This patch adds a ne method named 'update_memory' that can update simultaneously the virtual and guaranteed memory, and that supports matrix parameters. The 'memory=' and 'memory_reserve=' methods are then re-implemented as simple calls to the new method. Signed-off-by: Juan Hernandez <[email protected]>
|
@bdunne this new version addresses your comments. |
|
Checked commits https://github.com/jhernand/ovirt/compare/440bd13a9a63a4fc7e6e879f404e93313d20af02~...e579b2c100719e1c1abed5d1e42b86685a4769b5 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 lib/ovirt/version.rb
|
bdunne
reviewed
Nov 18, 2016
| @@ -1,3 +1,3 @@ | |||
| module Ovirt | |||
| VERSION = "0.13.0" | |||
| VERSION = "0.14.0" | |||
There was a problem hiding this comment.
Please don't change the version in your PR.
|
@bdunne removed the commit that changes the version. |
bdunne
approved these changes
Nov 18, 2016
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Currently the methods
memory=andmemory_reserve=of theVmclassdon't support adding matrix parameters, and thus they don't support
adding the
next_runparameter which is very relevant when updatingvirtual machines. This patch adds a new method named
update_memorythat can update simultaneously the virtual and guaranteed memory, and
that supports matrix parameters. The
memory=andmemory_reserve=methods are then re-implemented as simple calls to the new method.
https://bugzilla.redhat.com/1356468
https://bugzilla.redhat.com/1356475