Arik Hadas has uploaded a new change for review.

Change subject: core: check vm custom properties from parameters on run vm
......................................................................

core: check vm custom properties from parameters on run vm

On run VM we get a VM object from the DB and parameters with custom
attributes for the run operation, mainly changes that are set as part of
run once mode. When validating that it is possible to run the VM, we use
to check the values from the parameters and the default values for the
VM. One exception is the validation of the custom properties where we
check the actual value within the VM object which enforce us to set it
before the can-do-action phase of the run command.

Thus, this patch changes the custom properties validation in run VM flow
such that it will check the custom properties from the parameters.

In addition, minor refactoring was made to the validation of boot
sequence on run VM.

Change-Id: I66cec8aa76958831924a48b8a7805cf0d181dcf2
Signed-off-by: Arik Hadas <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
M 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java
3 files changed, 16 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/05/25405/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java
index f0b77c8..f02027f 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java
@@ -383,9 +383,9 @@
     }
 
     protected List<ValidationError> validateCustomProperties(VmStatic 
vmStaticFromParams) {
-        return VmPropertiesUtils.getInstance().validateVMProperties(
+        return VmPropertiesUtils.getInstance().validateVmProperties(
                 getVdsGroup().getcompatibility_version(),
-                vmStaticFromParams);
+                vmStaticFromParams.getCustomProperties());
     }
 
     /**
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
index aea07f2..9621953 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
@@ -105,7 +105,7 @@
         }
 
         return
-                validateVmProperties(vm, messages) &&
+                validateVmProperties(vm, runVmParam.getCustomProperties(), 
messages) &&
                 validate(validateBootSequence(vm, 
runVmParam.getBootSequence(), getVmDisks(), activeIsoDomainId), messages) &&
                 validate(new VmValidator(vm).vmNotLocked(), messages) &&
                 
validate(getSnapshotValidator().vmNotDuringSnapshot(vm.getId()), messages) &&
@@ -143,38 +143,39 @@
     }
 
 
-    protected boolean validateVmProperties(VM vm, List<String> messages) {
+    protected boolean validateVmProperties(VM vm, String vmProperties, 
List<String> messages) {
         List<ValidationError> validationErrors =
-                getVmPropertiesUtils().validateVMProperties(
+                getVmPropertiesUtils().validateVmProperties(
                         vm.getVdsGroupCompatibilityVersion(),
-                        vm.getStaticData());
+                        vmProperties);
 
         if (!validationErrors.isEmpty()) {
-            
VmPropertiesUtils.getInstance().handleCustomPropertiesError(validationErrors, 
messages);
+            
getVmPropertiesUtils().handleCustomPropertiesError(validationErrors, messages);
             return false;
         }
 
         return true;
     }
 
-    protected ValidationResult validateBootSequence(VM vm, BootSequence 
bootSequence, List<Disk> vmDisks, Guid activeIsoDomainId) {
-        BootSequence boot_sequence = (bootSequence != null) ?
-                bootSequence : vm.getDefaultBootSequence();
+    protected ValidationResult validateBootSequence(VM vm, BootSequence 
customBootSequence,
+            List<Disk> vmDisks, Guid activeIsoDomainId) {
+        BootSequence bootSequence = (customBootSequence != null) ?
+                customBootSequence : vm.getDefaultBootSequence();
         // Block from running a VM with no HDD when its first boot device is
         // HD and no other boot devices are configured
-        if (boot_sequence == BootSequence.C && vmDisks.isEmpty()) {
+        if (bootSequence == BootSequence.C && vmDisks.isEmpty()) {
             return new 
ValidationResult(VdcBllMessages.VM_CANNOT_RUN_FROM_DISK_WITHOUT_DISK);
         }
 
         // If CD appears as first and there is no ISO in storage
         // pool/ISO inactive - you cannot run this VM
-        if (boot_sequence == BootSequence.CD && activeIsoDomainId == null) {
+        if (bootSequence == BootSequence.CD && activeIsoDomainId == null) {
             return new 
ValidationResult(VdcBllMessages.VM_CANNOT_RUN_FROM_CD_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO);
         }
 
         // if there is network in the boot sequence, check that the
         // vm has network, otherwise the vm cannot be run in vdsm
-        if (boot_sequence == BootSequence.N
+        if (bootSequence == BootSequence.N
                 && getVmNicDao().getAllForVm(vm.getId()).isEmpty()) {
             return new 
ValidationResult(VdcBllMessages.VM_CANNOT_RUN_FROM_NETWORK_WITHOUT_NETWORK);
         }
diff --git 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java
 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java
index ebb778f..6b6c07b 100644
--- 
a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java
+++ 
b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/customprop/VmPropertiesUtils.java
@@ -112,24 +112,12 @@
     }
 
     /**
-     * Validates all the custom properties of a VM
-     *
-     * @param vmStatic
-     * @return
-     */
-    public List<ValidationError> validateVMProperties(Version version, 
VmStatic vmStatic) {
-        List<ValidationError> errors = validateProperties(version, 
vmStatic.getCustomProperties());
-        return errors;
-    }
-
-    /**
      * Validates a properties field value (checks if its format matches 
key1=val1;key2=val2;....)
      *
      * @param fieldValue
      * @return a list of validation errors. if there are no errors - the list 
will be empty
      */
-    private List<ValidationError> validateProperties(Version version, String 
properties) {
-
+    public List<ValidationError> validateVmProperties(Version version, String 
properties) {
         if (StringUtils.isEmpty(properties)) { // No errors in case of empty 
value
             return Collections.emptyList();
         }
@@ -142,8 +130,7 @@
         // 2. Check if the value of the key is valid
         // In case either 1 or 2 fails, add an error to the errors list
         Map<String, String> map = new HashMap<String, String>();
-        List<ValidationError> result = populateVMProperties(version, 
properties, map);
-        return result;
+        return populateVMProperties(version, properties, map);
     }
 
     private boolean isValueValid(Version version, String key, String value) {


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

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