Bug 1139590

Summary: wrong nesting causing 'label name' and 'VmNetwork' change is not detected in canDoAction of UpdateNetworkCommand
Product: Red Hat Enterprise Virtualization Manager Reporter: Martin Mucha <mmucha>
Component: ovirt-engineAssignee: Martin Mucha <mmucha>
Status: CLOSED CURRENTRELEASE QA Contact: Meni Yakove <myakove>
Severity: medium Docs Contact:
Priority: medium    
Version: 3.5.0CC: ecohen, gklein, iheim, lpeer, lvernia, masayag, mburman, nyechiel, rbalakri, Rhev-m-bugs, yeylon
Target Milestone: ---   
Target Release: 3.5.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: network
Fixed In Version: vt3 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-02-17 17:14:47 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Network RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1142923, 1156165    

Description Martin Mucha 2014-09-09 09:38:10 UTC
last two lines is not properly nested, so changes to isVmNetwork and getLabel is not processed correctly.


return Objects.equals(oldNetwork.getName(), newNetwork.getName()) &&
                Objects.equals(oldNetwork.getDataCenterId(), newNetwork.getDataCenterId()) &&
                Objects.equals(oldNetwork.getId(), newNetwork.getId()) &&
                Objects.equals(oldNetwork.getMtu(), newNetwork.getMtu()) &&
                Objects.equals(oldNetwork.getName(), newNetwork.getName()) &&
                Objects.equals(oldNetwork.getProvidedBy(), newNetwork.getProvidedBy()) &&
                Objects.equals(oldNetwork.getStp(), newNetwork.getStp()) &&
                Objects.equals(oldNetwork.getVlanId(), newNetwork.getVlanId()) &&
                Objects.equals(oldNetwork.isVmNetwork(), newNetwork.isVmNetwork()) &&
                Objects.equals(oldNetwork.getLabel(), newNetwork.getLabel());

Comment 1 Lior Vernia 2014-09-16 11:30:29 UTC
Removing the CodeChange keyword; looking at the code, this should have caused a very specific bug that could have been found by end users.

If I'm not mistaken, prior to this change it would have been possible to update an existing network from non-VM to VM, if you also changed its label in the same operation. Then all validation would have been skipped, e.g. whether the network's name is already taken in the DC.

To conclude, scenario to test:
1. Set up a non-VM network.
2. Edit it; change it to VM network, and also change its label (it's okay if it didn't have a label before and you're adding a label). Also choose a name that should be taken by another network in the DC.
3. With this patch the operation should be blocked on the name being used; on an older deployment it should pass validation and perhaps even manage to change the network's name (or fail at a later point, e.g. throw a DB constraint violation exception in the engine log).

Comment 2 Michael Burman 2014-09-28 07:22:03 UTC
Verified on - 3.5.0-0.13.beta.el6ev

Comment 3 Eyal Edri 2015-02-17 17:14:47 UTC
rhev 3.5.0 was released. closing.