Lior Vernia has posted comments on this change.
Change subject: engine: Extracted some QoS validation to NetworkQosValidator
......................................................................
Patch Set 13:
(2 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/UpdateNetworkQoSCommand.java
Line 19: NetworkQosValidator validator = new
NetworkQosValidator(getNetworkQoS());
Line 20: if (!validate(validator.qosExists()) ||
!validate(validator.consistentDataCenter())) {
Line 21: return false;
Line 22: } else {
Line 23: NetworkQoS oldNetworkQoS =
getNetworkQoSDao().get(getNetworkQoS().getId());
I did, but there was no reason yet to push this validation into the validator.
I could definitely push that in there with the rest in a separate patch.
Line 24: if (validateValues()) {
Line 25: if
(!oldNetworkQoS.getName().equals(getNetworkQoS().getName())) {
Line 26: return validateNameNotExistInDC();
Line 27: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkQosValidator.java
Line 33: /**
Line 34: * Verify that the QoS entity has the same DC ID as the one stored
in the database.
Line 35: */
Line 36: public ValidationResult consistentDataCenter() {
Line 37: return (qos != null && (getOldQos() == null ||
!qos.getStoragePoolId().equals(getOldQos().getStoragePoolId())))
1. QoS will sometimes be null, in later usages.
2. Firstly, at this point there's no way to create an anonymous QoS entity even
though the constraint on the DB was lifted. Secondly, I would not expect this
validation to be called on anonymous QoS entities, because it is irrelevant to
them.
Line 38: ? new
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_INVALID_DC_ID)
Line 39: : ValidationResult.VALID;
Line 40: }
Line 41:
--
To view, visit http://gerrit.ovirt.org/22600
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I13dbb35bbd4cdbf4071e79d81eeb52667b734fcb
Gerrit-PatchSet: 13
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Moti Asayag <[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
