[Engine-patches] Change in ovirt-engine[master]: core: simplified RunVmValidator#canRunVm method

[ahadas]

Arik Hadas has uploaded a new change for review.

Change subject: core: simplified RunVmValidator#canRunVm method
......................................................................

core: simplified RunVmValidator#canRunVm method

The RunVmValidator#canRunVm should be a utility method that invoke the
validations which are required for running a VM, but it was also used to
have logic which determines whether to invoke images related validations
or not.

This patch changes the canRunVm method to invoke all the validations
(until the first failure) without any filtering. Instead, The images
related validations are checking whether the VM has images or not
inside.

Change-Id: I277a24fa26bb9aae3641aa316689ff7350e9d63f
Signed-off-by: Arik Hadas <aha...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
1 file changed, 26 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/42/18242/1

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 742ed25..42f44f6 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
@@ -62,6 +62,7 @@
     private boolean isInternalExecution;
 
     private List<Disk> cachedVmDisks;
+    private List<DiskImage> cachedVmImageDisks;
 
     public RunVmValidator(VM vm, RunVmParams rumVmParam, boolean 
isInternalExecution) {
         this.vm = vm;
@@ -94,18 +95,13 @@
                 !validate(new VmValidator(vm).vmNotLocked(), messages) ||
                 
!validate(getSnapshotValidator().vmNotDuringSnapshot(vm.getId()), messages) ||
                 !validate(validateVmStatusUsingMatrix(vm), messages) ||
+                !validate(validateStoragePoolUp(vm, storagePool, 
getVmImageDisks()), messages) ||
                 !validate(validateIsoPath(vm, runVmParam.getDiskPath(), 
runVmParam.getFloppyPath()), messages)  ||
                 !validate(vmDuringInitialization(vm), messages) ||
                 !validate(validateVdsStatus(vm), messages) ||
-                !validate(validateStatelessVm(vm, getVmDisks(), 
runVmParam.getRunAsStateless()), messages)) {
-            return false;
-        }
-
-        List<DiskImage> images = ImagesHandler.filterImageDisks(getVmDisks(), 
true, false);
-        if (!images.isEmpty() && (
-                !validate(validateStoragePoolUp(vm, storagePool), messages) ||
-                !validate(validateStorageDomains(vm, isInternalExecution, 
images), messages) ||
-                !validate(validateImagesForRunVm(vm, images), messages))) {
+                !validate(validateStatelessVm(vm, getVmDisks(), 
runVmParam.getRunAsStateless()), messages) ||
+                !validate(validateStorageDomains(vm, isInternalExecution, 
getVmImageDisks()), messages) ||
+                !validate(validateImagesForRunVm(vm, getVmImageDisks()), 
messages)) {
             return false;
         }
 
@@ -177,6 +173,10 @@
      */
     public ValidationResult validateStorageDomains(VM vm, boolean 
isInternalExecution,
             List<DiskImage> vmImages) {
+        if (vmImages.isEmpty()) {
+            return ValidationResult.VALID;
+        }
+
         if (!vm.isAutoStartup() || !isInternalExecution) {
             Set<Guid> storageDomainIds = 
ImagesHandler.getAllStorageIdsForImageIds(vmImages);
             MultipleStorageDomainsValidator storageDomainValidator =
@@ -201,6 +201,10 @@
      * Check isValid only if VM is not HA VM
      */
     public ValidationResult validateImagesForRunVm(VM vm, List<DiskImage> 
vmDisks) {
+        if (vmDisks.isEmpty()) {
+            return ValidationResult.VALID;
+        }
+
         return !vm.isAutoStartup() ?
                 new DiskImagesValidator(vmDisks).diskImagesNotLocked() : 
ValidationResult.VALID;
     }
@@ -299,9 +303,12 @@
         return ValidationResult.VALID;
     }
 
-    public ValidationResult validateStoragePoolUp(VM vm, StoragePool 
storagePool) {
-        return !vm.isAutoStartup() ?
-                new StoragePoolValidator(storagePool).isUp() : 
ValidationResult.VALID;
+    public ValidationResult validateStoragePoolUp(VM vm, StoragePool 
storagePool, List<DiskImage> vmImages) {
+        if (vmImages.isEmpty() || vm.isAutoStartup()) {
+            return ValidationResult.VALID;
+        }
+
+        return new StoragePoolValidator(storagePool).isUp();
     }
 
     /**
@@ -494,4 +501,11 @@
         }
         return cachedVmDisks;
     }
+
+    private List<DiskImage> getVmImageDisks() {
+        if (cachedVmImageDisks == null) {
+            cachedVmImageDisks = ImagesHandler.filterImageDisks(getVmDisks(), 
true, false);
+        }
+        return cachedVmImageDisks;
+    }
 }


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I277a24fa26bb9aae3641aa316689ff7350e9d63f
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches