Bug 1113478 - Host PM port is empty after disabling PM checkbox and save
Summary: Host PM port is empty after disabling PM checkbox and save
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine-webadmin-portal
Version: 3.5.0
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: ---
: 3.5.0
Assignee: Martin Perina
QA Contact: Antonin Pagac
URL:
Whiteboard: infra
Depends On:
Blocks: rhev3.5beta 1156165
TreeView+ depends on / blocked
 
Reported: 2014-06-26 09:22 UTC by Eli Mesika
Modified: 2016-02-10 19:40 UTC (History)
9 users (show)

Fixed In Version: ovirt-3.5.0_rc1
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-02-17 17:09:33 UTC
oVirt Team: Infra
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 29593 0 master MERGED webadmin: Save PM options even when PM disabled Never
oVirt gerrit 30565 0 ovirt-engine-3.5 MERGED webadmin: Save PM options even when PM disabled Never

Description Eli Mesika 2014-06-26 09:22:20 UTC
Description of problem:
Host PM port is empty after disabling PM enabled checkbox and save 

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.Add a host with PM definitions and agent that has port (apc_snmp) for example 
2.Edit the Host and open PM TAB
3.Disable "Enable Power Management" checkbox
4.click OK
5.Edit the Host and open PM TAB
6.port value is vanished 
 
Actual results:
port value is vanished after disabling "Enable Power Management" checkbox

Expected results:
port value should be preserved 

Additional info:

Comment 1 Oved Ourfali 2014-07-14 11:12:09 UTC
Eli - I discussed it with Martin, and it seems like we shouldn't save anything from the PM details entered in the dialog, in case PM is disabled.
Any reason we're doing it so far?

Comment 2 Eli Mesika 2014-07-19 19:34:44 UTC
(In reply to Oved Ourfali from comment #1)
> Eli - I discussed it with Martin, and it seems like we shouldn't save
> anything from the PM details entered in the dialog, in case PM is disabled.
> Any reason we're doing it so far?

Why not?
If I am configuring a Host PM and want only to disable PM for a period of time from my reasons, why I will be required to insert all the PM information again.
The operation is disable PM not delete so I expect all data to remain in DB except of the change of the PM enabled flag

Comment 3 Martin Perina 2014-07-21 07:59:09 UTC
Eli, you are right, but my idea was a bit different. Let me show you this example:

1) You have valid PM options and PM enabled
2) You enter invalid options (like removing username, setting port number
   to XXX), disable PM options and click on the Save button -> when PM is 
   disabled, there's no validation, so there will be saved invalid values into DB


My idea:

When PM is disabled, don't update PM options in the database (for example by  
not propagating user input from UI to saved entity or loading current DB values 
PM options values prior to update query or using customized query or ...).
So we will  always have valid PM options in db no matter if PM is enabled/disabled.
I know it's a bit problematic because PM options are part of vds_static, but if we decide to move PM options into its own entity, it will be much easier to achieve this.

Comment 4 Oved Ourfali 2014-07-22 07:28:18 UTC
(In reply to Martin Perina from comment #3)
> Eli, you are right, but my idea was a bit different. Let me show you this
> example:
> 
> 1) You have valid PM options and PM enabled
> 2) You enter invalid options (like removing username, setting port number
>    to XXX), disable PM options and click on the Save button -> when PM is 
>    disabled, there's no validation, so there will be saved invalid values
> into DB
> 
> 
> My idea:
> 
> When PM is disabled, don't update PM options in the database (for example by
> 
> not propagating user input from UI to saved entity or loading current DB
> values 
> PM options values prior to update query or using customized query or ...).
> So we will  always have valid PM options in db no matter if PM is
> enabled/disabled.
> I know it's a bit problematic because PM options are part of vds_static, but
> if we decide to move PM options into its own entity, it will be much easier
> to achieve this.

You may have a use-case in which someone sets the values when adding the host, but doesn't want to enable it for now. So I suggest leaving the current behavior, and just saving the missing field (port).

Comment 5 Oved Ourfali 2014-07-22 07:30:38 UTC
What we can do is remove the periodic PM status check, not to test PM details when PM is disabled, however it doesn't relate to this bug.

Scott - thoughts?

Comment 6 Eli Mesika 2014-07-22 07:56:37 UTC
(In reply to Oved Ourfali from comment #5)
> What we can do is remove the periodic PM status check, not to test PM
> details when PM is disabled, however it doesn't relate to this bug.
> 
> Scott - thoughts?

It is not periodic, it is done when the user press OK on the dialog.
The periodic check is he PM Health feature and it is not done for host with pm_enabled = false

Comment 7 Scott Herold 2014-07-29 19:46:31 UTC
(In reply to Oved Ourfali from comment #5)
> What we can do is remove the periodic PM status check, not to test PM
> details when PM is disabled, however it doesn't relate to this bug.
> 
> Scott - thoughts?

Oved - It appears as if the use case is simply how we deal with the database values on PM Disable.  From what I gather in this BZ, when a user disables PM and saves, we are explicitly wiping all DB data related to PM settings.  Is there any reason we can't simply disable PM while still maintaining the specific PM configuration settings?  This will make re-enabling PM a simple single checkbox, not a checkbox and re-filling in 3-5 options every time.

Comment 8 Oved Ourfali 2014-07-31 07:37:32 UTC
(In reply to Scott Herold from comment #7)
> (In reply to Oved Ourfali from comment #5)
> > What we can do is remove the periodic PM status check, not to test PM
> > details when PM is disabled, however it doesn't relate to this bug.
> > 
> > Scott - thoughts?
> 
> Oved - It appears as if the use case is simply how we deal with the database
> values on PM Disable.  From what I gather in this BZ, when a user disables
> PM and saves, we are explicitly wiping all DB data related to PM settings. 
> Is there any reason we can't simply disable PM while still maintaining the
> specific PM configuration settings?  This will make re-enabling PM a simple
> single checkbox, not a checkbox and re-filling in 3-5 options every time.

We are saving the values.
However, we still do the PM status check, even if disabled.
It is harmless, but might trigger some unwanted alerts.
I'd leave this as-is, unless someone complains about it.

Comment 9 Antonin Pagac 2014-09-10 14:05:17 UTC
Verified.

All the values in PM tab are saved.

oVirt Engine Version: 3.5.0-0.0.master.20140821064931.gitb794d66.el6

Comment 10 Eyal Edri 2015-02-17 17:09:33 UTC
rhev 3.5.0 was released. closing.


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