Arik Hadas has uploaded a new change for review.

Change subject: core: refactor CommonVmPoolWithVmsCommand
......................................................................

core: refactor CommonVmPoolWithVmsCommand

- Extract the code that add VMs to the pool to separate method
- Convert the field that contains the configuration value of the number
  of maximum number of subsequent failures to create VMs to final static
  class member
- Rename addVmsSucceded to addVmsSucceeded
- Remove the variable that reflected whether at least one VM creation
  failed as we can know that according to the addVmsSucceeded field

Change-Id: If9cc7214071e0b92aaa65480f0c5c801538d2c50
Signed-off-by: Arik Hadas <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java
1 file changed, 16 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/25/28325/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java
index d569fa3..b152c83 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java
@@ -57,8 +57,10 @@
     protected HashMap<Guid, DiskImage> diskInfoDestinationMap;
     protected Map<Guid, List<DiskImage>> storageToDisksMap;
     protected Map<Guid, StorageDomain> destStorages = new HashMap<>();
-    private boolean addVmsSucceded = true;
+    private boolean addVmsSucceeded = true;
     private NameForVmInPoolGenerator nameForVmInPoolGenerator;
+
+    private static final int VM_POOL_MAX_SUBSEQUENT_FAILURES = 
Config.<Integer> getValue(ConfigValues.VmPoolMaxSubsequentFailures);
 
     /**
      * Constructor for command creation when compensation is applied on startup
@@ -118,14 +120,21 @@
         VmHandler.warnMemorySizeLegal(getParameters().getVmStaticData(), 
getVdsGroup().getcompatibility_version());
 
         Guid poolId = getPoolId();
-        boolean isAtLeastOneVMCreationFailed = false;
         setActionReturnValue(poolId);
-
         
VmTemplateHandler.lockVmTemplateInTransaction(getParameters().getVmStaticData().getVmtGuid(),
                 getCompensationContext());
 
+        addVmsToPool(poolId);
+
+        getReturnValue().setCanDoAction(getAddVmsSucceded());
+        setSucceeded(getAddVmsSucceded());
+        
VmTemplateHandler.unlockVmTemplate(getParameters().getVmStaticData().getVmtGuid());
+        getCompensationContext().resetCompensation();
+    }
+
+    private void addVmsToPool(Guid poolId) {
         int subsequentFailedAttempts = 0;
-        int vmPoolMaxSubsequentFailures = Config.<Integer> 
getValue(ConfigValues.VmPoolMaxSubsequentFailures);
+
         for (int i=0; i<getParameters().getVmsCount(); i++) {
             String currentVmName = generateUniqueVmName();
             VdcReturnValueBase returnValue =
@@ -139,25 +148,19 @@
                         getReturnValue().getCanDoActionMessages().add(msg);
                     }
                 }
-                addVmsSucceded = returnValue.getSucceeded() && addVmsSucceded;
+                addVmsSucceeded = false;
                 subsequentFailedAttempts++;
             }
             else { // Succeed on that , reset subsequentFailedAttempts.
                 subsequentFailedAttempts = 0;
             }
             // if subsequent attempts failure exceeds configuration value , 
abort the loop.
-            if (subsequentFailedAttempts == vmPoolMaxSubsequentFailures) {
+            if (subsequentFailedAttempts == VM_POOL_MAX_SUBSEQUENT_FAILURES) {
                 AuditLogableBase logable = new AuditLogableBase();
                 AuditLogDirector.log(logable, 
AuditLogType.USER_VM_POOL_MAX_SUBSEQUENT_FAILURES_REACHED);
                 break;
             }
-            isAtLeastOneVMCreationFailed = isAtLeastOneVMCreationFailed || 
!addVmsSucceded;
         }
-
-        getReturnValue().setCanDoAction(!isAtLeastOneVMCreationFailed);
-        setSucceeded(!isAtLeastOneVMCreationFailed);
-        
VmTemplateHandler.unlockVmTemplate(getParameters().getVmStaticData().getVmtGuid());
-        getCompensationContext().resetCompensation();
     }
 
     private String generateUniqueVmName() {
@@ -385,7 +388,7 @@
     }
 
     protected boolean getAddVmsSucceded() {
-        return addVmsSucceded;
+        return addVmsSucceeded;
     }
 
     public String getVmsCount() {


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: If9cc7214071e0b92aaa65480f0c5c801538d2c50
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