Bug 989221

Summary: vms_sp.sql argument error in InsertVm function
Product: [Retired] oVirt Reporter: Shahar Havivi <shavivi>
Component: ovirt-engine-coreAssignee: Shahar Havivi <shavivi>
Status: CLOSED CURRENTRELEASE QA Contact:
Severity: high Docs Contact:
Priority: unspecified    
Version: unspecifiedCC: acathrow, iheim, juan.hernandez, michal.skrivanek, ofrenkel, shavivi, yeylon
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Linux   
Whiteboard: virt
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-09-23 07:28:10 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Shahar Havivi 2013-07-28 13:39:08 UTC 
Description of problem:
Database script vms_sp.sql have insertVm function that gets few arguments.
One of the arguments is v_num_of_monitors followed by v_allow_console_reconnect.

v_num_of_monitors is integer and usually get the values 1,2 or 4.
v_allow_console_reconnect is boolean, 
the values of this two arguments are v_num_of_monitors, v_num_of_monitors
i.e v_num_of_monitors is getting the correct value but v_allow_console_reconnect is getting the v_num_of_monitors which is always positive integer, which mean its always true!
Comment 1 Juan Hernández 2013-07-29 08:04:15 UTC 
Shahar, how did you detect this? Was it doing a code inspection or did you detect it as a real problem? I ask because as far as I can tell this stored procedure is only called from the VmDAO.save() method, and that method isn't currently (in the current master) called by anyone. This is important to determine, as if it is actually causing a problem then we need to backport it to 3.2.z.
Comment 2 Shahar Havivi 2013-07-29 08:14:18 UTC 
(In reply to Juan Hernández from comment #1)
I did not encounter any problem, I notice the bug by examining the function.
We may have a hidden bug, if not we centrally have a potential one.
Comment 3 Juan Hernández 2013-07-29 08:44:34 UTC 
I checked this in the downstream 3.2.z branch and, like in the upstream master, the VmDAO.save() method isn't called at all, so this change doesn't need to be backported.

I would also suggest to remove the VmDAO.save() method and the stored procedure.
Comment 4 Shahar Havivi 2013-07-29 08:55:28 UTC 
(In reply to Juan Hernández from comment #3)
> I would also suggest to remove the VmDAO.save() method and the stored
> procedure.
Omer note that two...
Michal, what do you think?
Comment 5 Michal Skrivanek 2013-07-31 11:23:17 UTC 
I'm fine with cleanup, as soon as Omer confirms it's not needed and there are no plans to use it
Comment 6 Omer Frenkel 2013-08-04 06:25:20 UTC 
since currently no one uses it, i prefer to remove it, no point for unused code.
Comment 7 Shahar Havivi 2013-08-04 13:19:36 UTC 
merged at: http://gerrit.ovirt.org/gitweb?p=ovirt-engine.git;a=commit;h=971e17c28438644c63d4b9fd51eb6f32ecf566f1
Comment 8 Itamar Heim 2013-08-21 16:43:35 UTC 
as RC is built, moving to ON_QA (hopefully did not catch incorrect bugs when doing this)
Comment 9 Itamar Heim 2013-09-23 07:28:10 UTC 
closing as this should be in 3.3 (doing so in bulk, so may be incorrect)