Bug 989221 - vms_sp.sql argument error in InsertVm function
vms_sp.sql argument error in InsertVm function
Status: CLOSED CURRENTRELEASE
Product: oVirt
Classification: Community
Component: ovirt-engine-core (Show other bugs)
unspecified
Unspecified Linux
unspecified Severity high
: ---
: ---
Assigned To: Shahar Havivi
virt
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-28 09:39 EDT by Shahar Havivi
Modified: 2013-09-23 03:28 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-09-23 03:28:10 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)


External Trackers
Tracker ID Priority Status Summary Last Updated
oVirt gerrit 17381 None None None Never

  None (edit)
Description Shahar Havivi 2013-07-28 09:39:08 EDT
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 04:04:15 EDT
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 04:14:18 EDT
(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 04:44:34 EDT
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 04:55:28 EDT
(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 07:23:17 EDT
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 02:25:20 EDT
since currently no one uses it, i prefer to remove it, no point for unused code.
Comment 8 Itamar Heim 2013-08-21 12:43:35 EDT
as RC is built, moving to ON_QA (hopefully did not catch incorrect bugs when doing this)
Comment 9 Itamar Heim 2013-09-23 03:28:10 EDT
closing as this should be in 3.3 (doing so in bulk, so may be incorrect)

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