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. In doing so, space allocation validation for stateless VM became redundant and was therefore removed. Change-Id: Ib3f7c7f31d738f84d356d4e89b05a21d4ca7a118 Bug-Url: https://bugzilla.redhat.com/1149809 Signed-off-by: Vered Volansky <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java 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 3 files changed, 19 insertions(+), 54 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/76/33876/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java index d21f833..a72b160 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java @@ -23,7 +23,6 @@ import org.ovirt.engine.core.bll.scheduling.VdsFreeMemoryChecker; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; -import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.bll.validator.RunVmValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.FeatureSupported; @@ -39,7 +38,6 @@ import org.ovirt.engine.core.common.businessentities.ActionGroup; import org.ovirt.engine.core.common.businessentities.BootSequence; import org.ovirt.engine.core.common.businessentities.Disk; -import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.DisplayType; import org.ovirt.engine.core.common.businessentities.Entities; import org.ovirt.engine.core.common.businessentities.ImageFileType; @@ -853,10 +851,6 @@ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_RNG_SOURCE_NOT_SUPPORTED); } - if (isRunAsStateless()) { - return validateSpaceRequirements(); - } - return true; } @@ -879,14 +873,6 @@ } return true; - } - - protected boolean validateSpaceRequirements() { - fetchVmDisksFromDb(); - List<DiskImage> disksList = getVm().getDiskList(); - MultipleStorageDomainsValidator msdValidator = createMultipleStorageDomainsValidator(disksList); - return validate(msdValidator.allDomainsHaveSpaceForNewDisks(disksList)) - && validate(msdValidator.allDomainsWithinThresholds()); } @Override 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..502b288 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,20 @@ * 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 msdValidator = getStorageDomainsValidator(sdIds); + ValidationResult retVal = msdValidator.allDomainsWithinThresholds(); + if (retVal == ValidationResult.VALID) { + return msdValidator.allDomainsHaveSpaceForNewDisks(getVmImageDisks()); + } + return retVal; + } + + protected MultipleStorageDomainsValidator getStorageDomainsValidator(Collection<Guid> sdIds) { + Guid spId = vm.getStoragePoolId(); + return new MultipleStorageDomainsValidator(spId, sdIds); } protected ValidationResult validateStoragePoolUp(VM vm, StoragePool storagePool, List<DiskImage> vmImages) { @@ -503,25 +502,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/33876 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib3f7c7f31d738f84d356d4e89b05a21d4ca7a118 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Vered Volansky <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
