Martin Mucha has posted comments on this change.

Change subject: core: Introduce HostSetupNetworksParameters parameters
......................................................................


Patch Set 20:

(1 comment)

https://gerrit.ovirt.org/#/c/32776/20/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/HostSetupNetworksParameters.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/HostSetupNetworksParameters.java:

Line 26:     /*This field currently cannot be validated via bean validation due 
to validation conflict:
Line 27:     {@link NetworkAttachment} in this class is present as new entity 
and entity to be removed, both requiring different
Line 28:     conflicting validations. So {@link NetworkAttachment} will be 
validated manually in
Line 29:      {@link org.ovirt.engine.core.bll.CommandBase#canDoAction}*/
Line 30:     private List<NetworkAttachment> removedNetworkAttachments;
> Why do you need the whole attachment and not just the id list?
looking into following patches; it seems that to vdsm is sent list of *names* 
of networks to be removed. Thus sending all instances was probably motivated by 
'not-having-to-query-db'. If I have to pass network names to vdsm and I send 
only network attachments id, I have to query all removed attachments from db. 
That's not expensive , so we can change that. But please, confirm me that.

And I don't think we can use bean validation even after this change. This field:
org.ovirt.engine.core.common.action.HostSetupNetworksParameters#networkAttachments
contains both new and updated instances. Which would require to add both 
UpdateEntity.class and CreateEntity validation groups, which is not possible.
Line 31: 
Line 32:     private List<Bond> bonds;
Line 33: 
Line 34:     private List<Bond> removedBonds;


-- 
To view, visit https://gerrit.ovirt.org/32776
To unsubscribe, visit https://gerrit.ovirt.org/settings

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

Reply via email to