Roy Golan has uploaded a new change for review.

Change subject: core: log instead of CanDo on illegal memory value
......................................................................

core: log instead of CanDo on illegal memory value

Change-Id: I6ab029a1bec86cac86b0e29a8e9aaa38bcdf6c87
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1014952
Signed-off-by: Roy Golan <[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/AddVmTemplateCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java
7 files changed, 34 insertions(+), 49 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/78/20178/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 9ec850a..f7164ed 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
@@ -7,6 +7,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.lang.StringUtils;
 import org.ovirt.engine.core.bll.job.ExecutionHandler;
@@ -328,6 +329,9 @@
 
     @Override
     protected boolean canDoAction() {
+        // warn only on memory exceeding values
+        VmHandler.isMemorySizeLegal(getVm().getVmOsId(), 
getVm().getMemSizeMb(), getVdsGroup().getcompatibility_version());
+
         if (getVmTemplate() == null) {
             return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST);
         }
@@ -638,10 +642,6 @@
             if (!validatePinningAndMigration(reasons, vmStaticData, 
getParameters().getVm().getCpuPinning())) {
                 returnValue = false;
             }
-
-            returnValue = returnValue
-                    && VmHandler.isMemorySizeLegal(vmStaticData.getOsId(), 
vmStaticData.getMemSizeMb(),
-                            reasons, getVdsGroup().getcompatibility_version());
 
         }
         return returnValue;
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
index 3ed7e4f..9c03772 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java
@@ -241,13 +241,13 @@
 
     @Override
     protected boolean canDoAction() {
+        // warn only on memory exceeding values
+        VmHandler.isMemorySizeLegal(getParameters().getMasterVm().getOsId(),
+                getParameters().getMasterVm().getMemSizeMb(),
+                getVdsGroup().getcompatibility_version());
+
         if (getVdsGroup() == null) {
             addCanDoActionMessage(VdcBllMessages.VDS_CLUSTER_IS_NOT_VALID);
-            return false;
-        }
-        if 
(!VmHandler.isMemorySizeLegal(getParameters().getMasterVm().getOsId(),
-                getParameters().getMasterVm().getMemSizeMb(),
-                getReturnValue().getCanDoActionMessages(), 
getVdsGroup().getcompatibility_version())) {
             return false;
         }
         if 
(!isVmPriorityValueLegal(getParameters().getMasterVm().getPriority(), 
getReturnValue()
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 01dc391..dbbfbcf 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
@@ -35,7 +35,6 @@
 import org.ovirt.engine.core.common.osinfo.OsRepository;
 import org.ovirt.engine.core.common.utils.SimpleDependecyInjector;
 import org.ovirt.engine.core.compat.Guid;
-import org.ovirt.engine.core.compat.Version;
 import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector;
 import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase;
 import org.ovirt.engine.core.dal.job.ExecutionMessageDirector;
@@ -193,9 +192,9 @@
             return false;
         }
 
-        if (!isMemorySizeLegal(grp.getcompatibility_version())) {
-            return false;
-        }
+        // warn only on memory exceeding values
+        
VmHandler.isMemorySizeLegal(getParameters().getVmStaticData().getOsId(), 
getParameters().getVmStaticData()
+                .getMemSizeMb(), grp.getcompatibility_version());
 
         VmPool pool = 
getVmPoolDAO().getByName(getParameters().getVmPool().getName());
         if (pool != null
@@ -249,15 +248,6 @@
         }
 
         return checkFreeSpaceAndTypeOnDestDomains();
-    }
-
-    protected boolean isMemorySizeLegal(Version version) {
-        VmStatic vmStaticData = getParameters().getVmStaticData();
-        return VmHandler.isMemorySizeLegal
-                (vmStaticData.getOsId(),
-                        vmStaticData.getMemSizeMb(),
-                        getReturnValue().getCanDoActionMessages(),
-                        version);
     }
 
     protected boolean verifyAddVM() {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
index 95459f2..00c0d19 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
@@ -214,6 +214,11 @@
         VM vmFromDB = getVm();
         VM vmFromParams = getParameters().getVm();
 
+        // warn on memory exceeding the OS
+        VmHandler.isMemorySizeLegal(vmFromParams.getOs(),
+                vmFromParams.getMemSizeMb(),
+                getVdsGroup().getcompatibility_version());
+
         if (!isVmExist()) {
             
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_EXIST);
             return false;
@@ -249,12 +254,6 @@
         if (!validationErrors.isEmpty()) {
             
VmPropertiesUtils.getInstance().handleCustomPropertiesError(validationErrors,
                     getReturnValue().getCanDoActionMessages());
-            return false;
-        }
-
-        if (!VmHandler.isMemorySizeLegal(vmFromParams.getOs(),
-                vmFromParams.getMemSizeMb(), 
getReturnValue().getCanDoActionMessages(),
-                getVdsGroup().getcompatibility_version())) {
             return false;
         }
 
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java
index e030ee9..3bd9932 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java
@@ -69,23 +69,22 @@
         } else {
             if (getVdsGroup() == null) {
                 addCanDoActionMessage(VdcBllMessages.VMT_CLUSTER_IS_NOT_VALID);
-            } else if (VmHandler.isMemorySizeLegal(mOldTemplate.getOsId(),
-                    mOldTemplate.getMemSizeMb(),
-                    getReturnValue()
-                            .getCanDoActionMessages(),
-                    getVdsGroup().getcompatibility_version())) {
-                if 
(isVmPriorityValueLegal(getParameters().getVmTemplateData().getPriority(), 
getReturnValue()
-                        .getCanDoActionMessages())
-                        && 
isDomainLegal(getParameters().getVmTemplateData().getDomain(), getReturnValue()
-                        .getCanDoActionMessages())) {
-                    returnValue = 
VmTemplateHandler.isUpdateValid(mOldTemplate, getVmTemplate());
-                    if (!returnValue) {
-                        
addCanDoActionMessage(VdcBllMessages.VMT_CANNOT_UPDATE_ILLEGAL_FIELD);
-                    }
+            } else if 
(isVmPriorityValueLegal(getParameters().getVmTemplateData().getPriority(), 
getReturnValue()
+                    .getCanDoActionMessages())
+                    && 
isDomainLegal(getParameters().getVmTemplateData().getDomain(), getReturnValue()
+                            .getCanDoActionMessages())) {
+                returnValue = VmTemplateHandler.isUpdateValid(mOldTemplate, 
getVmTemplate());
+                if (!returnValue) {
+                    
addCanDoActionMessage(VdcBllMessages.VMT_CANNOT_UPDATE_ILLEGAL_FIELD);
                 }
             }
         }
 
+        // warn only on memory exceeding values
+        VmHandler.isMemorySizeLegal(mOldTemplate.getOsId(),
+                mOldTemplate.getMemSizeMb(),
+                getVdsGroup().getcompatibility_version());
+
         // Check that the USB policy is legal
         if (returnValue) {
             returnValue = 
VmHandler.isUsbPolicyLegal(getParameters().getVmTemplateData().getUsbPolicy(), 
getParameters().getVmTemplateData().getOsId(), getVdsGroup(), 
getReturnValue().getCanDoActionMessages());
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
index faa06e4..edae08c 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
@@ -328,20 +328,18 @@
      *            Type of the os.
      * @param memSizeInMB
      *            The mem size in MB.
-     * @param reasons
-     *            The reasons.VdsGroups
      * @return
      */
     public static boolean isMemorySizeLegal(int osId,
                                             int memSizeInMB,
-                                            List<String> reasons,
                                             Version clusterVersion) {
         boolean result = VmValidationUtils.isMemorySizeLegal(osId, 
memSizeInMB, clusterVersion);
         if (!result) {
-            
reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_ILLEGAL_MEMORY_SIZE.toString());
-            reasons.add(String.format("$minMemorySize %s", 
VmValidationUtils.getMinMemorySizeInMb(osId, clusterVersion)));
-            reasons.add(String.format("$maxMemorySize %s",
-                    VmValidationUtils.getMaxMemorySizeInMb(osId, 
clusterVersion)));
+            log.warnFormat("The VM RAM {0}mb is not between {1}mb and {2}mb",
+                    memSizeInMB,
+                    VmValidationUtils.getMinMemorySizeInMb(osId, 
clusterVersion),
+                    VmValidationUtils.getMaxMemorySizeInMb(osId, 
clusterVersion)
+                    );
         }
         return result;
     }
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java
index 0b6de67..b265bcf 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommandTestAbstract.java
@@ -126,7 +126,6 @@
     protected void setUpCommand() {
         command = createCommand();
         
doReturn(true).when(command).areTemplateImagesInStorageReady(any(Guid.class));
-        doReturn(true).when(command).isMemorySizeLegal(any(Version.class));
         doReturn(true).when(command).verifyAddVM();
     }
 


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

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

Reply via email to