Idan Shaby has uploaded a new change for review. Change subject: core: move logic regarding memory storage handling ......................................................................
core: move logic regarding memory storage handling This patch moves the logic regarding memory storage handling from VmHandler to a more specific new handler, MemoryStorageHandler. Change-Id: I367ee3b1e902495472e687b6dad63bc305b2c8bb Bug-Url: https://bugzilla.redhat.com/1186230 Signed-off-by: Idan Shaby <ish...@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/HibernateVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandler.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandlerTest.java 7 files changed, 156 insertions(+), 120 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/33/40433/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 e293231..ee2d4d7 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 @@ -11,6 +11,7 @@ import org.ovirt.engine.core.bll.job.ExecutionHandler; import org.ovirt.engine.core.bll.memory.LiveSnapshotMemoryImageBuilder; import org.ovirt.engine.core.bll.memory.MemoryImageBuilder; +import org.ovirt.engine.core.bll.memory.MemoryStorageHandler; import org.ovirt.engine.core.bll.memory.MemoryUtils; import org.ovirt.engine.core.bll.memory.NullableMemoryImageBuilder; import org.ovirt.engine.core.bll.memory.StatelessSnapshotMemoryImageBuilder; @@ -212,7 +213,8 @@ public Guid getStorageDomainIdForVmMemory(List<DiskImage> memoryDisksList) { if (cachedStorageDomainId.equals(Guid.Empty) && getVm() != null) { - StorageDomain storageDomain = VmHandler.findStorageDomainForMemory(getVm().getStoragePoolId(), memoryDisksList); + StorageDomain storageDomain = MemoryStorageHandler.findStorageDomainForMemory(getVm().getStoragePoolId(), + memoryDisksList); if (storageDomain != null) { cachedStorageDomainId = storageDomain.getId(); } 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 7e869d8..b1aea13 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 @@ -5,6 +5,7 @@ import java.util.List; import java.util.Map; +import org.ovirt.engine.core.bll.memory.MemoryStorageHandler; import org.ovirt.engine.core.bll.memory.MemoryUtils; import org.ovirt.engine.core.bll.tasks.CommandCoordinatorUtil; import org.ovirt.engine.core.bll.validator.LocalizedVmStatus; @@ -81,7 +82,8 @@ List<DiskImage> diskDummiesForMemSize = MemoryUtils.createDiskDummies( getVm().getTotalMemorySizeInBytes(), MemoryUtils.META_DATA_SIZE_IN_BYTES); - StorageDomain storageDomain = VmHandler.findStorageDomainForMemory(getStoragePoolId(), diskDummiesForMemSize); + StorageDomain storageDomain = MemoryStorageHandler.findStorageDomainForMemory(getStoragePoolId(), + diskDummiesForMemSize); if (storageDomain != null) { cachedStorageDomainId = storageDomain.getId(); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java index 3d9c6e2..4a7553e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java @@ -15,6 +15,7 @@ import org.apache.commons.lang.NotImplementedException; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.context.CommandContext; +import org.ovirt.engine.core.bll.memory.MemoryStorageHandler; import org.ovirt.engine.core.bll.memory.MemoryUtils; import org.ovirt.engine.core.bll.network.VmInterfaceManager; import org.ovirt.engine.core.bll.profiles.CpuProfileHelper; @@ -552,7 +553,8 @@ private StorageDomain updateStorageDomainInMemoryVolumes(List<DiskImage> disksList) { List<DiskImage> memoryDisksList = MemoryUtils.createDiskDummies(getVm().getTotalMemorySizeInBytes(), MemoryUtils.META_DATA_SIZE_IN_BYTES); - StorageDomain storageDomain = VmHandler.findStorageDomainForMemory(getParameters().getStoragePoolId(), memoryDisksList); + StorageDomain storageDomain = MemoryStorageHandler.findStorageDomainForMemory(getParameters().getStoragePoolId(), + memoryDisksList); disksList.addAll(memoryDisksList); return storageDomain; } 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 0ecc8e4..9ff21b1 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 @@ -4,7 +4,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -20,7 +19,6 @@ import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; import org.ovirt.engine.core.bll.validator.VmValidationUtils; -import org.ovirt.engine.core.bll.validator.storage.StorageDomainValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.VdcObjectType; @@ -39,8 +37,6 @@ import org.ovirt.engine.core.common.businessentities.GuestAgentStatus; import org.ovirt.engine.core.common.businessentities.NumaTuneMode; import org.ovirt.engine.core.common.businessentities.Snapshot; -import org.ovirt.engine.core.common.businessentities.StorageDomain; -import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.UsbPolicy; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VDSGroup; @@ -57,8 +53,6 @@ import org.ovirt.engine.core.common.businessentities.storage.Disk; import org.ovirt.engine.core.common.businessentities.storage.DiskImage; import org.ovirt.engine.core.common.businessentities.storage.DiskInterface; -import org.ovirt.engine.core.common.businessentities.storage.StorageType; -import org.ovirt.engine.core.common.businessentities.storage.VolumeType; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; import org.ovirt.engine.core.common.errors.VdcBLLException; @@ -720,54 +714,6 @@ if (osRepository.isLinux(vmBase.getOsId()) && vmBase.getUsbPolicy().equals(UsbPolicy.ENABLED_LEGACY)) { vmBase.setUsbPolicy(UsbPolicy.DISABLED); } - } - - /** - * 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) { - - updateDisksStorage(currDomain, disksList); - if (currDomain.getStorageDomainType().isDataDomain() - && currDomain.getStatus() == StorageDomainStatus.Active - && validateSpaceRequirements(currDomain, disksList)) { - return currDomain; - } - } - return null; - } - - private static void updateDisksStorage(StorageDomain storageDomain, List<DiskImage> disksList) { - for (DiskImage disk : disksList) { - disk.setStorageIds(new ArrayList<Guid>(Collections.singletonList(storageDomain.getId()))); - } - //There should be two disks in the disksList, first of which is memory disk. Only its volume type should be modified. - updateDiskVolumeType(storageDomain.getStorageType(), disksList.get(0)); - } - - private static void updateDiskVolumeType(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) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandler.java new file mode 100644 index 0000000..fb5e475 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandler.java @@ -0,0 +1,70 @@ +package org.ovirt.engine.core.bll.memory; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.ovirt.engine.core.bll.validator.storage.StorageDomainValidator; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; +import org.ovirt.engine.core.common.businessentities.storage.DiskImage; +import org.ovirt.engine.core.common.businessentities.storage.StorageType; +import org.ovirt.engine.core.common.businessentities.storage.VolumeType; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; + +public class MemoryStorageHandler { + + /** + * 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) { + + updateDisksStorage(currDomain, disksList); + if (currDomain.getStorageDomainType().isDataDomain() + && currDomain.getStatus() == StorageDomainStatus.Active + && validateSpaceRequirements(currDomain, disksList)) { + return currDomain; + } + } + return null; + } + + private static void updateDisksStorage(StorageDomain storageDomain, List<DiskImage> disksList) { + for (DiskImage disk : disksList) { + disk.setStorageIds(new ArrayList<Guid>(Collections.singletonList(storageDomain.getId()))); + } + /* + There should be two disks in the disksList, first of which is memory disk. + Only its volume type should be modified. + */ + updateDiskVolumeType(storageDomain.getStorageType(), disksList.get(0)); + } + + private static void updateDiskVolumeType(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()); + } +} 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 4a243c2..e4d4757 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 @@ -1,12 +1,7 @@ package org.ovirt.engine.core.bll; -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; @@ -15,12 +10,7 @@ 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.StorageDomain; -import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; -import org.ovirt.engine.core.common.businessentities.StorageDomainType; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VmDevice; import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType; @@ -32,27 +22,12 @@ import org.ovirt.engine.core.common.businessentities.storage.DiskStorageType; import org.ovirt.engine.core.common.businessentities.storage.ImageStatus; import org.ovirt.engine.core.common.businessentities.storage.LunDisk; -import org.ovirt.engine.core.common.businessentities.storage.StorageType; -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() { @@ -101,44 +76,6 @@ assertTrue(vm.getManagedVmDeviceMap().containsKey(regularDisk.getId())); assertFalse(vm.getManagedVmDeviceMap().containsKey(lunDisk.getId())); 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) { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandlerTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandlerTest.java new file mode 100644 index 0000000..8c2e781 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandlerTest.java @@ -0,0 +1,77 @@ +package org.ovirt.engine.core.bll.memory; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.ovirt.engine.core.utils.MockConfigRule.mockConfig; + +import java.util.Arrays; +import java.util.List; + +import org.junit.Rule; +import org.junit.Test; +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.storage.DiskImage; +import org.ovirt.engine.core.common.businessentities.storage.StorageType; +import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.common.utils.SizeConverter; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.utils.MockConfigRule; + +public class MemoryStorageHandlerTest { + + 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)); + + @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 = MemoryStorageHandler.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 = MemoryStorageHandler.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; + } + +} -- To view, visit https://gerrit.ovirt.org/40433 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I367ee3b1e902495472e687b6dad63bc305b2c8bb Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Idan Shaby <ish...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
