Vered Volansky has uploaded a new change for review. Change subject: core: Fix storage allocation check when creating a VM snapshot ......................................................................
core: Fix storage allocation check when creating a VM snapshot Allocation checks for memory and data volumes were done either way - whether the VM is running or not. This patch takes these volumes into consideration only when the VM is running. Change-Id: I085d49dc90053e375f829900c75d2fe72c263161 Bug-Url: https://bugzilla.redhat.com/1053752 Bug-Url: https://bugzilla.redhat.com/1119022 Signed-off-by: Vered Volansky <vvola...@redhat.com> --- 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/ImagesHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java 4 files changed, 64 insertions(+), 35 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/89/36889/1 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 e072ad6..27012bb 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 @@ -142,19 +142,38 @@ return toReturn; } - private boolean validateStorageDomains(List<DiskImage> vmDisksList, List<DiskImage> memoryDisksList) { - List<DiskImage> disksList = getAllDisks(vmDisksList, memoryDisksList); - MultipleStorageDomainsValidator sdValidator = createMultipleStorageDomainsValidator(disksList); - return validate(sdValidator.allDomainsExistAndActive()) - && validate(sdValidator.allDomainsWithinThresholds()) - && validate(sdValidator.allDomainsHaveSpaceForAllDisks(vmDisksList, memoryDisksList)); - } + private boolean validateStorageDomains() { + List<DiskImage> vmDisksList = getDisksListForChecks(); + if (vmDisksList.size() > 0) { + DiskImagesValidator diskImagesValidator = createDiskImageValidator(vmDisksList); + if (!(validate(diskImagesValidator.diskImagesNotLocked()) + && validate(diskImagesValidator.diskImagesNotIllegal()))) { + return false; + } + } + vmDisksList = ImagesHandler.getDisksDummiesForStorageAllocations(vmDisksList); + List<DiskImage> allDisks = new ArrayList<>(vmDisksList); - private static List<DiskImage> getAllDisks(List<DiskImage> newDisksList, List<DiskImage> cloneDisksList) { - List<DiskImage> disksList = new ArrayList<>(); - disksList.addAll(newDisksList); - disksList.addAll(cloneDisksList); - return disksList; + List<DiskImage> memoryDisksList = null; + if (getParameters().isSaveMemory()) { + memoryDisksList = MemoryUtils.createDiskDummies(getVm().getTotalMemorySizeInBytes(), MemoryUtils.META_DATA_SIZE_IN_BYTES); + if (Guid.Empty.equals(getStorageDomainIdForVmMemory(memoryDisksList))) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NO_SUITABLE_DOMAIN_FOUND); + } + allDisks.addAll(memoryDisksList); + } + + MultipleStorageDomainsValidator sdValidator = createMultipleStorageDomainsValidator(allDisks); + if (!validate(sdValidator.allDomainsExistAndActive()) + || !validate(sdValidator.allDomainsWithinThresholds())) { + return false; + } + + if (memoryDisksList == null) { //no memory volumes + return validate(sdValidator.allDomainsHaveSpaceForNewDisks(vmDisksList)); + } + + return validate(sdValidator.allDomainsHaveSpaceForAllDisks(vmDisksList, memoryDisksList)); } protected MemoryImageBuilder getMemoryImageBuilder() { @@ -500,26 +519,7 @@ return false; } - List<DiskImage> disksList = getDisksListForChecks(); - if (disksList.size() > 0) { - DiskImagesValidator diskImagesValidator = createDiskImageValidator(disksList); - if (!(validate(diskImagesValidator.diskImagesNotLocked()) - && validate(diskImagesValidator.diskImagesNotIllegal()))) { - return false; - } - } - - List<DiskImage> memoryDisksList = MemoryUtils.createDiskDummies(getVm().getTotalMemorySizeInBytes(), MemoryUtils.META_DATA_SIZE_IN_BYTES); - getStorageDomainIdForVmMemory(memoryDisksList); - if (getParameters().isSaveMemory() && Guid.Empty.equals(cachedStorageDomainId)) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NO_SUITABLE_DOMAIN_FOUND); - } - - if (!validateStorageDomains(disksList, memoryDisksList)) { - return false; - } - - return true; + return validateStorageDomains(); } protected StoragePoolValidator createStoragePoolValidator() { 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 622aacb..6bb5899 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 @@ -866,8 +866,8 @@ * which could have another volume type/format than the target disk volume type/format, which is not yet created. * "Real" override for these values is done in CreateSnapshotCommand, when creating the new DiskImages. */ - public static Collection<DiskImage> getDisksDummiesForStorageAllocations(Collection<DiskImage> originalDisks) { - Collection<DiskImage> diskDummies = new HashSet<>(); + public static List<DiskImage> getDisksDummiesForStorageAllocations(Collection<DiskImage> originalDisks) { + List<DiskImage> diskDummies = new ArrayList<>(originalDisks.size()); for (DiskImage diskImage : originalDisks) { DiskImage clone = DiskImage.copyOf(diskImage); clone.setVolumeType(VolumeType.Sparse); 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 907a1df..0f892aa 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 @@ -1,6 +1,7 @@ package org.ovirt.engine.core.bll; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Set; @@ -563,7 +564,7 @@ return true; } - protected MultipleStorageDomainsValidator createMultipleStorageDomainsValidator(List<DiskImage> disksList) { + protected MultipleStorageDomainsValidator createMultipleStorageDomainsValidator(Collection<DiskImage> disksList) { return new MultipleStorageDomainsValidator(getVm().getStoragePoolId(), ImagesHandler.getAllStorageIdsForImageIds(disksList)); } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java index e09601c..8ba7163 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java @@ -263,6 +263,29 @@ } @Test + public void testAllDomainsHaveSpaceForNewDisksFailure() { + setUpGeneralValidations(); + setUpDiskValidations(); + List<DiskImage> disksList = getNonEmptyDiskList(); + doReturn(disksList).when(cmd).getDisksList(); + doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN)).when(multipleStorageDomainsValidator) + .allDomainsHaveSpaceForNewDisks(disksList); + CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN); + verify(multipleStorageDomainsValidator).allDomainsHaveSpaceForNewDisks(disksList); + } + + @Test + public void testAllDomainsHaveSpaceForNewDisksSuccess() { + setUpGeneralValidations(); + setUpDiskValidations(); + List<DiskImage> disksList = getNonEmptyDiskList(); + doReturn(disksList).when(cmd).getDisksList(); + doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsHaveSpaceForNewDisks(disksList); + CanDoActionTestUtils.runAndAssertCanDoActionSuccess(cmd); + verify(multipleStorageDomainsValidator).allDomainsHaveSpaceForNewDisks(disksList); + } + + @Test public void testAllDomainsWithinThreshold() { setUpGeneralValidations(); setUpDiskValidations(); @@ -280,6 +303,8 @@ setUpGeneralValidations(); setUpDiskValidations(); doReturn(Collections.emptyList()).when(cmd).getDisksList(); + cmd.getParameters().setSaveMemory(true); + doReturn(Guid.newGuid()).when(cmd).getStorageDomainIdForVmMemory(eq(Collections.<DiskImage>emptyList())); doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN)).when(multipleStorageDomainsValidator) .allDomainsHaveSpaceForAllDisks(eq(Collections.<DiskImage>emptyList()), anyList()); CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN); @@ -291,6 +316,8 @@ setUpGeneralValidations(); setUpDiskValidations(); doReturn(Collections.emptyList()).when(cmd).getDisksList(); + cmd.getParameters().setSaveMemory(true); + doReturn(Guid.newGuid()).when(cmd).getStorageDomainIdForVmMemory(eq(Collections.<DiskImage>emptyList())); CanDoActionTestUtils.runAndAssertCanDoActionSuccess(cmd); verify(multipleStorageDomainsValidator).allDomainsHaveSpaceForAllDisks(eq(Collections.<DiskImage>emptyList()), anyList()); } @@ -301,6 +328,7 @@ doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsExistAndActive(); doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsWithinThresholds(); doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsHaveSpaceForAllDisks(anyList(), anyList()); + doReturn(ValidationResult.VALID).when(multipleStorageDomainsValidator).allDomainsHaveSpaceForNewDisks(anyList()); } private void setUpGeneralValidations() { -- To view, visit http://gerrit.ovirt.org/36889 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I085d49dc90053e375f829900c75d2fe72c263161 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vered Volansky <vvola...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
