Vered Volansky has uploaded a new change for review. Change subject: core: Add memory allocation check to hibernate VM ......................................................................
core: Add memory allocation check to hibernate VM When hibernating a VM, two volumes are created, one to save memory, and one for the metadata. When searching for a Storage Domain with enough space for these volumes, wrong space allocations checks are done. Added the correct storage allocation checks, replacing existing checks in HibernateVmCommand. Added test class to the command - HibernateVmCommandTest, as well as a test to VmHandlerTest. Change-Id: I1250e6ea8d67f8026d66cf06589538343d39756a Bug-Url: https://bugzilla.redhat.com/1124143 Signed-off-by: Vered Volansky <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryUtils.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java 4 files changed, 136 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/54/32054/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java index 2671aa2..a5702e8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Map; import org.ovirt.engine.core.bll.memory.MemoryUtils; @@ -16,6 +17,7 @@ import org.ovirt.engine.core.common.action.VmOperationParameterBase; import org.ovirt.engine.core.common.asynctasks.AsyncTaskType; import org.ovirt.engine.core.common.asynctasks.EntityInfo; +import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageType; @@ -88,9 +90,8 @@ @Override public Guid getStorageDomainId() { if (_storageDomainId.equals(Guid.Empty) && getVm() != null) { - long sizeNeeded = (getVm().getTotalMemorySizeInBytes() - + META_DATA_SIZE_IN_BYTES) / BYTES_IN_GB; - StorageDomain storageDomain = VmHandler.findStorageDomainForMemory(getVm().getStoragePoolId(), sizeNeeded); + List<DiskImage> diskDummiesForMemSize = MemoryUtils.createDiskDummies(getVm().getTotalMemorySizeInBytes(), META_DATA_SIZE_IN_BYTES); + StorageDomain storageDomain = VmHandler.findStorageDomainForMemory(getVm().getStoragePoolId(), diskDummiesForMemSize); if (storageDomain != null) { _storageDomainId = storageDomain.getId(); } @@ -273,7 +274,6 @@ return true; } - @Override protected void setActionMessageParameters() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java index f722bf6..a060a7e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java @@ -33,6 +33,7 @@ import org.ovirt.engine.core.common.businessentities.EditableOnVmStatusField; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; +import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.businessentities.UsbPolicy; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VM; @@ -41,6 +42,7 @@ import org.ovirt.engine.core.common.businessentities.VmDynamic; import org.ovirt.engine.core.common.businessentities.VmInit; import org.ovirt.engine.core.common.businessentities.VmStatic; +import org.ovirt.engine.core.common.businessentities.VolumeType; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; import org.ovirt.engine.core.common.businessentities.network.VmNic; import org.ovirt.engine.core.common.config.Config; @@ -704,11 +706,51 @@ return null; } - protected static boolean doesStorageDomainHaveSpaceForRequest(StorageDomain storageDomain, long sizeRequested) { + private static boolean doesStorageDomainHaveSpaceForRequest(StorageDomain storageDomain, long sizeRequested) { // not calling validate in order not to add the messages per domain return (new StorageDomainValidator(storageDomain).isDomainHasSpaceForRequest(sizeRequested)).isValid(); } + /** + * Returns a <code>StorageDomain</code> in the given <code>StoragePool</code> that has + * at least as much as requested free space and can be used to store memory images + * + * @param storagePoolId + * The storage pool where the search for a domain will be made + * @param disksList + * Disks for which space is needed + * @return storage domain in the given pool with at least the required amount of free space, + * or null if no such storage domain exists in the pool + */ + public static StorageDomain findStorageDomainForMemory(Guid storagePoolId, List<DiskImage> disksList) { + List<StorageDomain> domainsInPool = DbFacade.getInstance().getStorageDomainDao().getAllForStoragePool(storagePoolId); + return findStorageDomainForMemory(domainsInPool, disksList); + } + + protected static StorageDomain findStorageDomainForMemory(List<StorageDomain> domainsInPool, List<DiskImage> disksList) { + for (StorageDomain currDomain : domainsInPool) { + //There should be two disks in the disksList, first of which is memory disk. Only its volume type should be modified. + updateDisksStorageType(currDomain.getStorageType(), disksList.get(0)); + if (currDomain.getStorageDomainType().isDataDomain() + && currDomain.getStatus() == StorageDomainStatus.Active + && validateSpaceRequirements(currDomain, disksList)) { + return currDomain; + } + } + return null; + } + + private static void updateDisksStorageType(StorageType storageType, DiskImage disk) { + VolumeType volumeType = storageType.isFileDomain() ? VolumeType.Sparse : VolumeType.Preallocated; + disk.setVolumeType(volumeType); + } + + private static boolean validateSpaceRequirements(StorageDomain storageDomain, List<DiskImage> disksList) { + StorageDomainValidator storageDomainValidator = new StorageDomainValidator(storageDomain); + return (storageDomainValidator.isDomainWithinThresholds().isValid() && + storageDomainValidator.hasSpaceForClonedDisks(disksList).isValid()); + } + public static ValidationResult canRunActionOnNonManagedVm(VM vm, VdcActionType actionType) { ValidationResult validationResult = ValidationResult.VALID; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryUtils.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryUtils.java index 02de6d6..49480a4 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryUtils.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryUtils.java @@ -1,11 +1,15 @@ package org.ovirt.engine.core.bll.memory; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; import org.apache.commons.lang.StringUtils; +import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.Snapshot; +import org.ovirt.engine.core.common.businessentities.VolumeFormat; +import org.ovirt.engine.core.common.businessentities.VolumeType; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.utils.GuidUtils; @@ -42,4 +46,23 @@ memories.remove(StringUtils.EMPTY); return memories; } + + public static List<DiskImage> createDiskDummies(long memorySize, long metadataSize) { + DiskImage memoryVolume = new DiskImage(); + memoryVolume.setDiskAlias("memory"); + memoryVolume.setvolumeFormat(VolumeFormat.RAW); + memoryVolume.setSize(memorySize); + memoryVolume.setActualSizeInBytes(memorySize); + memoryVolume.getSnapshots().add(memoryVolume); + + DiskImage dataVolume = new DiskImage(); + memoryVolume.setDiskAlias("metadata"); + dataVolume.setvolumeFormat(VolumeFormat.COW); + dataVolume.setVolumeType(VolumeType.Sparse); + dataVolume.setSize(metadataSize); + dataVolume.setActualSizeInBytes(metadataSize); + dataVolume.getSnapshots().add(dataVolume); + + return Arrays.asList(memoryVolume, dataVolume); + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java index f795882..340ee81 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java @@ -52,9 +52,14 @@ // - +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; + import java.util.ArrayList; import java.util.Arrays; @@ -62,23 +67,43 @@ import java.util.List; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.ovirt.engine.core.bll.memory.MemoryUtils; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.ImageStatus; import org.ovirt.engine.core.common.businessentities.LunDisk; +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.StorageType; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; import org.ovirt.engine.core.common.businessentities.VmDeviceId; import org.ovirt.engine.core.common.businessentities.VmStatic; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; +import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.common.utils.SizeConverter; import org.ovirt.engine.core.common.utils.VmDeviceType; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.utils.MockConfigRule; import org.ovirt.engine.core.utils.RandomUtils; /** A test case for the {@link VmHandler} class. */ public class VmHandlerTest { + + public static final long META_DATA_SIZE_IN_GB = 1; + public static final Integer LOW_SPACE_IN_GB = 3; + public static final Integer ENOUGH_SPACE_IN_GB = 4; + public static final Integer THRESHOLD_IN_GB = 4; + public static final Integer THRESHOLD_HIGH_GB = 10; + public static final int VM_SPACE_IN_MB = 2000; + + @Rule + public MockConfigRule mcr = new MockConfigRule( + mockConfig(ConfigValues.FreeSpaceCriticalLowInGB, THRESHOLD_IN_GB)); @Before public void setUp() { @@ -129,6 +154,44 @@ assertFalse(vm.getManagedVmDeviceMap().containsKey(snapshotDisk.getId())); } + @Test + public void verifyDomainForMemory() { + Guid sdId = Guid.newGuid(); + List<StorageDomain> storageDomains = createStorageDomains(sdId); + long vmSpaceInBytes = SizeConverter.convert(VM_SPACE_IN_MB, SizeConverter.SizeUnit.MB, SizeConverter.SizeUnit.BYTES).intValue(); + List<DiskImage> disksList = MemoryUtils.createDiskDummies(vmSpaceInBytes, META_DATA_SIZE_IN_GB); + + StorageDomain storageDomain = VmHandler.findStorageDomainForMemory(storageDomains, disksList); + assertThat(storageDomain, notNullValue()); + if (storageDomain != null) { + Guid selectedId = storageDomain.getId(); + assertThat(selectedId.equals(sdId), is(true)); + } + + mcr.mockConfigValue(ConfigValues.FreeSpaceCriticalLowInGB, THRESHOLD_HIGH_GB); + + storageDomain = VmHandler.findStorageDomainForMemory(storageDomains, disksList); + assertThat(storageDomain, nullValue()); + } + + private static List<StorageDomain> createStorageDomains(Guid sdIdToBeSelected) { + StorageDomain sd1 = createStorageDomain(Guid.newGuid(), StorageType.NFS, LOW_SPACE_IN_GB); + StorageDomain sd2 = createStorageDomain(Guid.newGuid(), StorageType.NFS, LOW_SPACE_IN_GB); + StorageDomain sd3 = createStorageDomain(sdIdToBeSelected, StorageType.NFS, ENOUGH_SPACE_IN_GB); + List<StorageDomain> storageDomains = Arrays.asList(sd1, sd2, sd3); + return storageDomains; + } + + private static StorageDomain createStorageDomain(Guid guid, StorageType storageType, Integer size) { + StorageDomain storageDomain = new StorageDomain(); + storageDomain.setId(guid); + storageDomain.setStorageDomainType(StorageDomainType.Data); + storageDomain.setStorageType(storageType); + storageDomain.setStatus(StorageDomainStatus.Active); + storageDomain.setAvailableDiskSize(size); + return storageDomain; + } + private void populateVmWithDisks(List<Disk> disks, VM vm) { VmHandler.updateDisksForVm(vm, disks); for (Disk disk : disks) { @@ -154,7 +217,7 @@ return lunDisk; } - private DiskImage createDiskImage(boolean active) { + private static DiskImage createDiskImage(boolean active) { DiskImage di = new DiskImage(); di.setActive(active); di.setId(Guid.newGuid()); @@ -315,3 +378,4 @@ // assertFalse(VmHandler.verifyAddVm(new ArrayList<String>(), 0, null, Guid.NewGuid(), 0)); // } //} + -- To view, visit http://gerrit.ovirt.org/32054 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1250e6ea8d67f8026d66cf06589538343d39756a 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
