Vered Volansky has uploaded a new change for review.

Change subject: core: RunVmValidator storage allocation
......................................................................

core: RunVmValidator storage allocation

This patch is a part of a series of patches, adding storage allocation
validations to the system when they're missing, and replacing old
verification usage with unified, new, correct and tested verification.
This patch did this for RunVmValidator.
This also resulted in removing tests of old/buggy verification.

Change-Id: Ib3f7c7f31d738f84d356d4e89b05a21d4ca7a118
Signed-off-by: Vered Volansky <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
2 files changed, 14 insertions(+), 40 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/00/33800/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 707177e..70a94f8 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
@@ -1,11 +1,9 @@
 package org.ovirt.engine.core.bll.validator;
 
 import java.util.Arrays;
-import java.util.HashMap;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Set;
 
 import org.apache.commons.lang.StringUtils;
@@ -27,7 +25,6 @@
 import org.ovirt.engine.core.common.businessentities.Entities;
 import org.ovirt.engine.core.common.businessentities.ImageFileType;
 import org.ovirt.engine.core.common.businessentities.RepoImage;
-import org.ovirt.engine.core.common.businessentities.StorageDomain;
 import org.ovirt.engine.core.common.businessentities.StoragePool;
 import org.ovirt.engine.core.common.businessentities.VDSGroup;
 import org.ovirt.engine.core.common.businessentities.VDSStatus;
@@ -118,7 +115,7 @@
                 validate(validateStoragePoolUp(vm, storagePool, 
getVmImageDisks()), messages) &&
                 validate(validateIsoPath(vm, runVmParam.getDiskPath(), 
runVmParam.getFloppyPath(), activeIsoDomainId), messages)  &&
                 validate(vmDuringInitialization(vm), messages) &&
-                validate(validateStatelessVm(vm, getVmDisks(), 
runVmParam.getRunAsStateless()), messages) &&
+                validate(validateStatelessVm(vm, 
runVmParam.getRunAsStateless()), messages) &&
                 validate(validateStorageDomains(vm, isInternalExecution, 
getVmImageDisks()), messages) &&
                 validate(validateImagesForRunVm(vm, getVmImageDisks()), 
messages) &&
                 validate(validateMemorySize(vm), messages) &&
@@ -312,7 +309,7 @@
         return ValidationResult.VALID;
     }
 
-    protected ValidationResult validateStatelessVm(VM vm, List<Disk> 
plugDisks, Boolean stateless) {
+    protected ValidationResult validateStatelessVm(VM vm, Boolean stateless) {
         // if the VM is not stateless, there is nothing to check
         if (stateless != null ? !stateless : !vm.isStateless()) {
             return ValidationResult.VALID;
@@ -328,7 +325,7 @@
             return new 
ValidationResult(VdcBllMessages.VM_CANNOT_RUN_STATELESS_HA);
         }
 
-        ValidationResult hasSpaceValidation = hasSpaceForSnapshots(vm, 
plugDisks);
+        ValidationResult hasSpaceValidation = hasSpaceForSnapshots();
         if (!hasSpaceValidation.isValid()) {
             return hasSpaceValidation;
         }
@@ -349,18 +346,15 @@
      * check that we can create snapshots for all disks
      * return true if all storage domains have enough space to create 
snapshots for this VM plugged disks
      */
-    protected ValidationResult hasSpaceForSnapshots(VM vm, List<Disk> 
plugDisks) {
-        Integer minSnapshotSize = Config.<Integer> 
getValue(ConfigValues.InitStorageSparseSizeInGB);
-        Map<StorageDomain, Integer> mapStorageDomainsToNumOfDisks = 
mapStorageDomainsToNumOfDisks(vm, plugDisks);
-        for (Entry<StorageDomain, Integer> e : 
mapStorageDomainsToNumOfDisks.entrySet()) {
-            ValidationResult validationResult =
-                    new 
StorageDomainValidator(e.getKey()).isDomainHasSpaceForRequest(minSnapshotSize * 
e.getValue());
-            if (!validationResult.isValid()) {
-                return validationResult;
-            }
-        }
+    protected ValidationResult hasSpaceForSnapshots() {
+        Set<Guid> sdIds = 
ImagesHandler.getAllStorageIdsForImageIds(getVmImageDisks());
 
-        return ValidationResult.VALID;
+        MultipleStorageDomainsValidator storageDomainsValidator = 
getStorageDomainsValidator(vm.getStoragePoolId(), sdIds);
+        return 
storageDomainsValidator.allDomainsHaveSpaceForNewDisks(getVmImageDisks());
+    }
+
+    protected MultipleStorageDomainsValidator getStorageDomainsValidator(Guid 
spId, Collection<Guid> sdIds) {
+        return new MultipleStorageDomainsValidator(spId, sdIds);
     }
 
     protected ValidationResult validateStoragePoolUp(VM vm, StoragePool 
storagePool, List<DiskImage> vmImages) {
@@ -503,25 +497,6 @@
 
     protected SnapshotsValidator getSnapshotValidator() {
         return new SnapshotsValidator();
-    }
-
-    /**
-     * map the VM number of pluggable and snapable disks from their domain.
-     * @param vm
-     * @param plugDisks
-     * @return
-     */
-    public Map<StorageDomain, Integer> mapStorageDomainsToNumOfDisks(VM vm, 
List<Disk> plugDisks) {
-        Map<StorageDomain, Integer> map = new HashMap<StorageDomain, 
Integer>();
-        for (Disk disk : plugDisks) {
-            if (disk.isAllowSnapshot()) {
-                for (StorageDomain domain : 
getStorageDomainDAO().getAllStorageDomainsByImageId(((DiskImage) 
disk).getImageId())) {
-                    map.put(domain, map.containsKey(domain) ? 
Integer.valueOf(map.get(domain) + 1) : Integer.valueOf(1));
-                }
-            }
-        }
-
-        return map;
     }
 
     private VdsDynamic getVdsDynamic(Guid vdsId) {
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
index 37587d1..556ca2e 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/RunVmValidatorTest.java
@@ -277,15 +277,14 @@
             };
 
             @Override
-            public ValidationResult hasSpaceForSnapshots(VM vm, List<Disk> 
plugDisks) {
+            public ValidationResult hasSpaceForSnapshots() {
                 return ValidationResult.VALID;
             }
         };
         VM vm = new VM();
         vm.setAutoStartup(autoStartUp);
         vm.setStateless(isVmStateless);
-        List<Disk> disks = new ArrayList<Disk>();
-        validateResult(runVmValidator.validateStatelessVm(vm, disks, 
isStatelessParam),
+        validateResult(runVmValidator.validateStatelessVm(vm, 
isStatelessParam),
                 shouldPass,
                 message);
     }


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

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

Reply via email to