Alona Kaplan has posted comments on this change.

Change subject: core: Add host network QoS entity
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.ovirt.org/#/c/34119/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/HostNetworkQos.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/network/HostNetworkQos.java:

Line 11: public class HostNetworkQos extends QosBase {
Line 12: 
Line 13:     private static final long serialVersionUID = -5062624700835301848L;
Line 14: 
Line 15:     @Range(min = 0, max = 
BusinessEntitiesDefinitions.HOST_NETWORK_QOS_MAX_SHARES,
Why isn't it a config value like the others?
Line 16:             message = 
"ACTION_TYPE_FAILED_NETWORK_QOS_OUT_OF_RANGE_VALUES")
Line 17:     private Integer outAverageLinkshare;
Line 18: 
Line 19:     @ConfiguredRange(min = 0, maxConfigValue = 
ConfigValues.MaxAverageNetworkQoSValue,


Line 64:                 .append("]");
Line 65:         return builder.toString();
Line 66:     }
Line 67: 
Line 68:     public boolean equalValues(HostNetworkQos other) {
Please make this method private or even part of the equals method.
If you need this functionality the compare partially initialized object, add it 
at the point you need it.
Line 69:         return ObjectUtils.objectsEqual(getOutAverageLinkshare(), 
other.getOutAverageLinkshare())
Line 70:                 && ObjectUtils.objectsEqual(getOutAverageUpperlimit(), 
other.getOutAverageUpperlimit())
Line 71:                 && ObjectUtils.objectsEqual(getOutAverageRealtime(), 
other.getOutAverageRealtime());
Line 72:     }


http://gerrit.ovirt.org/#/c/34119/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/QosBase.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/QosBase.java:

Line 57:     public String getString() {
Line 58:         return toString();
Line 59:     }
Line 60: 
Line 61:     protected String renderQosParameter(Object qosParameter) {
Please use this method not just in HostNetworkQos but also for other classes 
extends QosBase (or at least on NetworkQos).
Line 62:         return (qosParameter == null) ? UNLIMITED : 
String.valueOf(qosParameter);
Line 63:     }
Line 64: 
Line 65:     @Override


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9471e68ba36fcc2f1465059037125a4c19a4c07f
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Lior Vernia <[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