Allon Mureinik has uploaded a new change for review. Change subject: core: Extract SD validations from ImagesHandler ......................................................................
core: Extract SD validations from ImagesHandler Extracted the logic to perform validations on Storage Domains from ImagesHandler.PerformImagesChecks, and made the relevant commands use it directly. In order to do this, a new type of validator was introduced, MultipleStoroageDomainsValidator, which aggregates validations from individual StorageDomainValidators in a fast-fail pattern. This is done for performance reasons, so there is no need to load all the storage domains from the database preemptively. This refactoring removes three parameters from PerformImagesChecks: * checkStorageDomain and diskSpaceCheck - are now done directly by MultipleStorageDomainsValidator. * storageDomainId - is used by the previous two checks, and by the image existence check. However, it is always equal to the storage domain id member of the DiskImage, so it is redundant as an additional parameter. Change-Id: I4cfc895dceed4de520476ed5e3fa9c1c7cfd78f4 Signed-off-by: Allon Mureinik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.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/CreateAllSnapshotsFromVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateCommand.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidatorTest.java 19 files changed, 361 insertions(+), 140 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/49/12249/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java index 3b127ea..f7cc736 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java @@ -32,6 +32,7 @@ import org.ovirt.engine.core.common.businessentities.LUNs; import org.ovirt.engine.core.common.businessentities.LunDisk; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId; import org.ovirt.engine.core.common.businessentities.StorageServerConnections; import org.ovirt.engine.core.common.businessentities.StorageType; @@ -40,7 +41,6 @@ import org.ovirt.engine.core.common.businessentities.VmDeviceId; import org.ovirt.engine.core.common.businessentities.VolumeType; import org.ovirt.engine.core.common.businessentities.permissions; -import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.storage_pool; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; @@ -199,10 +199,7 @@ return ImagesHandler.PerformImagesChecks( getReturnValue().getCanDoActionMessages(), spId, - getStorageDomainId().getValue(), - false, true, - false, false, false, true, 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 c77f4e5..7acb0c9 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 @@ -16,6 +16,7 @@ import org.ovirt.engine.core.bll.storage.StoragePoolValidator; 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.StorageDomainValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; @@ -25,6 +26,7 @@ import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainType; import org.ovirt.engine.core.common.businessentities.VMStatus; @@ -33,7 +35,6 @@ import org.ovirt.engine.core.common.businessentities.VmTemplate; import org.ovirt.engine.core.common.businessentities.VmTemplateStatus; import org.ovirt.engine.core.common.businessentities.permissions; -import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.network.VmInterfaceType; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; import org.ovirt.engine.core.common.errors.VdcBLLException; @@ -202,22 +203,21 @@ return false; } - for (Guid srcStorageDomainId : sourceImageDomainsImageMap.keySet()) { - boolean checkIsValid = true; - if (!ImagesHandler.PerformImagesChecks( - getReturnValue().getCanDoActionMessages(), - getVm().getStoragePoolId(), - srcStorageDomainId, - false, - true, - true, - true, - true, - checkIsValid, - sourceImageDomainsImageMap.get(srcStorageDomainId))) { - return false; - } - checkIsValid = false; + MultipleStorageDomainsValidator storageDomainsValidator = + new MultipleStorageDomainsValidator(getStoragePoolId(), sourceImageDomainsImageMap.keySet()); + if (!validate(storageDomainsValidator.allDomainsExistAndActive())) { + return false; + } + + if (!ImagesHandler.PerformImagesChecks( + getReturnValue().getCanDoActionMessages(), + getVm().getStoragePoolId(), + true, + true, + true, + true, + mImages)) { + return false; } Map<Guid, StorageDomain> storageDomains = new HashMap<Guid, StorageDomain>(); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java index 67ce42f..ae4ef22 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java @@ -13,6 +13,7 @@ import org.ovirt.engine.core.bll.snapshots.SnapshotsManager; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.storage.StoragePoolValidator; +import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; @@ -286,6 +287,9 @@ if (result && disksList.size() > 0) { StoragePoolValidator spValidator = new StoragePoolValidator(getStoragePool()); SnapshotsValidator snapshotValidator = new SnapshotsValidator(); + MultipleStorageDomainsValidator sdValidator = + new MultipleStorageDomainsValidator(getVm().getStoragePoolId(), + ImagesHandler.getAllStorageIdsForImageIds(disksList)); result = validate(spValidator.isUp()) && validate(snapshotValidator.vmNotDuringSnapshot(getVmId())) && validate(snapshotValidator.vmNotInPreview(getVmId())) @@ -295,15 +299,14 @@ && ImagesHandler.PerformImagesChecks( getReturnValue().getCanDoActionMessages(), getVm().getStoragePoolId(), - Guid.Empty, - true, - true, true, true, true, true, disksList) - && validate(vmValidator.vmNotLocked()); + && validate(vmValidator.vmNotLocked()) + && validate(sdValidator.allDomainsExistAndActive()) + && validate(sdValidator.allDomainsWithinThresholds()); } return result; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java index fc9889a..78adb84 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmCommand.java @@ -13,6 +13,7 @@ import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.storage.StoragePoolValidator; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; +import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.bll.validator.StorageDomainValidator; import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.AuditLogType; @@ -27,12 +28,12 @@ import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.Snapshot; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainType; import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VmTemplate; import org.ovirt.engine.core.common.businessentities.VolumeFormat; -import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.queries.DiskImageList; @@ -157,15 +158,14 @@ && validate(snapshotValidator.vmNotDuringSnapshot(getVmId())) && validate(snapshotValidator.vmNotInPreview(getVmId())) && validate(new VmValidator(getVm()).vmDown()) + && validate(new MultipleStorageDomainsValidator(getVm().getStoragePoolId(), + ImagesHandler.getAllStorageIdsForImageIds(getDisksBasedOnImage())).allDomainsExistAndActive()) && ImagesHandler.PerformImagesChecks( getReturnValue().getCanDoActionMessages(), getVm().getStoragePoolId(), - Guid.Empty, - false, true, false, false, - true, true, getDisksBasedOnImage()))) { return false; @@ -174,6 +174,7 @@ return true; } + @Override protected boolean doesStorageDomainhaveSpaceForRequest(StorageDomain storageDomain, long sizeRequested) { return validate(new StorageDomainValidator(storageDomain).isDomainHasSpaceForRequest(sizeRequested)); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java index 081fb3a..757cd02 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java @@ -2,7 +2,6 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -23,6 +22,7 @@ import org.ovirt.engine.core.common.businessentities.ImageStatus; import org.ovirt.engine.core.common.businessentities.LUNs; import org.ovirt.engine.core.common.businessentities.LunDisk; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatic; import org.ovirt.engine.core.common.businessentities.StorageServerConnections; import org.ovirt.engine.core.common.businessentities.StorageType; @@ -32,7 +32,6 @@ import org.ovirt.engine.core.common.businessentities.VolumeFormat; import org.ovirt.engine.core.common.businessentities.VolumeType; import org.ovirt.engine.core.common.businessentities.image_storage_domain_map; -import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.common.utils.ListUtils; @@ -365,17 +364,15 @@ return String.format("%1$s/%2$s", isoPrefix, fileName); } - public static boolean isImagesExists(Iterable<DiskImage> images, Guid storagePoolId, Guid storageDomainId) { - return isImagesExists(images, storagePoolId, storageDomainId, new ArrayList<DiskImage>()); + public static boolean isImagesExists(Iterable<DiskImage> images, Guid storagePoolId) { + return isImagesExists(images, storagePoolId, new ArrayList<DiskImage>()); } - private static boolean isImagesExists(Iterable<DiskImage> images, Guid storagePoolId, Guid domainId, - ArrayList<DiskImage> irsImages) { + private static boolean isImagesExists(Iterable<DiskImage> images, Guid storagePoolId, ArrayList<DiskImage> irsImages) { boolean returnValue = true; for (DiskImage image : images) { - Guid storageDomainId = !Guid.Empty.equals(domainId) ? domainId : image.getStorageIds().get(0); - DiskImage fromIrs = isImageExist(storagePoolId, storageDomainId, image); + DiskImage fromIrs = isImageExist(storagePoolId, image); if (fromIrs == null) { returnValue = false; break; @@ -386,14 +383,10 @@ return returnValue; } - private static DiskImage isImageExist(Guid storagePoolId, - Guid domainId, - DiskImage image) { + private static DiskImage isImageExist(Guid storagePoolId, DiskImage image) { DiskImage fromIrs = null; try { - Guid storageDomainId = - image.getStorageIds() != null && !image.getStorageIds().isEmpty() ? image.getStorageIds() - .get(0) : domainId; + Guid storageDomainId = image.getStorageIds().get(0); Guid imageGroupId = image.getId() != null ? image.getId() : Guid.Empty; fromIrs = (DiskImage) Backend .getInstance() @@ -440,12 +433,9 @@ public static boolean PerformImagesChecks( List<String> messages, Guid storagePoolId, - Guid storageDomainId, - boolean diskSpaceCheck, boolean checkImagesLocked, boolean checkImagesIllegal, boolean checkImagesExist, - boolean checkStorageDomain, boolean checkIsValid, Collection<? extends Disk> diskImageList) { @@ -460,11 +450,8 @@ returnValue = returnValue && checkDiskImages(messages, storagePoolId, - storageDomainId, - diskSpaceCheck, checkImagesIllegal, checkImagesExist, - checkStorageDomain, images); } else if (checkImagesExist) { returnValue = false; @@ -507,43 +494,14 @@ private static boolean checkDiskImages(List<String> messages, Guid storagePoolId, - Guid storageDomainId, - boolean diskSpaceCheck, boolean checkImagesIllegal, boolean checkImagesExist, - boolean checkStorageDomain, List<DiskImage> images) { boolean returnValue = true; ArrayList<DiskImage> irsImages = new ArrayList<DiskImage>(); - if (diskSpaceCheck || checkStorageDomain) { - Set<Guid> domainsIds; - if (!Guid.Empty.equals(storageDomainId)) { - domainsIds = Collections.singleton(storageDomainId); - } else { - domainsIds = getAllStorageIdsForImageIds(images); - } - - for (Guid domainId : domainsIds) { - StorageDomain domain = DbFacade.getInstance().getStorageDomainDao().getForStoragePool( - domainId, storagePoolId); - if (checkStorageDomain) { - StorageDomainValidator storageDomainValidator = - new StorageDomainValidator(domain); - ValidationResult res = storageDomainValidator.isDomainExistAndActive(); - returnValue = res.isValid(); - if (!returnValue) { - messages.add(res.getMessage().toString()); - } - } - if (diskSpaceCheck && returnValue && !isStorageDomainWithinThresholds(domain, messages, true)) { - return false; - } - } - } - if (returnValue && checkImagesExist) { - boolean isImagesExist = isImagesExists(images, storagePoolId, storageDomainId, irsImages); + boolean isImagesExist = isImagesExists(images, storagePoolId, irsImages); if (!isImagesExist) { returnValue = false; ListUtils.nullSafeAdd(messages, VdcBllMessages.ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST.toString()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java index a32642d..e604d49 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveVmCommand.java @@ -75,12 +75,9 @@ ImagesHandler.PerformImagesChecks( getReturnValue().getCanDoActionMessages(), getVm().getStoragePoolId(), - Guid.Empty, - false, true, true, true, - false, true, diskImages); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java index 6aebde1..de5ab6f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java @@ -225,34 +225,33 @@ } private boolean canRemoveVmImageDisk() { - boolean firstTime = true; - SnapshotsValidator snapshotsValidator = new SnapshotsValidator(); List<Disk> diskList = Arrays.asList(getDisk()); if (!listVms.isEmpty()) { - storage_pool sp = getStoragePoolDAO().get(listVms.get(0).getStoragePoolId()); + Guid storagePoolId = listVms.get(0).getStoragePoolId(); + storage_pool sp = getStoragePoolDAO().get(storagePoolId); if (!validate(new StoragePoolValidator(sp).isUp())) { + return false; + } + + if (!ImagesHandler.PerformImagesChecks( + getReturnValue().getCanDoActionMessages(), + storagePoolId, + true, + false, + false, + true, + diskList)) { return false; } } + SnapshotsValidator snapshotsValidator = new SnapshotsValidator(); for (VM vm : listVms) { if (!validate(snapshotsValidator.vmNotDuringSnapshot(vm.getId())) || - !validate(snapshotsValidator.vmNotInPreview(vm.getId())) || - !ImagesHandler.PerformImagesChecks( - getReturnValue().getCanDoActionMessages(), - vm.getStoragePoolId(), - getParameters().getStorageDomainId(), - false, - firstTime, - false, - false, - false, - firstTime, - diskList)) { + !validate(snapshotsValidator.vmNotInPreview(vm.getId()))) { return false; } - firstTime = false; } return true; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java index da03de6..39cb8d8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java @@ -191,7 +191,7 @@ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CANNOT_REMOVE_ACTIVE_IMAGE); } - if (!isEnoughSpaceToMergeSnapshots()) { + if (!validateStorageDomains()) { return false; } } @@ -200,24 +200,33 @@ } /** - * Validates if each storage domain has enough free space to perform removeSnapshot. <BR/> - * The remove snapshot logic in VDSM include creating a new temporary volume which might be large as the disk's + * Validates the storage domains. + * + * Each domain is validated for status and for enough free space to perform removeSnapshot. <BR/> + * The remove snapshot logic in VDSM includes creating a new temporary volume which might be as large as the disk's * actual size. <BR/> * Hence, as part of the validation, we sum up all the disks virtual sizes, for each storage domain. * * @return True if there is enough space in all relevant storage domains. False otherwise. */ - protected boolean isEnoughSpaceToMergeSnapshots() { + protected boolean validateStorageDomains() { for (final Entry<Guid, List<DiskImage>> storageToDiskEntry : getStorageToDiskMap().entrySet()) { Guid storageDomainId = storageToDiskEntry.getKey(); + StorageDomain storageDomain = + getStorageDomainDAO().getForStoragePool(storageDomainId, getStoragePoolId()); + StorageDomainValidator validator = new StorageDomainValidator(storageDomain); + + if (!validate(validator.isDomainExistAndActive())) { + return false; + } + List<DiskImage> diskImages = storageToDiskEntry.getValue(); long sizeRequested = 0l; for (DiskImage diskImage : diskImages) { sizeRequested += diskImage.getActualSize(); } - StorageDomain storageDomain = - getStorageDomainDAO().getForStoragePool(storageDomainId, getStoragePoolId()); - if (!validate(new StorageDomainValidator(storageDomain).isDomainHasSpaceForRequest(sizeRequested, false))) { + + if (!validate(validator.isDomainHasSpaceForRequest(sizeRequested, false))) { return false; } } @@ -252,8 +261,8 @@ protected boolean validateImages() { return ImagesHandler.PerformImagesChecks(getReturnValue().getCanDoActionMessages(), - getVm().getStoragePoolId(), Guid.Empty, - false, true, true, true, true, true, getDiskDao().getAllForVm(getVmId())); + getVm().getStoragePoolId(), + true, true, true, true, getDiskDao().getAllForVm(getVmId())); } protected boolean validateImageNotInTemplate() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java index 18029f5..2e05a1b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java @@ -5,6 +5,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.job.ExecutionHandler; @@ -15,6 +16,7 @@ import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.storage.StoragePoolValidator; import org.ovirt.engine.core.bll.utils.PermissionSubject; +import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.RemoveAllVmImagesParameters; @@ -155,18 +157,23 @@ Collection<Disk> vmDisks = getVm().getDiskMap().values(); List<DiskImage> vmImages = ImagesHandler.filterImageDisks(vmDisks, true, false); - if (!vmImages.isEmpty() && !ImagesHandler.PerformImagesChecks( + if (!vmImages.isEmpty()) { + Set<Guid> storageIds = ImagesHandler.getAllStorageIdsForImageIds(vmImages); + MultipleStorageDomainsValidator storageValidator = new MultipleStorageDomainsValidator(getVm().getStoragePoolId(), storageIds); + if (!validate(storageValidator.allDomainsExistAndActive())) { + return false; + } + + if (!ImagesHandler.PerformImagesChecks( getReturnValue().getCanDoActionMessages(), getVm().getStoragePoolId(), - Guid.Empty, - false, !getParameters().getForce(), false, false, true, - true, vmImages)) { - return false; + return false; + } } // Handle VM status with ImageLocked diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java index 1478cdf..a98f1ae 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java @@ -15,6 +15,7 @@ import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.storage.StoragePoolValidator; import org.ovirt.engine.core.bll.utils.PermissionSubject; +import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; @@ -301,7 +302,13 @@ SnapshotsValidator snapshotValidator = createSnapshotValidator(); VmValidator vmValidator = new VmValidator(getVm()); if (!validate(snapshotValidator.snapshotExists(getVmId(), getParameters().getDstSnapshotId())) || - !validate(new StoragePoolValidator(getStoragePool()).isUp()) || + !validate(new StoragePoolValidator(getStoragePool()).isUp())) { + return false; + } + + MultipleStorageDomainsValidator storageValidator = createStorageDomainValidator(); + if (!validate(storageValidator.allDomainsExistAndActive()) || + !validate(storageValidator.allDomainsWithinThresholds()) || !performImagesChecks() || !validate(vmValidator.vmDown())) { return false; @@ -326,16 +333,18 @@ return new SnapshotsValidator(); } + protected MultipleStorageDomainsValidator createStorageDomainValidator() { + Set<Guid> storageIds = ImagesHandler.getAllStorageIdsForImageIds(getImagesList()); + return new MultipleStorageDomainsValidator(getStoragePoolId(), storageIds); + } + protected boolean performImagesChecks() { return ImagesHandler.PerformImagesChecks (getReturnValue().getCanDoActionMessages(), getVm().getStoragePoolId(), - Guid.Empty, - true, true, false, false, - true, true, getImagesList()); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java index 9994a47..5db8238 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java @@ -4,11 +4,13 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import org.ovirt.engine.core.bll.job.ExecutionHandler; import org.ovirt.engine.core.bll.snapshots.SnapshotsManager; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.storage.StoragePoolValidator; +import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; @@ -214,16 +216,18 @@ Collection<DiskImage> diskImages = ImagesHandler.filterImageDisks(getVm().getDiskMap().values(), false, true); if (!diskImages.isEmpty()) { + Set<Guid> storageIds = ImagesHandler.getAllStorageIdsForImageIds(diskImages); + MultipleStorageDomainsValidator storageValidator = + new MultipleStorageDomainsValidator(getVm().getStoragePoolId(), storageIds); if (!validate(new StoragePoolValidator(getStoragePool()).isUp()) + || !validate(storageValidator.allDomainsExistAndActive()) + || !validate(storageValidator.allDomainsWithinThresholds()) || !ImagesHandler.PerformImagesChecks( getReturnValue().getCanDoActionMessages(), getVm().getStoragePoolId(), - Guid.Empty, - true, true, false, false, - true, true, diskImages)) { return false; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java index 376dd27..c497180 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java @@ -208,10 +208,7 @@ if (!ImagesHandler.PerformImagesChecks( messages, vm.getStoragePoolId(), - storageDomainId, - false, true, - false, false, false, true, diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java index 48a95f4..f252543 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java @@ -6,11 +6,13 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Set; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.storage.StoragePoolValidator; +import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.bll.validator.StorageDomainValidator; import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.VdcActionUtils; @@ -20,6 +22,7 @@ import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.RepoFileMetaData; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainType; import org.ovirt.engine.core.common.businessentities.VDS; @@ -27,7 +30,6 @@ import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.businessentities.VmDevice; -import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.storage_pool; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; @@ -130,6 +132,20 @@ retValue = false; } + if (retValue && (!vm.isAutoStartup() || !runParams.getIsInternal())) { + Set<Guid> storageDomainIds = ImagesHandler.getAllStorageIdsForImageIds(vmImages); + MultipleStorageDomainsValidator storageDomainValidator = + new MultipleStorageDomainsValidator(vm.getStoragePoolId(), storageDomainIds); + if (!validateAllDomainsUp(storageDomainValidator, message)) { + retValue = false; + } + + if (retValue && !vm.isAutoStartup() + && !validateAllDomainsThresholds(storageDomainValidator, message)) { + retValue = false; + } + } + if (retValue && !performImageChecksForRunningVm(vm, message, runParams, vmImages)) { retValue = false; } @@ -196,6 +212,15 @@ return retValue; } + protected boolean validateAllDomainsUp(MultipleStorageDomainsValidator storageDomainValidator, List<String> message) { + return validate(storageDomainValidator.allDomainsExistAndActive(), message); + } + + protected boolean validateAllDomainsThresholds(MultipleStorageDomainsValidator storageDomainValidator, + List<String> message) { + return validate(storageDomainValidator.allDomainsWithinThresholds(), message); + } + /** * check that we can create snapshots for all disks * @@ -216,7 +241,7 @@ return validate(new StorageDomainValidator(storageDomain).isDomainHasSpaceForRequest(sizeRequested), message); } - protected boolean validate(ValidationResult validationResult, ArrayList<String> message) { + protected boolean validate(ValidationResult validationResult, List<String> message) { if (!validationResult.isValid()) { message.add(validationResult.getMessage().name()); if (validationResult.getVariableReplacements() != null) { @@ -252,9 +277,8 @@ protected boolean performImageChecksForRunningVm (VM vm, List<String> message, RunVmParams runParams, List<DiskImage> vmDisks) { return ImagesHandler.PerformImagesChecks(message, - vm.getStoragePoolId(), Guid.Empty, !vm.isAutoStartup(), + vm.getStoragePoolId(), true, false, false, - !vm.isAutoStartup() || !runParams.getIsInternal(), !vm.isAutoStartup() || !runParams.getIsInternal(), vmDisks); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateCommand.java index 43b99b6..bcb0929 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateCommand.java @@ -88,8 +88,7 @@ .getAllForVm(vmTemplate.getId()), false, false); } if (vmtImages.size() > 0 - && !ImagesHandler.isImagesExists(vmtImages, vmtImages.get(0).getStoragePoolId().getValue(), - storageDomainId)) { + && !ImagesHandler.isImagesExists(vmtImages, vmtImages.get(0).getStoragePoolId().getValue())) { reasons.add(VdcBllMessages.TEMPLATE_IMAGE_NOT_EXIST.toString()); returnValue = false; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java new file mode 100644 index 0000000..fd7b226 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java @@ -0,0 +1,103 @@ +package org.ovirt.engine.core.bll.validator; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; + +import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.compat.NGuid; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dao.StorageDomainDAO; + +/** + * A validator for multiple storage domains. + * + * This class offers several validations similar to those offered in + * {@link StorageDomainValidator} which can be performed on several domains. + * + * The guideline of this class is performance and short circuit logic, so any + * of these validations will fail on the first storage domain which fails + * validation, and the others will not be inspected. + */ +public class MultipleStorageDomainsValidator { + + /** The ID of the storage pool all the domains belong to */ + private NGuid spId; + + /** A map from the ids of each domain being validated to its validator */ + private Map<Guid, StorageDomainValidator> domainValidators; + + /** + * Constructor from Guids + * @param sdIds A {@link Collection} of storage domain IDs to be validated + */ + public MultipleStorageDomainsValidator(NGuid spId, Collection<Guid> sdIds) { + this.spId = spId; + domainValidators = new HashMap<Guid, StorageDomainValidator>(sdIds.size()); + for (Guid id : sdIds) { + domainValidators.put(id, null); + } + } + + /** + * Validates that all the domains exist and are active. + * @return {@link ValidationResult#VALID} if all the domains are OK, or a {@link ValidationResult} with the first non-active domain encountered. + */ + public ValidationResult allDomainsExistAndActive() { + return validOrFirstFailure(new ValidatorPredicate() { + @Override + public ValidationResult evaluate(StorageDomainValidator validator) { + return validator.isDomainExistAndActive(); + } + }); + } + + /** + * Validates that all the domains are withing free disk space threshold. + * @return {@link ValidationResult#VALID} if all the domains are OK, or a {@link ValidationResult} with the first low space domain encountered. + */ + public ValidationResult allDomainsWithinThresholds() { + return validOrFirstFailure(new ValidatorPredicate() { + @Override + public ValidationResult evaluate(StorageDomainValidator validator) { + return validator.isDomainWithinThresholds(); + } + }); + } + + /** @return The lazy-loaded validator for the given map entry */ + private StorageDomainValidator getStorageDomainValidator(Map.Entry<Guid, StorageDomainValidator> entry) { + if (entry.getValue() == null) { + entry.setValue(new StorageDomainValidator(getStorageDomainDAO().getForStoragePool(entry.getKey(), spId))); + } + + return entry.getValue(); + } + + /** @return The DAO object used to retrieve storage domains */ + protected StorageDomainDAO getStorageDomainDAO() { + return DbFacade.getInstance().getStorageDomainDao(); + } + + /** + * Validates all the storage domains by a given predicate. + * + * @return {@link ValidationResult#VALID} if all the domains are OK, or the + * first validation error if they aren't. + */ + private ValidationResult validOrFirstFailure(ValidatorPredicate predicate) { + for (Map.Entry<Guid, StorageDomainValidator> entry : domainValidators.entrySet()) { + ValidationResult currResult = predicate.evaluate(getStorageDomainValidator(entry)); + if (!currResult.isValid()) { + return currResult; + } + } + return ValidationResult.VALID; + } + + /** A predicate for evaluating storage domains */ + private static interface ValidatorPredicate { + public ValidationResult evaluate(StorageDomainValidator validator); + } +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java index 1c4fe4d..b18cff6 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RemoveSnapshotCommandTest.java @@ -132,7 +132,7 @@ when(diskImageDAO.get(mockSourceImage())).thenReturn(new DiskImage()); mockStorageDomainDAOGetForStoragePool(10, STORAGE_DOMAIN_ID); assertTrue("Validation should succeed. Free space minus threshold should be bigger then disk size", - cmd.isEnoughSpaceToMergeSnapshots()); + cmd.validateStorageDomains()); } @Test @@ -140,7 +140,7 @@ when(diskImageDAO.get(mockSourceImage())).thenReturn(new DiskImage()); mockStorageDomainDAOGetForStoragePool(3, STORAGE_DOMAIN_ID); assertFalse("Validation should fail. Free space minus threshold should be smaller then disk size", - cmd.isEnoughSpaceToMergeSnapshots()); + cmd.validateStorageDomains()); } @Test @@ -149,7 +149,7 @@ doReturn(imagesDisks).when(cmd).getSourceImages(); mockStorageDomainDAOGetForStoragePool(22, STORAGE_DOMAIN_ID); assertTrue("Validation should succeed. Free space minus threshold should be bigger then summarize all disks size", - cmd.isEnoughSpaceToMergeSnapshots()); + cmd.validateStorageDomains()); } @Test @@ -158,7 +158,7 @@ doReturn(imagesDisks).when(cmd).getSourceImages(); mockStorageDomainDAOGetForStoragePool(15, STORAGE_DOMAIN_ID); assertFalse("Validation should fail. Free space minus threshold should be smaller then summarize all disks size", - cmd.isEnoughSpaceToMergeSnapshots()); + cmd.validateStorageDomains()); } @Test @@ -169,7 +169,7 @@ mockStorageDomainDAOGetForStoragePool(22, STORAGE_DOMAIN_ID); mockStorageDomainDAOGetForStoragePool(22, STORAGE_DOMAIN_ID2); assertTrue("Validation should succeed. Free space minus threshold should be bigger then summarize all disks size for each domain", - cmd.isEnoughSpaceToMergeSnapshots()); + cmd.validateStorageDomains()); } @Test @@ -180,7 +180,7 @@ mockStorageDomainDAOGetForStoragePool(15, STORAGE_DOMAIN_ID); mockStorageDomainDAOGetForStoragePool(22, STORAGE_DOMAIN_ID2); assertFalse("Validation should fail. First domain should not have enough free space for request.", - cmd.isEnoughSpaceToMergeSnapshots()); + cmd.validateStorageDomains()); } @Test @@ -191,7 +191,7 @@ mockStorageDomainDAOGetForStoragePool(22, STORAGE_DOMAIN_ID); mockStorageDomainDAOGetForStoragePool(10, STORAGE_DOMAIN_ID2); assertFalse("Validation should fail. Second domain should not have enough free space for request.", - cmd.isEnoughSpaceToMergeSnapshots()); + cmd.validateStorageDomains()); } @Test @@ -202,7 +202,7 @@ mockStorageDomainDAOGetForStoragePool(10, STORAGE_DOMAIN_ID); mockStorageDomainDAOGetForStoragePool(10, STORAGE_DOMAIN_ID2); assertFalse("Validation should fail. Second domain should not have enough free space for request.", - cmd.isEnoughSpaceToMergeSnapshots()); + cmd.validateStorageDomains()); } @Test diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java index 7dd804d..f4d6345 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RestoreAllSnapshotCommandTest.java @@ -20,18 +20,19 @@ import org.mockito.runners.MockitoJUnitRunner; import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; +import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.common.action.RestoreAllSnapshotsParameters; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.Snapshot; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.businessentities.VmDynamic; -import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.storage_pool; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend; @@ -80,6 +81,9 @@ @Mock private VmNetworkInterfaceDao vmNetworkInterfaceDAO; + + @Mock + private MultipleStorageDomainsValidator storageValidator; private Guid vmId = Guid.NewGuid(); private Guid diskImageId = Guid.NewGuid(); @@ -148,8 +152,11 @@ RestoreAllSnapshotsParameters parameters = new RestoreAllSnapshotsParameters(vmId, dstSnapshotId); List<DiskImage> diskImageList = createDiskImageList(); parameters.setImagesList(diskImageList); + doReturn(ValidationResult.VALID).when(storageValidator).allDomainsExistAndActive(); + doReturn(ValidationResult.VALID).when(storageValidator).allDomainsWithinThresholds(); spyCommand = spy(new RestoreAllSnapshotsCommand<RestoreAllSnapshotsParameters>(parameters)); doReturn(true).when(spyCommand).performImagesChecks(); + doReturn(storageValidator).when(spyCommand).createStorageDomainValidator(); } private void mockDaos() { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java index f630a96..b506ef0 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java @@ -32,17 +32,18 @@ import org.mockito.stubbing.Answer; import org.ovirt.engine.core.bll.interfaces.BackendInternal; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; +import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.common.action.RunVmParams; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.IVdsAsyncCommand; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.VmDeviceId; import org.ovirt.engine.core.common.businessentities.VmStatic; -import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.storage_pool; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend; @@ -332,11 +333,14 @@ doReturn(spDao).when(vmRunHandler).getStoragePoolDAO(); doReturn(vmRunHandler).when(command).getVmRunHandler(); - + doReturn(true).when(vmRunHandler).validateAllDomainsUp(any(MultipleStorageDomainsValidator.class), + anyListOf(String.class)); + doReturn(true).when(vmRunHandler).validateAllDomainsThresholds(any(MultipleStorageDomainsValidator.class), + anyListOf(String.class)); doReturn(true).when(vmRunHandler).performImageChecksForRunningVm(any(VM.class), anyListOf(String.class), any(RunVmParams.class), - anyListOf(Disk.class)); + anyListOf(DiskImage.class)); } @Test diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidatorTest.java new file mode 100644 index 0000000..2f551f7 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidatorTest.java @@ -0,0 +1,103 @@ +package org.ovirt.engine.core.bll.validator; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; +import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; + +import java.util.Arrays; + +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; +import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dal.VdcBllMessages; +import org.ovirt.engine.core.dao.StorageDomainDAO; +import org.ovirt.engine.core.utils.MockConfigRule; + +/** A test class for the {@link MultipleStorageDomainsValidator} class. */ +@RunWith(MockitoJUnitRunner.class) +public class MultipleStorageDomainsValidatorTest { + + @ClassRule + public static MockConfigRule mcr = new MockConfigRule(mockConfig(ConfigValues.FreeSpaceCriticalLowInGB, 10)); + + @Mock + private StorageDomainDAO dao; + + private Guid spId; + + private Guid sdId1; + private Guid sdId2; + + private StorageDomain domain1; + private StorageDomain domain2; + + private MultipleStorageDomainsValidator validator; + + @Before + public void setUp() { + spId = Guid.NewGuid(); + + sdId1 = Guid.NewGuid(); + sdId2 = Guid.NewGuid(); + + domain1 = new StorageDomain(); + domain1.setId(sdId1); + + domain2 = new StorageDomain(); + domain2.setId(sdId2); + + when(dao.getForStoragePool(sdId1, spId)).thenReturn(domain1); + when(dao.getForStoragePool(sdId2, spId)).thenReturn(domain2); + + validator = spy(new MultipleStorageDomainsValidator(spId, Arrays.asList(sdId1, sdId2))); + doReturn(dao).when(validator).getStorageDomainDAO(); + } + + @Test + public void testAllDomainsExistAndActiveAllActive() { + domain1.setStatus(StorageDomainStatus.Active); + domain2.setStatus(StorageDomainStatus.Active); + assertTrue("Both domains should be active", validator.allDomainsExistAndActive().isValid()); + } + + @Test + public void testAllDomainsExistAndActiveOneInactive() { + domain1.setStatus(StorageDomainStatus.Active); + domain2.setStatus(StorageDomainStatus.InActive); + ValidationResult result = validator.allDomainsExistAndActive(); + assertFalse("Both domains should not be active", result.isValid()); + assertEquals("Wrong validation error", + VdcBllMessages.ACTION_TYPE_FAILED_STORAGE_DOMAIN_STATUS_ILLEGAL, + result.getMessage()); + } + + @Test + public void testAllDomainsWithinThresholdAllOk() { + domain1.getStorageDynamicData().setAvailableDiskSize(15); + domain2.getStorageDynamicData().setAvailableDiskSize(15); + assertTrue("Both domains should be withing space threshold", validator.allDomainsWithinThresholds().isValid()); + } + + @Test + public void testAllDomainsWithinThresholdsOneLacking() { + domain1.getStorageDynamicData().setAvailableDiskSize(15); + domain2.getStorageDynamicData().setAvailableDiskSize(7); + ValidationResult result = validator.allDomainsWithinThresholds(); + assertFalse("Both domains should not be withing thresholds", result.isValid()); + assertEquals("Wrong validation error", + VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_TARGET_STORAGE_DOMAIN, + result.getMessage()); + } +} -- To view, visit http://gerrit.ovirt.org/12249 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4cfc895dceed4de520476ed5e3fa9c1c7cfd78f4 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
