Mike Kolesnik has posted comments on this change.

Change subject: engine: Extracted some QoS validation to NetworkQosValidator
......................................................................


Patch Set 6:

(3 comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkQosValidator.java
Line 6: import org.ovirt.engine.core.dal.dbbroker.DbFacade;
Line 7: 
Line 8: public class NetworkQosValidator {
Line 9: 
Line 10:     boolean oldQosRetrieved;
Not sure why you would need this flag, if the QoS retrieved is null then any of 
the checks would fail, so not expecting the caller to continue but even if he 
does this doesn't translate to a big saving (and we already have some cache 
mechanism on the DAL layer anyway).
Line 11: 
Line 12:     private final NetworkQoS qos;
Line 13:     private NetworkQoS oldQos;
Line 14: 


Line 27:     /**
Line 28:      * Verify that the QoS entity had previously existed in the 
database.
Line 29:      */
Line 30:     public ValidationResult qosExists() {
Line 31:         return (qos != null && getOldQos() == null)
Why would a null QoS field make sense?
Line 32:                 ? new 
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_NOT_FOUND)
Line 33:                 : ValidationResult.VALID;
Line 34:     }
Line 35: 


Line 36:     /**
Line 37:      * Verify that the QoS entity has the same DC ID as the one stored 
in the database.
Line 38:      */
Line 39:     public ValidationResult consistentDataCenter() {
Line 40:         return (qos != null && (getOldQos() == null || 
!qos.getStoragePoolId().equals(getOldQos().getStoragePoolId())))
Same here..
Line 41:                 ? new 
ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_NETWORK_QOS_INVALID_DC_ID)
Line 42:                 : ValidationResult.VALID;
Line 43:     }
Line 44: 


-- 
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: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to