Idan Shaby has uploaded a new change for review. Change subject: core: add MemoryStorageHandler the ability to sort ......................................................................
core: add MemoryStorageHandler the ability to sort This patch adds the MemoryStorageHandler mechanism the ability to sort the list of storage domains according to given comparators logic. For example, sort by the available disk size of a storage domain. This way, if we have more than one valid candidate, we can sort the list and get the most suitable domain out of it. When there is more than one comparator, a nested sort is performed. Change-Id: I00ad78fca97527a7dbf426a28ced7314a7bd921c 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/memory/MemoryStorageHandler.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainAvailableDiskSizeComparator.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainNumberOfVmDisksComparator.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageTypeComparator.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandlerTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterAbstractTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainSpaceRequirementsFilterTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainStatusFilterTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainTypeFilterTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainAvailableDiskSizeComparatorTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainComparatorAbstractTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainNumberOfVmDisksComparatorTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageTypeComparatorTest.java 17 files changed, 429 insertions(+), 33 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/38/40438/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 4aa8135..d4322a8 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 @@ -214,7 +214,7 @@ public Guid getStorageDomainIdForVmMemory(List<DiskImage> memoryDisksList) { if (cachedStorageDomainId.equals(Guid.Empty) && getVm() != null) { StorageDomain storageDomain = MemoryStorageHandler.getInstance().findStorageDomainForMemory( - getVm().getStoragePoolId(), memoryDisksList); + getVm().getStoragePoolId(), memoryDisksList, getDisksList()); 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 246d414..1cbf44c 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 @@ -83,7 +83,8 @@ getVm().getTotalMemorySizeInBytes(), MemoryUtils.META_DATA_SIZE_IN_BYTES); StorageDomain storageDomain = MemoryStorageHandler.getInstance().findStorageDomainForMemory( - getStoragePoolId(), diskDummiesForMemSize); + getStoragePoolId(), diskDummiesForMemSize, + ImagesHandler.filterImageDisks(getDiskDao().getAllForVm(getVmId()), false, false, false)); 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 6f33751..965f56f 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 @@ -7,6 +7,7 @@ import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; @@ -554,11 +555,21 @@ private StorageDomain updateStorageDomainInMemoryVolumes(List<DiskImage> disksList) { List<DiskImage> memoryDisksList = MemoryUtils.createDiskDummies(getVm().getTotalMemorySizeInBytes(), MemoryUtils.META_DATA_SIZE_IN_BYTES); StorageDomain storageDomain = MemoryStorageHandler.getInstance().findStorageDomainForMemory( - getParameters().getStoragePoolId(), memoryDisksList); + getParameters().getStoragePoolId(), memoryDisksList, getVmDisksDummies()); disksList.addAll(memoryDisksList); return storageDomain; } + private Collection<DiskImage> getVmDisksDummies() { + Collection<DiskImage> disksDummies = new LinkedList<>(); + for (Guid storageDomainId : getParameters().getImageToDestinationDomainMap().values()) { + DiskImage diskImage = new DiskImage(); + diskImage.setStorageIds(new ArrayList<Guid>(Arrays.asList(storageDomainId))); + disksDummies.add(diskImage); + } + return disksDummies; + } + /** * Validates that there is no duplicate VM. * @return <code>true</code> if the validation passes, <code>false</code> otherwise. 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 index 7904374..f8e7386 100644 --- 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 @@ -2,13 +2,19 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.List; +import org.apache.commons.collections.comparators.ComparatorChain; import org.ovirt.engine.core.bll.memory.storageDomainFilters.StorageDomainFilter; import org.ovirt.engine.core.bll.memory.storageDomainFilters.StorageDomainSpaceRequirementsFilter; import org.ovirt.engine.core.bll.memory.storageDomainFilters.StorageDomainStatusFilter; import org.ovirt.engine.core.bll.memory.storageDomainFilters.StorageDomainTypeFilter; +import org.ovirt.engine.core.bll.memory.storageDomainsComparators.StorageDomainAvailableDiskSizeComparator; +import org.ovirt.engine.core.bll.memory.storageDomainsComparators.StorageDomainNumberOfVmDisksComparator; +import org.ovirt.engine.core.bll.memory.storageDomainsComparators.StorageTypeComparator; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.storage.DiskImage; import org.ovirt.engine.core.common.businessentities.storage.StorageType; @@ -33,34 +39,39 @@ * * @param storagePoolId * The storage pool where the search for a domain will be made - * @param disksList + * @param memoryDisks * Disks for which space is needed + * @param vmDisks + * The vm's active snapshot disks. * @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 StorageDomain findStorageDomainForMemory(Guid storagePoolId, List<DiskImage> disksList) { + public StorageDomain findStorageDomainForMemory(Guid storagePoolId, List<DiskImage> memoryDisks, + Collection<DiskImage> vmDisks) { List<StorageDomain> domainsInPool = DbFacade.getInstance().getStorageDomainDao().getAllForStoragePool(storagePoolId); - StorageDomain storageDomainForMemory = findStorageDomainForMemory(domainsInPool, disksList); + StorageDomain storageDomainForMemory = findStorageDomainForMemory(domainsInPool, memoryDisks, vmDisks); if (storageDomainForMemory != null) { - updateDisksStorage(storageDomainForMemory, disksList); + updateDisksStorage(storageDomainForMemory, memoryDisks); } return storageDomainForMemory; } - public void updateDisksStorage(StorageDomain storageDomain, List<DiskImage> disksList) { - for (DiskImage disk : disksList) { + public void updateDisksStorage(StorageDomain storageDomain, List<DiskImage> memoryDisks) { + for (DiskImage disk : memoryDisks) { 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)); + updateDiskVolumeType(storageDomain.getStorageType(), memoryDisks.get(0)); } - protected StorageDomain findStorageDomainForMemory(List<StorageDomain> domainsInPool, List<DiskImage> disksList) { - domainsInPool = filterStorageDomains(domainsInPool, disksList); + protected StorageDomain findStorageDomainForMemory(List<StorageDomain> domainsInPool, List<DiskImage> memoryDisks, + Collection<DiskImage> vmDisks) { + domainsInPool = filterStorageDomains(domainsInPool, memoryDisks); + sortStorageDomains(domainsInPool, vmDisks); return domainsInPool.isEmpty() ? null : domainsInPool.get(0); } @@ -70,13 +81,29 @@ new StorageDomainSpaceRequirementsFilter()); } - protected List<StorageDomain> filterStorageDomains(List<StorageDomain> domainsInPool, List<DiskImage> disksList) { + protected List<? extends Comparator<StorageDomain>> getStorageDomainComparators( + List<StorageDomain> domainsInPool, Collection<DiskImage> vmDisks) { + return Arrays.asList(new StorageDomainNumberOfVmDisksComparator(domainsInPool, vmDisks), + new StorageTypeComparator(), + new StorageDomainAvailableDiskSizeComparator()); + } + + protected List<StorageDomain> filterStorageDomains(List<StorageDomain> domainsInPool, List<DiskImage> memoryDisks) { for (StorageDomainFilter storageDomainFilter : getStorageDomainFilters()) { - domainsInPool = storageDomainFilter.filterStorageDomains(domainsInPool, disksList); + domainsInPool = storageDomainFilter.filterStorageDomains(domainsInPool, memoryDisks); } return domainsInPool; } + protected void sortStorageDomains(List<StorageDomain> domainsInPool, Collection<DiskImage> vmDisks) { + ComparatorChain comparatorChain = new ComparatorChain(); + // When there is more than one comparator, a nested sort is performed. + for (Comparator<StorageDomain> comparator : getStorageDomainComparators(domainsInPool, vmDisks)) { + comparatorChain.addComparator(comparator, true); + } + Collections.sort(domainsInPool, comparatorChain); + } + private void updateDiskVolumeType(StorageType storageType, DiskImage disk) { VolumeType volumeType = storageType.isFileDomain() ? VolumeType.Sparse : VolumeType.Preallocated; disk.setVolumeType(volumeType); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainAvailableDiskSizeComparator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainAvailableDiskSizeComparator.java new file mode 100644 index 0000000..a1fa37e --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainAvailableDiskSizeComparator.java @@ -0,0 +1,13 @@ +package org.ovirt.engine.core.bll.memory.storageDomainsComparators; + +import java.util.Comparator; + +import org.ovirt.engine.core.common.businessentities.StorageDomain; + +public class StorageDomainAvailableDiskSizeComparator implements Comparator<StorageDomain> { + + @Override + public int compare(StorageDomain storageDomain, StorageDomain storageDomain2) { + return storageDomain.getAvailableDiskSize().compareTo(storageDomain2.getAvailableDiskSize()); + } +} diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainNumberOfVmDisksComparator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainNumberOfVmDisksComparator.java new file mode 100644 index 0000000..64c4ded --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainNumberOfVmDisksComparator.java @@ -0,0 +1,42 @@ +package org.ovirt.engine.core.bll.memory.storageDomainsComparators; + +import java.util.Collection; +import java.util.Comparator; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.storage.DiskImage; +import org.ovirt.engine.core.compat.Guid; + +public class StorageDomainNumberOfVmDisksComparator implements Comparator<StorageDomain> { + + private Map<Guid, Integer> numOfVmDisksInStorageDomains; + + public StorageDomainNumberOfVmDisksComparator(List<StorageDomain> domainsInPool, Collection<DiskImage> vmDisks) { + setUpNumOfVmDisksInStorageDomains(domainsInPool, vmDisks); + } + + @Override + public int compare(StorageDomain storageDomain, StorageDomain storageDomain2) { + Integer numOfVmDisksInStorageDomain = numOfVmDisksInStorageDomains.get(storageDomain.getId()); + Integer numOfVmDisksInStorageDomain2 = numOfVmDisksInStorageDomains.get(storageDomain2.getId()); + return numOfVmDisksInStorageDomain.compareTo(numOfVmDisksInStorageDomain2); + } + + private void setUpNumOfVmDisksInStorageDomains(List<StorageDomain> domainsInPool, Collection<DiskImage> vmDisks) { + numOfVmDisksInStorageDomains = new HashMap<>(); + + // Initialize each storage domain to have 0 disks. + for (StorageDomain storageDomain : domainsInPool) { + numOfVmDisksInStorageDomains.put(storageDomain.getId(), 0); + } + + for (DiskImage vmDisk : vmDisks) { + Guid vmDiskStorageDomainId = vmDisk.getStorageIds().get(0); + numOfVmDisksInStorageDomains.put(vmDiskStorageDomainId, + numOfVmDisksInStorageDomains.get(vmDiskStorageDomainId) + 1); + } + } +} diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageTypeComparator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageTypeComparator.java new file mode 100644 index 0000000..ddaf8c7 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageTypeComparator.java @@ -0,0 +1,14 @@ +package org.ovirt.engine.core.bll.memory.storageDomainsComparators; + +import java.util.Comparator; + +import org.ovirt.engine.core.common.businessentities.StorageDomain; + +public class StorageTypeComparator implements Comparator<StorageDomain> { + + @Override + public int compare(StorageDomain storageDomain, StorageDomain storageDomain2) { + return Boolean.compare(storageDomain.getStorageType().isFileDomain(), + storageDomain2.getStorageType().isFileDomain()); + } +} 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 index 7a8cda2..e58e95b 100644 --- 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 @@ -1,9 +1,11 @@ package org.ovirt.engine.core.bll.memory; import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.anyListOf; import static org.mockito.Mockito.doReturn; import java.util.Arrays; +import java.util.Comparator; import java.util.LinkedList; import java.util.List; @@ -21,41 +23,48 @@ @RunWith(MockitoJUnitRunner.class) public class MemoryStorageHandlerTest { - private StorageDomain validStorageDomain; + private StorageDomain validStorageDomain1; + private StorageDomain validStorageDomain2; + private StorageDomain validStorageDomain3; private StorageDomain invalidStorageDomain1; private StorageDomain invalidStorageDomain2; private StorageDomain invalidStorageDomain3; private StorageDomainFilter filter1; private StorageDomainFilter filter2; private StorageDomainFilter filter3; - private List<DiskImage> disksList; + private Comparator<StorageDomain> comparator1; + private Comparator<StorageDomain> comparator2; + private List<DiskImage> memoryDisks; + private List<DiskImage> vmDisks; @Spy private MemoryStorageHandler memoryStorageHandler = MemoryStorageHandler.getInstance(); @Before public void setUp() { - disksList = new LinkedList<>(); + memoryDisks = new LinkedList<>(); + vmDisks = new LinkedList<>(); initStorageDomains(); initFilters(); + initComparators(); } @Test public void filterAllDomainsExceptForTheFirstOne() { filterAllStorageDomainsExceptOne(Arrays.asList( - validStorageDomain, invalidStorageDomain1, invalidStorageDomain2), validStorageDomain); + validStorageDomain1, invalidStorageDomain1, invalidStorageDomain2), validStorageDomain1); } @Test public void filterAllDomainsExceptForTheSecondOne() { filterAllStorageDomainsExceptOne(Arrays.asList( - invalidStorageDomain1, validStorageDomain, invalidStorageDomain2), validStorageDomain); + invalidStorageDomain1, validStorageDomain1, invalidStorageDomain2), validStorageDomain1); } @Test public void filterAllDomainsExceptForTheThirdOne() { filterAllStorageDomainsExceptOne(Arrays.asList( - invalidStorageDomain1, invalidStorageDomain2, validStorageDomain), validStorageDomain); + invalidStorageDomain1, invalidStorageDomain2, validStorageDomain1), validStorageDomain1); } @Test @@ -63,8 +72,29 @@ verifyDomainForMemory(Arrays.asList(invalidStorageDomain1, invalidStorageDomain2, invalidStorageDomain3), null); } + @Test + public void sortStorageDomainsWithSingleComparator() { + sortStorageDomains(Arrays.asList(validStorageDomain1, validStorageDomain3), + Arrays.asList(validStorageDomain3, validStorageDomain1)); + } + + @Test + public void sortStorageDomainsWithWithMultipleComparators() { + sortStorageDomains(Arrays.asList(validStorageDomain1, validStorageDomain2, validStorageDomain3), + Arrays.asList(validStorageDomain3, validStorageDomain1, validStorageDomain2)); + } + + @Test + public void testFindStorageDomainForMemory() { + verifyDomainForMemory( + Arrays.asList(invalidStorageDomain1, validStorageDomain1, invalidStorageDomain2, + validStorageDomain2, invalidStorageDomain3, validStorageDomain3), validStorageDomain3); + } + private void initStorageDomains() { - validStorageDomain = initStorageDomain(); + validStorageDomain1 = initStorageDomain(); + validStorageDomain2 = initStorageDomain(); + validStorageDomain3 = initStorageDomain(); invalidStorageDomain1 = initStorageDomain(); invalidStorageDomain2 = initStorageDomain(); invalidStorageDomain3 = initStorageDomain(); @@ -80,6 +110,14 @@ doReturn(storageDomainFilters).when(memoryStorageHandler).getStorageDomainFilters(); } + private void initComparators() { + List<? extends Comparator<StorageDomain>> comparators = Arrays.asList( + new SmallestStorageDomainComparator(validStorageDomain2), + new BiggestStorageDomainComparator(validStorageDomain3)); + doReturn(comparators).when(memoryStorageHandler) + .getStorageDomainComparators(anyListOf(StorageDomain.class), anyListOf(DiskImage.class)); + } + private StorageDomain initStorageDomain() { StorageDomain storageDomain = new StorageDomain(); storageDomain.setId(Guid.newGuid()); @@ -89,14 +127,19 @@ private void filterAllStorageDomainsExceptOne(List<StorageDomain> storageDomains, StorageDomain expectedStorageDomain) { List<StorageDomain> filteredStorageDomains = - memoryStorageHandler.filterStorageDomains(storageDomains, disksList); + memoryStorageHandler.filterStorageDomains(storageDomains, memoryDisks); assertEquals(filteredStorageDomains, Arrays.asList(expectedStorageDomain)); } private void verifyDomainForMemory(List<StorageDomain> storageDomains, StorageDomain expectedStorageDomain) { StorageDomain storageDomain = - memoryStorageHandler.findStorageDomainForMemory(storageDomains, disksList); + memoryStorageHandler.findStorageDomainForMemory(storageDomains, memoryDisks, vmDisks); assertEquals(expectedStorageDomain, storageDomain); + } + + private void sortStorageDomains(List<StorageDomain> domainsInPool, List<StorageDomain> expectedSortedList) { + memoryStorageHandler.sortStorageDomains(domainsInPool, vmDisks); + assertEquals(domainsInPool, expectedSortedList); } private static class StorageDomainRejectingFilter extends StorageDomainFilter { @@ -117,4 +160,65 @@ }; } } + + private static abstract class StorageDomainAbstractComparator implements Comparator<StorageDomain> { + + private StorageDomain storageDomain; + + public StorageDomainAbstractComparator(StorageDomain storageDomain) { + this.storageDomain = storageDomain; + } + + @Override + public int compare(StorageDomain storageDomain, StorageDomain storageDomain2) { + if (storageDomain.equals(storageDomain2)) { + return 0; + } + else if (storageDomain.equals(this.storageDomain)){ + return equalsToFirstStorageDomain(); + } + else if (storageDomain2.equals(this.storageDomain)){ + return equalsToSecondStorageDomain(); + } + return 0; + } + + protected abstract int equalsToFirstStorageDomain(); + + protected abstract int equalsToSecondStorageDomain(); + } + + private static class BiggestStorageDomainComparator extends StorageDomainAbstractComparator { + + public BiggestStorageDomainComparator(StorageDomain storageDomain) { + super(storageDomain); + } + + @Override + protected int equalsToFirstStorageDomain() { + return 1; + } + + @Override + protected int equalsToSecondStorageDomain() { + return -1; + } + } + + private static class SmallestStorageDomainComparator extends StorageDomainAbstractComparator { + + public SmallestStorageDomainComparator(StorageDomain storageDomain) { + super(storageDomain); + } + + @Override + protected int equalsToFirstStorageDomain() { + return -1; + } + + @Override + protected int equalsToSecondStorageDomain() { + return 1; + } + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterAbstractTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterAbstractTest.java index d7c0972..3275b10 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterAbstractTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterAbstractTest.java @@ -11,12 +11,12 @@ public class StorageDomainFilterAbstractTest { protected StorageDomain storageDomain; - protected List<DiskImage> disksList; + protected List<DiskImage> memoryDisks; @Before public void setUp() { storageDomain = new StorageDomain(); storageDomain.setId(Guid.newGuid()); - disksList = new LinkedList<>(); + memoryDisks = new LinkedList<>(); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterTest.java index 0c04aa9..baa1902 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterTest.java @@ -46,6 +46,6 @@ when(predicate.eval(storageDomain)).thenReturn(!removeStorageDomainFromList); List<StorageDomain> storageDomains = Arrays.asList(storageDomain); - return filter.filterStorageDomains(storageDomains, disksList); + return filter.filterStorageDomains(storageDomains, memoryDisks); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainSpaceRequirementsFilterTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainSpaceRequirementsFilterTest.java index c954753..0743a6c 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainSpaceRequirementsFilterTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainSpaceRequirementsFilterTest.java @@ -39,21 +39,21 @@ @Test public void testStorageDomainForMemoryIsValid() { - assertTrue(filter.getPredicate(disksList).eval(storageDomain)); + assertTrue(filter.getPredicate(memoryDisks).eval(storageDomain)); } @Test public void testStorageDomainForMemoryIsNotValidWhenItHasLowSpace() { when(storageDomainValidator.isDomainWithinThresholds()) .thenReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN)); - assertFalse(filter.getPredicate(disksList).eval(storageDomain)); + assertFalse(filter.getPredicate(memoryDisks).eval(storageDomain)); } @Test public void testStorageDomainForMemoryIsNotValidWhenItHasNoSpaceForClonedDisks() { - when(storageDomainValidator.hasSpaceForClonedDisks(disksList)) + when(storageDomainValidator.hasSpaceForClonedDisks(memoryDisks)) .thenReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_STORAGE_DOMAIN)); - assertFalse(filter.getPredicate(disksList).eval(storageDomain)); + assertFalse(filter.getPredicate(memoryDisks).eval(storageDomain)); } private void initFilter() { @@ -63,7 +63,7 @@ private void initStorageDomainValidator() { when(storageDomainValidator.isDomainWithinThresholds()).thenReturn(ValidationResult.VALID); - when(storageDomainValidator.hasSpaceForClonedDisks(disksList)) + when(storageDomainValidator.hasSpaceForClonedDisks(memoryDisks)) .thenReturn(ValidationResult.VALID); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainStatusFilterTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainStatusFilterTest.java index 6039834..e84866e 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainStatusFilterTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainStatusFilterTest.java @@ -27,7 +27,7 @@ @Theory public void testStorageDomainForMemoryIsValidOnlyForActiveStatus(StorageDomainStatus storageDomainStatus) { storageDomain.setStatus(storageDomainStatus); - assertEquals(filter.getPredicate(disksList).eval(storageDomain), + assertEquals(filter.getPredicate(memoryDisks).eval(storageDomain), storageDomainStatus == StorageDomainStatus.Active); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainTypeFilterTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainTypeFilterTest.java index f000dc2..d3aa020 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainTypeFilterTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainTypeFilterTest.java @@ -27,6 +27,6 @@ @Theory public void testStorageDomainForMemoryIsValidOnlyForDataTypes(StorageDomainType storageDomainType) { storageDomain.setStorageDomainType(storageDomainType); - assertEquals(filter.getPredicate(disksList).eval(storageDomain), storageDomainType.isDataDomain()); + assertEquals(filter.getPredicate(memoryDisks).eval(storageDomain), storageDomainType.isDataDomain()); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainAvailableDiskSizeComparatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainAvailableDiskSizeComparatorTest.java new file mode 100644 index 0000000..76ee1e1 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainAvailableDiskSizeComparatorTest.java @@ -0,0 +1,43 @@ +package org.ovirt.engine.core.bll.memory.storageDomainsComparators; + +import org.junit.ClassRule; +import org.junit.Test; +import org.ovirt.engine.core.utils.RandomUtils; +import org.ovirt.engine.core.utils.RandomUtilsSeedingRule; + +public class StorageDomainAvailableDiskSizeComparatorTest extends StorageDomainComparatorAbstractTest { + + @ClassRule + public static RandomUtilsSeedingRule rusr = new RandomUtilsSeedingRule(); + + public StorageDomainAvailableDiskSizeComparatorTest() { + comparator = new StorageDomainAvailableDiskSizeComparator(); + } + + @Test + public void compareWhenSizesAreEqual() { + int availableDiskSize = getRandomNumberForTest(); + setStorageDomainsAvailableDiskSize(availableDiskSize, availableDiskSize); + + assertEqualsTo(storageDomain1, storageDomain2); + } + + @Test + public void compareWhenSizesAreNotEqual() { + int availableDiskSize = getRandomNumberForTest(); + setStorageDomainsAvailableDiskSize(availableDiskSize, availableDiskSize / 2); + + assertBiggerThan(storageDomain1, storageDomain2); + assertSmallerThan(storageDomain2, storageDomain1); + } + + private int getRandomNumberForTest() { + return RandomUtils.instance().nextInt(2, Integer.MAX_VALUE); + } + + private void setStorageDomainsAvailableDiskSize(int storageDomain1AvailableDiskSize, + int storageDomain2AvailableDiskSize) { + storageDomain1.setAvailableDiskSize(storageDomain1AvailableDiskSize); + storageDomain2.setAvailableDiskSize(storageDomain2AvailableDiskSize); + } +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainComparatorAbstractTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainComparatorAbstractTest.java new file mode 100644 index 0000000..22183f4 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainComparatorAbstractTest.java @@ -0,0 +1,44 @@ +package org.ovirt.engine.core.bll.memory.storageDomainsComparators; + +import static org.junit.Assert.assertTrue; + +import java.util.Comparator; + +import org.junit.Before; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.compat.Guid; + +public class StorageDomainComparatorAbstractTest { + + protected Comparator<StorageDomain> comparator; + protected StorageDomain storageDomain1; + protected StorageDomain storageDomain2; + + @Before + public void setUp() { + storageDomain1 = initStorageDomain(); + storageDomain2 = initStorageDomain(); + } + + protected void assertSmallerThan(StorageDomain firstStorageDomain, StorageDomain secondStorageDomain) { + assertTrue(compareStorageDomains(firstStorageDomain, secondStorageDomain) < 0); + } + + protected void assertEqualsTo(StorageDomain firstStorageDomain, StorageDomain secondStorageDomain) { + assertTrue(compareStorageDomains(firstStorageDomain, secondStorageDomain) == 0); + } + + protected void assertBiggerThan(StorageDomain firstStorageDomain, StorageDomain secondStorageDomain) { + assertTrue(compareStorageDomains(firstStorageDomain, secondStorageDomain) > 0); + } + + private int compareStorageDomains(StorageDomain firstStorageDomain, StorageDomain secondStorageDomain) { + return comparator.compare(firstStorageDomain, secondStorageDomain); + } + + private StorageDomain initStorageDomain() { + StorageDomain storageDomain = new StorageDomain(); + storageDomain.setId(Guid.newGuid()); + return storageDomain; + } +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainNumberOfVmDisksComparatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainNumberOfVmDisksComparatorTest.java new file mode 100644 index 0000000..6c65f5f --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageDomainNumberOfVmDisksComparatorTest.java @@ -0,0 +1,64 @@ +package org.ovirt.engine.core.bll.memory.storageDomainsComparators; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; +import org.ovirt.engine.core.common.businessentities.StorageDomain; +import org.ovirt.engine.core.common.businessentities.storage.DiskImage; +import org.ovirt.engine.core.compat.Guid; + +public class StorageDomainNumberOfVmDisksComparatorTest extends StorageDomainComparatorAbstractTest { + + private List<StorageDomain> storageDomains; + private DiskImage vmDisk1; + private DiskImage vmDisk2; + private DiskImage vmDisk3; + + @Before + @Override + public void setUp() { + super.setUp(); + storageDomains = Arrays.asList(storageDomain1, storageDomain2); + vmDisk1 = new DiskImage(); + vmDisk2 = new DiskImage(); + vmDisk3 = new DiskImage(); + } + + @Test + public void compareWhenStorageDomainsHaveNoDisks() { + initComparator(); + assertEqualsTo(storageDomain1, storageDomain2); + } + + @Test + public void compareWhenSizesAreEqual() { + attachVmDisksToStorageDomain(storageDomain1, vmDisk1); + attachVmDisksToStorageDomain(storageDomain2, vmDisk2); + initComparator(vmDisk1, vmDisk2); + + assertEqualsTo(storageDomain1, storageDomain2); + } + + @Test + public void compareWhenSizesAreNotEqual() { + attachVmDisksToStorageDomain(storageDomain1, vmDisk1, vmDisk2); + attachVmDisksToStorageDomain(storageDomain2, vmDisk3); + initComparator(vmDisk1, vmDisk2, vmDisk3); + + assertBiggerThan(storageDomain1, storageDomain2); + assertSmallerThan(storageDomain2, storageDomain1); + } + + private void attachVmDisksToStorageDomain(StorageDomain storageDomain, DiskImage... vmDisks) { + for (DiskImage diskImage : vmDisks) { + diskImage.setStorageIds(new ArrayList<Guid>(Arrays.asList(storageDomain.getId()))); + } + } + + private void initComparator(DiskImage... vmDisks) { + comparator = new StorageDomainNumberOfVmDisksComparator(storageDomains, Arrays.asList(vmDisks)); + } +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageTypeComparatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageTypeComparatorTest.java new file mode 100644 index 0000000..7ab4e9e --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageTypeComparatorTest.java @@ -0,0 +1,33 @@ +package org.ovirt.engine.core.bll.memory.storageDomainsComparators; + +import static org.junit.Assert.assertEquals; + +import org.junit.experimental.theories.DataPoints; +import org.junit.experimental.theories.Theories; +import org.junit.experimental.theories.Theory; +import org.junit.runner.RunWith; +import org.ovirt.engine.core.common.businessentities.storage.StorageType; + +@RunWith(Theories.class) +public class StorageTypeComparatorTest extends StorageDomainComparatorAbstractTest { + + @DataPoints + public static StorageType[] storageTypes = StorageType.values(); + + public StorageTypeComparatorTest() { + comparator = new StorageTypeComparator(); + } + + @Theory + public void testCompare(StorageType storageType) { + storageDomain1.setStorageType(storageType); + for (StorageType storageType2 : StorageType.values()) { + storageDomain2.setStorageType(storageType2); + int compareTypes = Boolean.compare(storageType.isFileDomain(), storageType2.isFileDomain()); + int comparatorReturnValue = comparator.compare(storageDomain1, storageDomain2); + assertEquals(compareTypes < 0, comparatorReturnValue < 0); + assertEquals(compareTypes == 0, comparatorReturnValue == 0); + assertEquals(compareTypes > 0, comparatorReturnValue > 0); + } + } +} -- To view, visit https://gerrit.ovirt.org/40438 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I00ad78fca97527a7dbf426a28ced7314a7bd921c 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
