Martin Mucha has posted comments on this change.

Change subject: core: fail when trying to remove labeled VLAN network.
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/32525/1//COMMIT_MSG
Commit Message:

Line 17: Also it seemed to me like a bug, that if isVlan(nic) is true, but it's
Line 18: base interface nic is present in setupnetworks request(ie. vlan
Line 19: related nic was deleted but base interface was not), vlan related
Line 20: nic is checked for label presence, although it's base resource should
Line 21: have been validated.
> Only the base interface should be validated for the existence of network la
if it was a bug, it was fixed. But in preexisting code was possible this(if I'm 
not mistaken):

if condition "NetworkUtils.isVlan(nic)" was true, but user send this nic in 
request, then it was present int nicsByName map, so following condition 
"nic==null" was false. Therefore no continue, and execution continued with 
"NetworkUtils.isLabeled(nic)" on nic, which is vlaned. Which contradicts 
statement, that only base interface should be validated. I didn't checked what 
happens if such validation is called on vlaned nic. But I think this is not  
problem. Problem was skipping validation as a whole when vlan is found, instead 
of validation of its base interface.
Line 22: 
Line 23: Change-Id: I132f4ae124e45144beee21f81f69e55ed25cd540
Line 24: Bug-Url: https://bugzilla.redhat.com/1134380


-- 
To view, visit http://gerrit.ovirt.org/32525
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I132f4ae124e45144beee21f81f69e55ed25cd540
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Martin Mucha <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to