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

Don't send 'next_run=false' to oVirt 3.6 #13677

Merged
merged 1 commit into from Feb 2, 2017
Merged

Don't send 'next_run=false' to oVirt 3.6 #13677

merged 1 commit into from Feb 2, 2017

Conversation

jhernand
Copy link
Contributor

In oVirt virtual machines have two sets of settings: the settings used
by the current running virtual machine, and the settings that will be
used the next time the virtual machine is started. To select which one
to update it is necessary to use the 'next_run' parameter. For example,
to update the memory that will be used the next time is started the
API request should be like this:

PUT /ovirt-engine/api/vms/{vm:id};next_run=true

The value 'false' should tell the server to update the current running
configuration. But in version 3.6 of oVirt it is just ignored, the mere
presence of the 'next_run' parameter is interpreted as a request to
modify the next start settings, regardless of its value.

Currently the oVirt provider uses 'next_run=false' explicitly, and as a
result it fails to update the current settings. This patch changes the
provider so that it doesn't send the 'next_run' parameter when its value
is 'false'.

https://bugzilla.redhat.com/1356468

@jhernand
Copy link
Contributor Author

@masayag, @borod108 please review.

@masayag
Copy link
Contributor

masayag commented Jan 29, 2017

@jhernand tests fails due to the change.

In oVirt virtual machines have two sets of settings: the settings used
by the current running virtual machine, and the settings that will be
used the next time the virtual machine is started. To select which one
to update it is necessary to use the 'next_run' parameter. For example,
to update the memory that will be used the next time is started the
API request should be like this:

  PUT /ovirt-engine/api/vms/{vm:id};next_run=true

The value 'false' should tell the server to update the current running
configuration. But in version 3.6 of oVirt it is just ignored, the mere
presence of the 'next_run' parameter is interpreted as a request to
modify the next start settings, regardless of its value.

Currently the oVirt provider uses 'next_run=false' explicitly, and as a
result it fails to update the current settings. This patch changes the
provider so that it doesn't send the 'next_run' parameter when its value
is 'false'.

https://bugzilla.redhat.com/1356468
@jhernand
Copy link
Contributor Author

@masayag you are right, I forgot to update the tests. It should be fixed now.

@miq-bot
Copy link
Member

miq-bot commented Jan 30, 2017

Checked commit https://github.com/jhernand/manageiq/commit/21d820eac6c929f8c552776649b072cbaadbe2f6 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 🏆

@jhernand
Copy link
Contributor Author

@durandom can you review/merge?

Copy link
Member

@durandom durandom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@miq-bot assign @agrare

@agrare please merge

@@ -157,7 +157,7 @@ def update_vm_memory(vm, virtual)
state = vm.attributes.fetch_path(:status, :state)
if state == 'up'
vm.update_memory(virtual, guaranteed, :next_run => true)
vm.update_memory(virtual, nil, :next_run => false)
vm.update_memory(virtual, nil)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhernand does it make sense to toggle on ovirt versions +3.6? Or are we assuming the absence of this will always be next_run => false ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

next_run=false is the default in all versions of oVirt, so there is no need to toggle.

@agrare agrare merged commit 7194f99 into ManageIQ:master Feb 2, 2017
@agrare agrare added this to the Sprint 54 Ending Feb 13, 2017 milestone Feb 2, 2017
simaishi pushed a commit that referenced this pull request Feb 2, 2017
@simaishi
Copy link
Contributor

simaishi commented Feb 2, 2017

Euwe backport details:

$ git log -1
commit a44104e64ce778dc3192c904e2ca69c5509f1c8f
Author: Adam Grare <agrare@redhat.com>
Date:   Thu Feb 2 08:22:05 2017 -0500

    Merge pull request #13677 from jhernand/do_not_send_next_run_false_to_ovirt
    
    Don't send 'next_run=false' to oVirt 3.6
    (cherry picked from commit 7194f99b79815413e6cf75d42b6e078f678ff55f)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1404316

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants