Memory checkbox will not show when VM is not up

[borod108]

When a VM is powered off there should not be a "save memory" checkbox:

When a VM is powered off there should be a "save memory" checkbox:

When creating a snapshot there is not reason to show
the option to save run time memory if the VM is down.

https://bugzilla.redhat.com/show_bug.cgi?id=1375850

[borod108]

@masayag please review

[masayag]

👍 LGTM

[borod108]

@miq-bot assign @durandom

[durandom]

👍 from the prvoiders side

@martinpovolny can you check the UI part and merge if ok?
@miq-bot assign @martinpovolny
@miq-bot add_label ui, providers/rhevm

[durandom]

@borod108 please resolve the confilict

[miq-bot]

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

[borod108]

@durandom conflict resolved

[borod108]

Create snapshot has memory checkbox, even though VM is Down.

[miq-bot]

Checked commit borod108@de813a1 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 0 offenses detected
Everything looks good. ⭐

[borod108]

@durandom again had a strange CI error https://travis-ci.org/ManageIQ/manageiq/jobs/177665079#L693
which I did not get locally. Closed and reopened to re-run.

[durandom]

@martinpovolny I'm 👍 please merge if you are ok too

[borod108]

@martinpovolny hi, can we merge this?

[durandom]

@himdel I'm good from the providers side, if you are from the UI, can you merge this?

[himdel]

@borod108 please update the description to include a screenshot and instructions where to find this in the UI.

(Also seems like you left a part of the PR template in the description..)

Not merging before I can test it :) (though it does look good)

[borod108]

@himdel added images is that ok?

[himdel]

@borod108 thanks, I guess I can try finding it by the images. But if you could also add real instructions (go there, click that, ..), would be even better :)

[borod108]

@himdel sure you are right. Will this be ok:
Setup: a CFME with RHV (v4.0 or higher) provider that has two vms one powered on and another powered off (or one vm and then power it off/on and refresh)

1. From the side menu go to Compute -> Infrastructure -> Virtual Machines

2. In the Vms & Templates choose the provider and select the VM that is powered on.

3. In the "Properties" section of the summery page click on "Snapshots"

4. In the "Snaphots" screen of the VM click the "Create a new snapshot for this VM" (a circle with a plus sign in it at the top toolbar)

5. a. The "Adding a new Snapshot" screen should have a checkbox with the label "Snapshot VM memory"
   Repeat step 1-4 with the snapshot that is powered off.
   b. The "Adding a new Snapshot" screen should not have a checkbox with the label "Snapshot VM memory"

[himdel]

@borod108 thanks, that really helps! :)

So .. the only question that remains is this..

• "Snapshots" in summary page are only clickable if @record.supports_snapshosts?
• supports_snapshots? is false for vm_or_template, and overridden only for openstack cloud & vmware infra
• the "Snapshot VM memory" checkbox is disabled only when @record.snapshotting_memoery_allowed? is false .. which can happen only for redhat infra VMs
• so .. as the code is, this PR can't possibly affect anything, can it?

[durandom]

@himdel

supports_snapshots? is false for vm_or_template, and overridden only for openstack cloud & vmware infra

it's actually only true for rhev?!

❯ g --no-pager grep 'supports :snapshots'
app/models/manageiq/providers/redhat/infra_manager/vm/operations/snapshot.rb:    supports :snapshots do

Could it be, you grepped for something else?

[borod108]

@himdel as @durandom wrote it is overridden for rhv to just using the supports_feature_mixin.
So this code effects rhv provider.

[himdel]

@durandom, @borod108 OK, thanks, I'll check further. I was grepping for the supports_snapshots? method.

But seems like we shouldn't both be defininng the method, overriding it, and creating it via supports at the same time..

Either way, I don't have any RHEV VM that'd have the button (snapshots in the summary) clickable..

[durandom]

Oh, @himdel you are absolutely right. This slipped my review on #11418
We should asap refactor supports_snapshot? to use the SupportsFeatureMixin
@gauravaradhye can you do this?

@himdel can you merge this as is or do you want to test it out with a VM?

[himdel]

@durandom Sure, merged, as long as it can happen.. :)

[simaishi]

Euwe backport details:

$ git log -1
commit 3bfba1edcdbd999482b383b406d9a394935c1866
Author: Martin Hradil <himdel@seznam.cz>
Date:   Thu Dec 8 06:35:27 2016 -0500

    Merge pull request #12678 from borod108/bugs/1375850memory_checkbox
    
    Memory checkbox will not show when VM is not up
    (cherry picked from commit d05b5f28253b6bf2a8c5d78e829d48092035fe7f)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1403981