Gilad Chaplik has posted comments on this change.

Change subject: core: Quota refactor - parameters
......................................................................


Patch Set 9: (7 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 585:             Transaction transaction = TransactionSupport.suspend();
Line 586:             try {
Line 587:                 returnValue =
Line 588:                         isUserAuthorizedToRunAction() && 
isBackwardsCompatible() && validateInputs() && acquireLock()
Line 589: //                                && internalValidateAndSetQuota()
call canDoAction before internalValidateAndSetQuota
Line 590:                                 && canDoAction();
Line 591:                 if (returnValue && this instanceof Quotable) {
Line 592:                     returnValue &= ((Quotable) 
this).validateAndSetQuota();
Line 593:                 }


Line 618:         }
Line 619: 
Line 620:         if (getStoragePool() == null) {
Line 621:             log.error("Command: " + this.getClass().getName()
Line 622:                     + ". Storage pool is not available for quota 
calculation. ");
if missing storagePool, the flow has errors. return false?
Line 623:         }
Line 624: 
Line 625:         QuotaConsumptionParametersWrapper 
quotaConsumptionParametersWrapper = new QuotaConsumptionParametersWrapper(this,
Line 626:                 getReturnValue().getCanDoActionMessages());


Line 805:             
addCanDoActionMessage(VdcBllMessages.USER_NOT_AUTHORIZED_TO_PERFORM_ACTION);
Line 806:             return false;
Line 807:         }
Line 808: 
Line 809:         if 
(!VdcActionType.QuotaDependency.NONE.equals(this.getActionType().getQuotaDependency()))
 {
put the 'if' within the method (and remove 'this')
Line 810:             addQuotaPermissionSubject(permSubjects);
Line 811:         }
Line 812: 
Line 813:         for (PermissionSubject permSubject : permSubjects) {


Line 843:     }
Line 844: 
Line 845:     public void addQuotaPermissionSubject(List<PermissionSubject> 
quotaPermissionList) {
Line 846:         if (!isInternalExecution() &&
Line 847:                 
!getStoragePool().getQuotaEnforcementType().equals(QuotaEnforcementTypeEnum.DISABLED))
 {
switch places in the 'equals', getStoragePool().getQuotaEnforcementType() can 
be null (or use ==)
Line 848: 
Line 849:             for (QuotaConsumptionParameter parameter : 
getQuotaConsumptionParameters()) {
Line 850:                 quotaPermissionList.add(new 
PermissionSubject(parameter.getQuotaGuid(), VdcObjectType.Quota, 
ActionGroup.CONSUME_QUOTA));
Line 851:             }


Line 844: 
Line 845:     public void addQuotaPermissionSubject(List<PermissionSubject> 
quotaPermissionList) {
Line 846:         if (!isInternalExecution() &&
Line 847:                 
!getStoragePool().getQuotaEnforcementType().equals(QuotaEnforcementTypeEnum.DISABLED))
 {
Line 848: 
getQuotaConsumptionParameters() can be null.
Line 849:             for (QuotaConsumptionParameter parameter : 
getQuotaConsumptionParameters()) {
Line 850:                 quotaPermissionList.add(new 
PermissionSubject(parameter.getQuotaGuid(), VdcObjectType.Quota, 
ActionGroup.CONSUME_QUOTA));
Line 851:             }
Line 852:         }


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/quota/QuotaConsumptionParametersWrapper.java
Line 32: 
Line 33:     public void setParameters(List<QuotaConsumptionParameter> 
parameters) {
Line 34:         this.parameters = parameters;
Line 35:     }
Line 36: 
handle BL in the BL class (separating into 2 lists)
Line 37:     public List<QuotaStorageConsumptionParameter> 
getQuotaStorageConsumptionParameters() {
Line 38:         List<QuotaStorageConsumptionParameter> list = new 
ArrayList<QuotaStorageConsumptionParameter>();
Line 39:         for (QuotaConsumptionParameter parameter : getParameters()){
Line 40:             if (parameter instanceof QuotaStorageConsumptionParameter) 
{


Line 79:     }
Line 80: 
Line 81:     /**
Line 82:      * Revert the numbers of all requested cpu, memory and storage
Line 83:      */
remove this method
Line 84:     public void revert() {
Line 85:         for (QuotaConsumptionParameter parameter : getParameters()) {
Line 86:             
parameter.setQuotaAction(QuotaAction.CONSUME.equals(parameter.getQuotaAction()) 
?
Line 87:                     QuotaAction.RELEASE :


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iebfc85569ba1aa8bd840f7239f83b7f921a4bd8e
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <oma...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Sharad Mishra <snmis...@linux.vnet.ibm.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: ofri masad <oma...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to