Arik Hadas has uploaded a new change for review.

Change subject: core: organize can-do methods
......................................................................

core: organize can-do methods

AddVmCommand#canAddVm:
1. Return false with an error as soon as a problem is detected instead
of using the 'returnValue' field to store the result
2. Inline the 'exists' variable

AddVmFromScratchCommand#canDoAction:
1. Return false with an error as soon as a problem is detected instead
of using the 'result' field to store the result
2. Remove redundant addition of action messages
VdcBllMessages.VAR__ACTION__ADD and VdcBllMessages.VAR__TYPE__VM as they
are already added by AddVmCommand

Change-Id: I4589fa4681913e41b1be595131686932d92ca062
Signed-off-by: Arik Hadas <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java
2 files changed, 14 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/59/15959/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
index ff776e8..627f89e 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
@@ -439,23 +439,17 @@
 
     protected boolean canAddVm(List<String> reasons, String name, Guid 
storagePoolId,
             int vmPriority) {
-        boolean returnValue;
         // Checking if a desktop with same name already exists
-        boolean exists = isVmWithSameNameExists(name);
+        if (isVmWithSameNameExists(name)) {
+            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_ALREADY_USED);
+        }
 
-        if (exists) {
-            if (reasons != null) {
-                
reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_NAME_ALREADY_USED.toString());
-            }
-
+        if (!verifyAddVM(reasons, vmPriority)) {
             return false;
         }
 
-        boolean checkTemplateLock = getParameters().getParentCommand() == 
VdcActionType.AddVmPoolWithVms ? false : true;
-
-        returnValue = verifyAddVM(reasons, vmPriority);
-
-        if (returnValue && !getParameters().getDontCheckTemplateImages()) {
+        if (!getParameters().getDontCheckTemplateImages()) {
+            boolean checkTemplateLock = getParameters().getParentCommand() != 
VdcActionType.AddVmPoolWithVms;
             for (StorageDomain storage : destStorages.values()) {
                 if 
(!VmTemplateCommand.isVmTemplateImagesReady(getVmTemplate(), storage.getId(),
                         reasons, false, checkTemplateLock, true, true, 
storageToDisksMap.get(storage.getId()))) {
@@ -464,7 +458,7 @@
             }
         }
 
-        return returnValue;
+        return true;
     }
 
     protected boolean verifyAddVM(List<String> reasons, int vmPriority) {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java
index e2adbae..4862051 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java
@@ -124,23 +124,16 @@
 
     @Override
     protected boolean canDoAction() {
-        boolean result = (getVdsGroup() != null || 
!Guid.Empty.equals(super.getStorageDomainId()));
-        if (!result) {
-            addCanDoActionMessage(VdcBllMessages.VM_CLUSTER_IS_NOT_VALID);
-        } else {
-            result = 
ImagesHandler.CheckImagesConfiguration(getStorageDomainId().getValue(),
-                                                            getParameters()
-                                                                           
.getDiskInfoList(),
-                                                            
getReturnValue().getCanDoActionMessages());
+        if (getVdsGroup() == null && 
Guid.Empty.equals(super.getStorageDomainId())) {
+            return failCanDoAction(VdcBllMessages.VM_CLUSTER_IS_NOT_VALID);
         }
 
-        if (!result) {
-            addCanDoActionMessage(VdcBllMessages.VAR__ACTION__ADD);
-            addCanDoActionMessage(VdcBllMessages.VAR__TYPE__VM);
-        } else {
-            result = super.canDoAction();
+        if 
(!ImagesHandler.CheckImagesConfiguration(getStorageDomainId().getValue(),
+                getParameters().getDiskInfoList(), 
getReturnValue().getCanDoActionMessages())) {
+            return false;
         }
-        return result;
+
+        return super.canDoAction();
     }
 
     @Override


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4589fa4681913e41b1be595131686932d92ca062
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to