Lior Vernia has posted comments on this change. Change subject: core: Add host network QoS entity ......................................................................
Patch Set 2: (4 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? Done, note I also changed to min=1 (min=0 would be valid but not very meaningful). 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. Done 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 15: * Base class for QoS objects derived class will hold qos limit according to type. Line 16: */ Line 17: public class QosBase extends IVdcQueryable implements BusinessEntity<Guid>, Serializable { Line 18: Line 19: private static final String UNLIMITED= "Unlimited"; > shouldn't a space character come before '=' sign? Done Line 20: private static final long serialVersionUID = 1122772549710787678L; Line 21: private Guid id = Guid.Empty; Line 22: private QosType qosType; Line 23: 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 classe Would you mind if I did this in another patch? Not directly related to the feature, it's a general improvement. 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: Yevgeny Zaspitsky <[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
