Bug 993639 - During create snapshot rest “Create Snapshot” buttons should become grayed out
During create snapshot rest “Create Snapshot” buttons should become grayed out
Status: CLOSED WONTFIX
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine-webadmin-portal (Show other bugs)
3.3.0
x86_64 Linux
unspecified Severity medium
: ---
: 3.5.0
Assigned To: Arik
meital avital
virt
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-06 06:45 EDT by vvyazmin@redhat.com
Modified: 2014-09-28 02:38 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-03-31 04:22:32 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
## Logs rhevm, vdsm, libvirt, thread dump, superVdsm (4.68 MB, application/x-gzip)
2013-08-06 06:45 EDT, vvyazmin@redhat.com
no flags Details

  None (edit)
Description vvyazmin@redhat.com 2013-08-06 06:45:02 EDT
Created attachment 783263 [details]
## Logs rhevm, vdsm, libvirt, thread dump, superVdsm

Description of problem:
During create snapshot rest “Create Snapshot” buttons should become grayed out

Version-Release number of selected component (if applicable):
RHEVM 3.3 - IS8 environment:

RHEVM:  rhevm-3.3.0-0.13.master.el6ev.noarch
VDSM:  vdsm-4.12.0-rc3.13.git06ed3cc.el6ev.x86_64
LIBVIRT:  libvirt-0.10.2-18.el6_4.9.x86_64
QEMU & KVM:  qemu-kvm-rhev-0.12.1.2-2.355.el6_4.5.x86_64
SANLOCK:  sanlock-2.6-2.el6.x86_64
PythonSDK:  rhevm-sdk-python-3.3.0.8-1.el6ev.noarch

How reproducible:
100%

Steps to Reproduce:
Scenario A:
Create VM with one or multiple disk
Power on relevant VM
Wait till your VM get status “Up”
Right click on relevant VM
On menu select “Create Snapshot”

Scenario B:
Create VM with one or multiple disk
Power on relevant VM
Wait till your VM get status “Up”
On “Virtual Machines” main-tab, select VM
“Create Snapshot”(from main toolbar)

Actual results:
On both scenarios, option “Create Snapshot” stay active (not grayed out). except option “Create” on sub-tub “Snapshots” in “Details Pane”. See print screen attached.

Expected results:
When pressing create snapshot from any of the buttons, rest are becoming grayed out.

Impact on user:
none

Workaround:
none

Additional info:

Functionality of  “Create Snapshot” work OK.

/var/log/ovirt-engine/engine.log

/var/log/vdsm/vdsm.log
Comment 1 Arik 2013-08-28 09:48:36 EDT
In order to solve this bug we'll need to periodically query for the snapshots when the grid of the VMs is displayed. That's different than what we're used to do - which is to query periodically only the data that is presented in the grid.

The data that is not presented in the grid is queried only when it is needed - so in this case, only when the user press the 'create snapshot' button (and if there's already snapshot creation operation in the background the user will receive an error).

In the snapshots sub-tab the snapshots, as the data that is presented in the grid, is queried periodically so we can disable the 'create' button if one of them is lock (which means snapshot is made in the background).

Einav, can I close this one as 'not a bug' since that's the way our UI works or do you think the snapshots should be periodically queried as well when the VMs grid is displayed?
Comment 2 Michal Skrivanek 2013-08-30 07:41:41 EDT
without bigger ux changes I don't think we can solve this...
Comment 3 Einav Cohen 2013-08-30 14:33:45 EDT
(In reply to Arik from comment #1)
> In order to solve this bug we'll need to periodically query for the
> snapshots when the grid of the VMs is displayed. That's different than what
> we're used to do - which is to query periodically only the data that is
> presented in the grid.

to be accurate: we will need to periodically query for the snapshots of the selected VM in the VMs main grid (probably via the UpdateActionAvailability() method in the VmListModel); in case there are no VMs selected or there are two or more selected VMs in the grid, or in case the VMStatus doesn't allow snapshot creation at the moment - we won't need to query anything, since the action won't be available by definition [1]

> 
> The data that is not presented in the grid is queried only when it is needed
> - so in this case, only when the user press the 'create snapshot' button
> (and if there's already snapshot creation operation in the background the
> user will receive an error).
> 
> In the snapshots sub-tab the snapshots, as the data that is presented in the
> grid, is queried periodically so we can disable the 'create' button if one
> of them is lock (which means snapshot is made in the background).
> 
> Einav, can I close this one as 'not a bug' since that's the way our UI works
> or do you think the snapshots should be periodically queried as well when
> the VMs grid is displayed?

Again: not constantly while the VMs grid is displayed, and not on all VMs in the grid; it should theoretically be done only when there is (exactly) one item selected in the grid and when the VMStatus allows snapshot creation at the moment, and only on the selected VM in the grid [1]

Having said that: AFAIK - we have never retrieved information from the server in any  UpdateActionAvailability() context. Not sure if it is such a good idea to do so, but it is theoretically possible.

a couple of notes:

- The "Create Snapshot" button in the main-tab seems a bit strange to me; I found out that it was added in the context of Bug 909930, but I am not sure that it is really imperative/correct to have it there, just like we don't have "Add NIC", "Add Disk", etc. as main-tab actions (although theoretically possible); in general, when we have a one-to-many relation between business entities, relevant actions are done via the relevant sub-tab. 

- I think it is important to keep the behavior consistent for the main-tab button and the sub-tab button (i.e. if the main-tab button is disabled, the sub-tab button should be disabled as well - their states should be constantly identical); it can be quite confusing for a user to select a VM, navigate to its Snapshots tab, and see the "Create Snapshot" button enabled in the main tab while the "Create Snapshot" button is disabled in the sub-tab.

These are the possible solutions in my view, ordered by my preference (maybe you can shoot an e-mail to engine-devel/users to poll for additional opinions):

(a) Remove the "Create Snapshot" button from the VMs main tab (keep it only in the Snapshots sub-tab), and disable the "Create Snapshot" button in the sub-tab according to the snapshots data that we are already retrieving periodically anyway.

(b) do nothing (i.e. button stays enabled in both main-tab and sub-tab, even if a snapshot is being created in the background).

(c) add a read-only Boolean field to the VM business entity / VM view in the data-base ("AreSnapshotsBeingCreatedNow") which will be 'true' if at least one snapshot is locked and 'false' otherwise, and disable the "Create Snapshot" button according to this new field's value (similar to (d) below, only the desired data is incorporated into the data that we are already retrieving periodically anyway, i.e. the VMs list).

(d) query snapshots status periodically when necessary for both VMs main tab (see [1]) and Snapshots sub-tab (which is done anyway), and disable the "Create Snapshot" buttons accordingly.

(e) like (a), without removing the "Create Snapshot" main-tab button (my least favorite, since will result in inconsistency between main-tab button state and sub-tab button state).

----

[1] in order to disable the "create snapshot button" while there are other snapshots for this VM being created in the background, the relevant section in the UpdateActionAvailability() method of the VmListModel would need to look something like:

...
getCreateSnapshotCommand().setIsExecutionAllowed(
   items.size() == 1  // ** condition 1 **
   && VdcActionUtils.CanExecute(...)  // ** condition 2 **
   && allSnapshotsAreNotLocked-CheckInServer(items[0]));
...

so practically, the call to the server to check the snapshots status ("allSnapshotsAreNotLocked-CheckInServer(items[0])") will be called only when "condition 1" and "condition 2" have passed (i.e. exactly one VM selected in the grid and current VMStatus is legal for snapshot creation), and only on a single VM (again, see "condition 1").
Comment 4 Daniel Erez 2013-09-01 05:38:53 EDT
Adding a "Create Snapshot" button to VMs main-tab has been highly requested on Captain KVM blog [1]: "...Easier would be to make “snapshot” a menu option when you ‘right click’ a VM...".

I understand it appears inconsistent having this action in a main-tab, however, in case of creating a VM snapshot, I tend to agree it makes sense and useful (after all, it's a VM action just like Export/Migrate/etc).

Regarding graying-out the button, there's already a check for number of selected items (i.e. the action is enabled only when one VM is selected). There's also a verification that the VM's status is applicable for snapshot creation (e.g. VMStatus.ImageLocked/VMStatus.MigratingFrom/etc).

I prefer not to query the backend on UpdateActionAvailability, especially on a main-tab, as we refrain any queries other than retrieving the items (which is probably already too much - as it happens every 5 seconds by default...).

So, although we must rely on the business entities, I'm really not sure we want to add a flag such as "AreSnapshotsBeingCreatedNow"; it would indeed be an optimal solution to have it but I'm afraid the performance penalty might be too harsh.

In short, consequent upon the infrastructural limitations, I'm in favor of keeping the inconsistency for now - looks to me like the best of a bad lot.

[1]
http://captainkvm.com/2013/02/use-cases-for-both-rhev-snapshots-and-netapp-snapshot-copies/
Comment 5 Michal Skrivanek 2013-09-02 08:03:54 EDT
(In reply to Daniel Erez from comment #4)
+1
Comment 6 Einav Cohen 2013-09-05 09:33:57 EDT
Thanks for the detailed explanation, Derez, I trust your opinion on this.

From your feedback, I understand that we should probably rule out solutions (a), (c) and (d), so we are left with (b) [which means that main-tab button and sub-tab button will be consistent with each other, but won't be disabled in case there is a snapshot created in the background], or (e) [which means that only the sub-tab button behavior will be fixed, which will cause the main-tab button and sub-tab button behavior to be inconsistent with each other].

as I mentioned in comment #3 - my suggested solutions were ordered by my preference, so I am in favor of (b) over (e) (i.e. doing nothing and closing this bug, as Arik suggested in Comment #1).

[one last small note - in case we decide to close this bug, it shouldn't be closed on NOT-A-BUG, but on WONT-FIX or CANT-FIX]
Comment 9 Michal Skrivanek 2014-03-31 04:22:32 EDT
going with (b)…

Note You need to log in before you can comment on or make changes to this bug.