Red Hat Bugzilla – Bug 989221
vms_sp.sql argument error in InsertVm function
Last modified: 2013-09-23 03:28:10 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!
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.
(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.
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.
(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?
I'm fine with cleanup, as soon as Omer confirms it's not needed and there are no plans to use it
since currently no one uses it, i prefer to remove it, no point for unused code.
merged at: http://gerrit.ovirt.org/gitweb?p=ovirt-engine.git;a=commit;h=971e17c28438644c63d4b9fd51eb6f32ecf566f1
as RC is built, moving to ON_QA (hopefully did not catch incorrect bugs when doing this)
closing as this should be in 3.3 (doing so in bulk, so may be incorrect)