Liron Ar has posted comments on this change. Change subject: common: introduce qos package, and storage qos ......................................................................
Patch Set 6: (9 comments) http://gerrit.ovirt.org/#/c/27093/6/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 23: @NotNull(message = "ACTION_TYPE_FAILED_QOS_INVALID_DC_ID") Line 24: private Guid storagePoolId; Line 25: Line 26: @NotNull(message = "QOS_NAME_NOT_NULL") Line 27: @Size(min = 1, max = BusinessEntitiesDefinitions.GENERAL_NAME_SIZE, message = "QOS_NAME_TOO_LONG") what if the size is 0? (wrong message) Line 28: @ValidI18NName(message = "QOS_NAME_INVALID") Line 29: private String name; Line 30: Line 31: @Size(max = BusinessEntitiesDefinitions.GENERAL_MAX_SIZE) Line 107: return getId(); Line 108: } Line 109: Line 110: @Override Line 111: public int hashCode() { atleast in theory, we might have same hashcode for unequal qos objects which is not optimal when using collections that use the hash function. Line 112: final int prime = 31; Line 113: int result = 1; Line 114: result = prime * result + ((description == null) ? 0 : description.hashCode()); Line 115: result = prime * result + ((id == null) ? 0 : id.hashCode()); Line 118: result = prime * result + ((storagePoolId == null) ? 0 : storagePoolId.hashCode()); Line 119: return result; Line 120: } Line 121: Line 122: @Override to make it short - i'm also in for "traditional" implementation for both equals and hashcode all over the hirerchy Line 123: public boolean equals(Object obj) { Line 124: if (this == obj) Line 125: return true; Line 126: if (obj == null) http://gerrit.ovirt.org/#/c/27093/6/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/QosType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/QosType.java: Line 24: public int getValue() { Line 25: return value; Line 26: } Line 27: Line 28: public static QosType forValue(int value) { when you rebase you have this as an Integer, as autoboxing will happen anyway..so if one sends an Integer, it won't be out boxed and then boxed again, Line 29: return valueToStatus.get(value); Line 30: } http://gerrit.ovirt.org/#/c/27093/6/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/StorageQos.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/StorageQos.java: Line 12: super(QosType.STORAGE); Line 13: } Line 14: Line 15: private static final long serialVersionUID = 1122123549710787758L; Line 16: general: please set the relevant validation groups for the validators (CreateEntity, UpdateEntity). Line 17: @ConfiguredRange(min = 0, maxConfigValue = ConfigValues.maxThroughputUpperBoundQosValue, Line 18: message = "ACTION_TYPE_FAILED_QOS_OUT_OF_RANGE_VALUES") Line 19: private Integer maxThroughput; Line 20: Line 100: && ObjectUtils.objectsEqual(maxWriteIops, other.getMaxWriteIops()); Line 101: } Line 102: Line 103: @Override Line 104: public int valuesHashCode() { doesn't seems like it's somewhere used. Line 105: final int prime = 31; Line 106: int result = 1; Line 107: result = prime * result + ((maxThroughput == null) ? 0 : maxThroughput.hashCode()); Line 108: result = prime * result + ((maxReadThroughput == null) ? 0 : maxReadThroughput.hashCode()); http://gerrit.ovirt.org/#/c/27093/6/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java: Line 1750: @TypeConverterAttribute(Integer.class) Line 1751: @DefaultValueAttribute("1500") Line 1752: DefaultMtu, Line 1753: Line 1754: @DefaultValueAttribute("1000000") missing @TypeConverterAttribute(Integer.class) Line 1755: maxThroughputUpperBoundQosValue, Line 1756: Line 1757: @TypeConverterAttribute(Integer.class) Line 1758: @DefaultValueAttribute("1000000") http://gerrit.ovirt.org/#/c/27093/6/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 1094: ACTION_TYPE_FAILED_GLUSTER_VOLUME_CANNOT_STOP_REBALANCE_IN_PROGRESS= Cannot ${action} ${type}. Rebalance is running on the volume ${volumeName} in cluster ${vdsGroup}. Line 1095: ACTION_TYPE_FAILED_GLUSTER_OPERATION_INPROGRESS=Cannot ${action} ${type}. Gluster operation is in progress in cluster. Please try again. Line 1096: ACTION_TYPE_FAILED_TAG_ID_REQUIRED=Cannot ${action} ${type}. Tag ID is required. Line 1097: Line 1098: ACTION_TYPE_FAILED_QOS_OUT_OF_RANGE_VALUES=Cannot ${action} ${type}. Values are out of range. I'd add specific message for each one, it may not be informative enough.. Line 1099: ACTION_TYPE_FAILED_QOS_INVALID_DC_ID=Cannot ${action} ${type}. Invalid data center. Line 1100: Line 1101: ACTION_TYPE_FAILED_NETWORK_QOS_MISSING_VALUES=Cannot ${action} ${type}. All three values are needed in order to define QoS on each network directions. Line 1102: ACTION_TYPE_FAILED_NETWORK_QOS_NEGATIVE_VALUES=Cannot ${action} ${type}. Negative values are not allowed. Line 1095: ACTION_TYPE_FAILED_GLUSTER_OPERATION_INPROGRESS=Cannot ${action} ${type}. Gluster operation is in progress in cluster. Please try again. Line 1096: ACTION_TYPE_FAILED_TAG_ID_REQUIRED=Cannot ${action} ${type}. Tag ID is required. Line 1097: Line 1098: ACTION_TYPE_FAILED_QOS_OUT_OF_RANGE_VALUES=Cannot ${action} ${type}. Values are out of range. Line 1099: ACTION_TYPE_FAILED_QOS_INVALID_DC_ID=Cannot ${action} ${type}. Invalid data center. you can use ACTION_TYPE_FAILED_STORAGE_POOL_NOT_EXIST message Line 1100: Line 1101: ACTION_TYPE_FAILED_NETWORK_QOS_MISSING_VALUES=Cannot ${action} ${type}. All three values are needed in order to define QoS on each network directions. Line 1102: ACTION_TYPE_FAILED_NETWORK_QOS_NEGATIVE_VALUES=Cannot ${action} ${type}. Negative values are not allowed. Line 1103: ACTION_TYPE_FAILED_NETWORK_QOS_OUT_OF_RANGE_VALUES=Cannot ${action} ${type}. Values are out of range. -- To view, visit http://gerrit.ovirt.org/27093 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a9af59277b5055453159f002f19046c0051d8ff Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Doron Fediuck <[email protected]> Gerrit-Reviewer: Einav Cohen <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Kobi Ianko <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[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
