Bug 1391466 - multi-dialog use in the VM dialog is incorrect and causing cleanup issues
Summary: multi-dialog use in the VM dialog is incorrect and causing cleanup issues
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: ovirt-engine
Classification: oVirt
Component: Frontend.WebAdmin
Version: 4.0.5.5
Hardware: Unspecified
OS: Unspecified
high
urgent
Target Milestone: ovirt-4.0.6
: 4.0.6.2
Assignee: Tomas Jelinek
QA Contact: Lucie Leistnerova
URL:
Whiteboard:
: 1400706 (view as bug list)
Depends On:
Blocks: 1396782 1396783
TreeView+ depends on / blocked
 
Reported: 2016-11-03 11:45 UTC by Petr Kubica
Modified: 2020-02-14 18:09 UTC (History)
9 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2017-01-11 14:55:26 UTC
oVirt Team: Virt
Embargoed:
oourfali: ovirt-4.0.z?
rule-engine: planning_ack?
rule-engine: devel_ack+
pstehlik: testing_ack+


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 66095 0 master POST webadmin,userportal: Improve dialog cleanup logic 2016-11-06 08:03:16 UTC
oVirt gerrit 66134 0 ovirt-engine-4.0 POST webadmin,userportal: Improve dialog cleanup logic 2016-11-07 10:02:59 UTC
oVirt gerrit 67281 0 master MERGED webadmin,userportal: Support multiple dialogs per model 2016-11-29 14:27:33 UTC
oVirt gerrit 67282 0 master MERGED webadmin: changed instance images flows to open the window above VM window 2016-11-29 17:50:17 UTC
oVirt gerrit 67521 0 ovirt-engine-4.0 MERGED webadmin,userportal: Support multiple dialogs per model 2016-11-29 22:14:57 UTC
oVirt gerrit 67522 0 ovirt-engine-4.0.6 MERGED webadmin,userportal: Support multiple dialogs per model 2016-11-29 22:15:42 UTC
oVirt gerrit 67535 0 ovirt-engine-4.0 MERGED webadmin: changed instance images flows to open the window above VM window 2016-11-29 22:14:49 UTC
oVirt gerrit 67536 0 ovirt-engine-4.0.6 MERGED webadmin: changed instance images flows to open the window above VM window 2016-11-29 22:15:38 UTC

Description Petr Kubica 2016-11-03 11:45:54 UTC
Description of problem:
When creating new VM, it should be possible to set custom number of VM. Now when setting different count of cpu, that column is marked as bad value when confirming the dialog

Version-Release number of selected component (if applicable):
4.0.5-5
ovirt-engine-webadmin-portal-4.0.5.4-0.1.el7ev.noarch

How reproducible:
always - you must add disk before (without adding disk, it works)

Steps to Reproduce:
1. install clean environment 
2. add host and storage domain
3. add VM with disk! and with more than 1 virtual CPU

Actual results:
- Column is marked red -> user can't modify count of virtual CPU 
- when VM is already created, edit VM and modify count of virtual CPU works

Expected results:
should work when creating VM

Additional info:
-

Comment 1 Tomas Jelinek 2016-11-04 08:38:29 UTC
This is a regression caused by https://gerrit.ovirt.org/#/c/63708/
The problem is that when you open the "create disk" dialog from the "create vm" dialog, the "create vm" dialog will get cleared up. Than, when the "create disk" dialog is closed, the "create vm" dialog is not properly inited again (especially the model listeners". The consequence is that big part of the dialog, including the CPU count will stop working.

I think that all places where a dialog is opened from other dialog and than moved back can cause this issue. For example the new quota dialog looks strange after pressing "edit" on the quota (e.g. the button is grey but clickable). For some reason it seems to be working on configure->and any window opened there. Not sure how the wizard of "new dc, new cluster" will work.

Moving to UX for further investigation.

Comment 2 Vojtech Szocs 2016-11-04 17:59:57 UTC
Tomas, many thanks for clearing up the situation.

I've posted a patch that defers cleanup of all dialog related resources to the point where there are no active (visible) dialogs left. This should cover the "dialog triggered by another dialog" scenario as described in comment #1.

Comment 3 Lucie Leistnerova 2016-11-22 21:46:17 UTC
still can't change count of CPU
in ovirt-engine-4.1.0-0.0.master.20161121231311.git19a0953.el7.centos.noarch

Steps to Reproduce:
1. click on 'New VM', enter name
2. click on 'Create' in section Instance Images, enter disk size, click 'OK'
3. go to System, change value in Total Virtual CPUs

VM can't be created until the number of CPU is back to value before disk creation and error 'Incorrect number of Total Virtual CPUs....' is shown on the input

Comment 4 Red Hat Bugzilla Rules Engine 2016-11-22 21:46:23 UTC
Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release.

Comment 5 Oved Ourfali 2016-11-24 06:22:40 UTC
After talking offline with UX and Virt guys, we come up with the following analysis (I'm quoting Vojtech):
"
The problem seems to be VM dialog
being closed [*] before the Disk dialog is shown.

[*] at this point, active dialog count == 0 -> perform dialog cleanup

If a dialog is closed && active dialog count == 0, there is no way for
UI infra to know if [some] dialog models are still needed. I'd suggest
to modify existing code to either:

1, show Disk dialog on top of VM dialog (prevent 0 active dialog count
   scenario that triggers dialog model cleanup)

2, exclude VM dialog resources from cleanup (results in memory leaks)

Again, as I wrote many times, dialogs are designed as *non*-singleton
components, including the associated dialog Model. Referring to dialog
model once the dialog is closed therefore contradicts existing design
(which has existed since the very beginning).

Our memory leak fixes basically uncovered cases where dialog models
are (incorrectly) referenced *after* the given dialog is closed.

The proper solution is to refactor code that goes against established
design. Trying to solve this on UI infra level is neither optimal nor
correct.
"

Therefor, moving this to Virt to implement one of the alternatives mentioned above. Let us know if you need assistance.

Comment 6 Vojtech Szocs 2016-11-25 19:10:49 UTC
We'll need a minimal UI infra change to support triggering Disk dialog on top of VM dialog. The minimal UI infra change + VM dialog code fix should be merged & backported into 4.0.

Afterwards, we can improve UI infra code (master only).

Comment 7 Oved Ourfali 2016-12-02 13:09:31 UTC
*** Bug 1400706 has been marked as a duplicate of this bug. ***

Comment 8 Lucie Leistnerova 2016-12-06 10:25:09 UTC
changing value of CPU with disk creation is OK

verified in ovirt-engine-webadmin-portal-4.1.0-0.2.master.20161205231208.gitf0af92b.el7.centos.noarch
and ovirt-engine-webadmin-portal-4.0.6.2-0.1.el7ev.noarch


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