Vered Volansky has uploaded a new change for review. Change subject: core: Use correct validation for SD selection ......................................................................
core: Use correct validation for SD selection When creating a snapshot, use new storage allocation validations for the vm volumes when selecting a domain. Some code has been removed as a result from this change. Change-Id: Iac901a638358b9b08bcc6992d783a42cbfcd9bfa 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/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/LiveSnapshotMemoryImageBuilder.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageBuilder.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryUtils.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/NullableMemoryImageBuilder.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommandTest.java 9 files changed, 30 insertions(+), 82 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/71/32171/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 2d1e3c7..2bc7ccb 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.context.CommandContext; import org.ovirt.engine.core.bll.memory.LiveSnapshotMemoryImageBuilder; import org.ovirt.engine.core.bll.memory.MemoryImageBuilder; +import org.ovirt.engine.core.bll.memory.MemoryUtils; import org.ovirt.engine.core.bll.memory.NullableMemoryImageBuilder; import org.ovirt.engine.core.bll.memory.StatelessSnapshotMemoryImageBuilder; import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter; @@ -145,8 +146,7 @@ return toReturn; } - private boolean validateStorageDomains(List<DiskImage> vmDisksList) { - List<DiskImage> memoryDisksList = getMemoryImageBuilder().getDisksToBeCreated(); + private boolean validateStorageDomains(List<DiskImage> vmDisksList, List<DiskImage> memoryDisksList) { List<DiskImage> disksList = getAllDisks(vmDisksList, memoryDisksList); MultipleStorageDomainsValidator sdValidator = createMultipleStorageDomainsValidator(disksList); return validate(sdValidator.allDomainsExistAndActive()) @@ -204,10 +204,9 @@ setSucceeded(true); } - public Guid getStorageDomainIdForVmMemory() { + public Guid getStorageDomainIdForVmMemory(List<DiskImage> memoryDisksList) { if (cachedStorageDomainId.equals(Guid.Empty) && getVm() != null) { - long sizeNeeded = getVm().getTotalMemorySizeInBytes() / BYTES_IN_GB; - StorageDomain storageDomain = VmHandler.findStorageDomainForMemory(getVm().getStoragePoolId(), sizeNeeded); + StorageDomain storageDomain = VmHandler.findStorageDomainForMemory(getVm().getStoragePoolId(), memoryDisksList); if (storageDomain != null) { cachedStorageDomainId = storageDomain.getId(); } @@ -225,7 +224,7 @@ } if (getParameters().isSaveMemory() && isLiveSnapshotApplicable()) { - return new LiveSnapshotMemoryImageBuilder(getVm(), getStorageDomainIdForVmMemory(), getStoragePool(), this); + return new LiveSnapshotMemoryImageBuilder(getVm(), cachedStorageDomainId, getStoragePool(), this); } return new NullableMemoryImageBuilder(); @@ -512,12 +511,14 @@ } } - if (!validateStorageDomains(disksList)) { - 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 (getParameters().isSaveMemory() && Guid.Empty.equals(getStorageDomainIdForVmMemory())) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NO_SUITABLE_DOMAIN_FOUND); + if (!validateStorageDomains(disksList, memoryDisksList)) { + return false; } return true; 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 a5702e8..8305dea 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 @@ -47,10 +47,6 @@ private static final String SAVE_RAM_STATE_TASK_KEY = "SAVE_RAM_STATE_TASK_KEY"; private boolean isHibernateVdsProblematic = false; - /** The size for the snapshot's meta data which is vm related properties at the - * time the snapshot was taken */ - public static final long META_DATA_SIZE_IN_BYTES = 10 * 1024; - /** * Constructor for command creation when compensation is applied on startup * @@ -90,7 +86,8 @@ @Override public Guid getStorageDomainId() { if (_storageDomainId.equals(Guid.Empty) && getVm() != null) { - List<DiskImage> diskDummiesForMemSize = MemoryUtils.createDiskDummies(getVm().getTotalMemorySizeInBytes(), META_DATA_SIZE_IN_BYTES); + List<DiskImage> diskDummiesForMemSize = MemoryUtils.createDiskDummies(getVm().getTotalMemorySizeInBytes(), + MemoryUtils.META_DATA_SIZE_IN_BYTES); StorageDomain storageDomain = VmHandler.findStorageDomainForMemory(getVm().getStoragePoolId(), diskDummiesForMemSize); if (storageDomain != null) { _storageDomainId = storageDomain.getId(); @@ -175,7 +172,7 @@ new CreateImageVDSCommandParameters(getVm().getStoragePoolId(), getStorageDomainId(), image2GroupId, - META_DATA_SIZE_IN_BYTES, + MemoryUtils.META_DATA_SIZE_IN_BYTES, VolumeType.Sparse, VolumeFormat.COW, hiberVol2, 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 b89edc5..fb10b66 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 @@ -671,25 +671,6 @@ * The storage pool where the search for a domain will be made * @param sizeRequested * The free size we need to have in the domain, in gigabytes - * @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 - * - * This method is deprecated. Instead use findStorageDomainForMemory(Guid storagePoolId, List<DiskImage> disksList), - * and prepare the relevant dikslisk (probably dummies to reflect size). - */ - @Deprecated - public static StorageDomain findStorageDomainForMemory(Guid storagePoolId, long sizeRequested) { - return findStorageDomainForMemory(storagePoolId, sizeRequested, Collections.<StorageDomain, Integer>emptyMap()); - } - - /** - * 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 sizeRequested - * The free size we need to have in the domain, in gigabytes * @param domain2reservedSpaceInDomain * Maps storage domain to size we already reserved on it * @return storage domain in the given pool with at least the required amount of free space, @@ -742,8 +723,8 @@ 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)); + + updateDisksStorage(currDomain, disksList); if (currDomain.getStorageDomainType().isDataDomain() && currDomain.getStatus() == StorageDomainStatus.Active && validateSpaceRequirements(currDomain, disksList)) { @@ -753,7 +734,15 @@ return null; } - private static void updateDisksStorageType(StorageType storageType, DiskImage disk) { + 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); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java index 3b131a8..e848389 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/LiveSnapshotMemoryImageBuilder.java @@ -1,15 +1,9 @@ package org.ovirt.engine.core.bll.memory; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - import org.ovirt.engine.core.bll.Backend; import org.ovirt.engine.core.bll.HibernateVmCommand; import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; import org.ovirt.engine.core.common.VdcObjectType; -import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.StorageDomainStatic; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.VM; @@ -140,20 +134,5 @@ public boolean isCreateTasks() { return true; - } - - public List<DiskImage> getDisksToBeCreated() { - DiskImage imageForMemory = new DiskImage(); - imageForMemory.setStorageIds(new ArrayList<Guid>(Collections.singletonList(storageDomainId))); - imageForMemory.setStoragePoolId(storagePool.getId()); - imageForMemory.setSize(vm.getTotalMemorySizeInBytes()); - imageForMemory.setVolumeType(getVolumeTypeForDomain()); - imageForMemory.setvolumeFormat(VolumeFormat.RAW); - - DiskImage imageForMetadata = DiskImage.copyOf(imageForMemory); - imageForMetadata.setSize(HibernateVmCommand.META_DATA_SIZE_IN_BYTES); - imageForMetadata.setVolumeType(VolumeType.Sparse); - imageForMetadata.setvolumeFormat(VolumeFormat.COW); - return Arrays.asList(imageForMemory, imageForMetadata); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageBuilder.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageBuilder.java index 7742c3e..d5b7da5 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageBuilder.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageBuilder.java @@ -1,9 +1,5 @@ package org.ovirt.engine.core.bll.memory; -import java.util.List; - -import org.ovirt.engine.core.common.businessentities.DiskImage; - public interface MemoryImageBuilder { /** * Create the images @@ -21,6 +17,4 @@ * @return true if tasks are created in {@link #build()}, false otherwise */ boolean isCreateTasks(); - - List<DiskImage> getDisksToBeCreated(); } 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 49480a4..879cbbc 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 @@ -15,6 +15,10 @@ public class MemoryUtils { + /** The size for the snapshot's meta data which is vm related properties at the + * time the snapshot was taken */ + public static final long META_DATA_SIZE_IN_BYTES = 10 * 1024; + /** * Modified the given memory volume String representation to have the given storage * pool and storage domain diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/NullableMemoryImageBuilder.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/NullableMemoryImageBuilder.java index f213344..929c729 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/NullableMemoryImageBuilder.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/NullableMemoryImageBuilder.java @@ -1,10 +1,6 @@ package org.ovirt.engine.core.bll.memory; -import java.util.Collections; -import java.util.List; - import org.apache.commons.lang.StringUtils; -import org.ovirt.engine.core.common.businessentities.DiskImage; /** * This builder is used when no memory image should be created @@ -21,9 +17,5 @@ public boolean isCreateTasks() { return false; - } - - public List<DiskImage> getDisksToBeCreated() { - return Collections.emptyList(); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java index 604c7bd..76ee9a4 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/StatelessSnapshotMemoryImageBuilder.java @@ -1,9 +1,5 @@ package org.ovirt.engine.core.bll.memory; -import java.util.Collections; -import java.util.List; - -import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.dal.dbbroker.DbFacade; @@ -33,9 +29,4 @@ public boolean isCreateTasks() { return false; } - - public List<DiskImage> getDisksToBeCreated() { - return Collections.emptyList(); - } - } 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 db64171..49f84a5 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 @@ -78,6 +78,7 @@ doReturn(diskImagesValidator).when(cmd).createDiskImageValidator(any(List.class)); doReturn(multipleStorageDomainsValidator).when(cmd).createMultipleStorageDomainsValidator(any(List.class)); doReturn(memoryImageBuilder).when(cmd).getMemoryImageBuilder(); + doReturn(Guid.newGuid()).when(cmd).getStorageDomainIdForVmMemory(anyList()); } @Test -- To view, visit http://gerrit.ovirt.org/32171 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iac901a638358b9b08bcc6992d783a42cbfcd9bfa Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Vered Volansky <vvola...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
