Arik Hadas has uploaded a new change for review. Change subject: core: cleanup related to memory state removal ......................................................................
core: cleanup related to memory state removal 1. changed MemoryImageRemover#isMemoryStateRemovable method not to be abstract, but to have a default check that checks that the memory state representation is not empty 2. Reduce the visibility of MemoryImageRemover#removeMemoryVolumes to 'protected' as it shouldn't be invoked by external classes. subclasses expose API that use it behind the scenes 3. Snapshot manager now uses MemoryUtils#getMemoryVolumesFromSnapshots instead of having duplicated logic Change-Id: I30f400400eb8233a61c4a9cef54c7a8844255b38 Signed-off-by: Arik Hadas <[email protected]> --- 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/RemoveVmFromImportExportCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java 6 files changed, 33 insertions(+), 28 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/07/17307/1 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 5975022..c0f9251 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 @@ -1114,10 +1114,9 @@ } private void removeVmSnapshots(VM vm) { - Set<String> memoriesOfRemovedSnapshots = - snapshotsManager.removeSnapshots(vm.getId()); - if (!memoriesOfRemovedSnapshots.isEmpty()) { - new MemoryImageRemoverOnDataDomain(vm, this).removeMemoryVolumes(memoriesOfRemovedSnapshots); + Set<String> memoryStates = snapshotsManager.removeSnapshots(vm.getId()); + if (!memoryStates.isEmpty()) { + new MemoryImageRemoverOnDataDomain(vm, this).remove(memoryStates); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java index 6041da3..03b6d30 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java @@ -9,7 +9,6 @@ import org.ovirt.engine.core.bll.job.ExecutionContext; import org.ovirt.engine.core.bll.memory.MemoryImageRemoverFromExportDomain; -import org.ovirt.engine.core.bll.memory.MemoryUtils; import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; @@ -132,7 +131,7 @@ private void removeMemoryImages() { new MemoryImageRemoverFromExportDomain(getVm(), this, getParameters().getStoragePoolId(), getParameters().getStorageDomainId()) - .removeMemoryVolumes(MemoryUtils.getMemoryVolumesFromSnapshots(getVm().getSnapshots())); + .remove(); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java index 99b9ee2..b203311 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java @@ -34,19 +34,24 @@ this.startPollingTasks = startPollingTasks; } - protected abstract boolean isMemoryStateRemovable(String memoryVolume); - protected abstract DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids); protected abstract DeleteImageGroupVDSCommandParameters buildDeleteMemoryConfParams(List<Guid> guids); - public void removeMemoryVolume(String memoryVolumes) { + /** + * Default implementation checks whether the memory state representation is not empty + */ + protected boolean isMemoryStateRemovable(String memoryVolume) { + return !memoryVolume.isEmpty(); + } + + protected void removeMemoryVolume(String memoryVolumes) { if (isMemoryStateRemovable(memoryVolumes)) { removeMemoryVolumes(memoryVolumes); } } - public void removeMemoryVolumes(Set<String> memoryVolumes) { + protected void removeMemoryVolumes(Set<String> memoryVolumes) { for (String memoryVols : memoryVolumes) { removeMemoryVolume(memoryVols); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java index 225ca63..32b5145 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java @@ -23,6 +23,17 @@ this.storageDomainId = storageDomainId; } + public void remove() { + removeMemoryVolumes(MemoryUtils.getMemoryVolumesFromSnapshots(vm.getSnapshots())); + } + + @Override + protected void removeMemoryVolume(String memoryVolumes) { + super.removeMemoryVolume( + MemoryUtils.changeStorageDomainAndPoolInMemoryState( + memoryVolumes, storageDomainId, storagePoolId)); + } + @Override protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids) { return new DeleteImageGroupVDSCommandParameters( @@ -54,17 +65,5 @@ }); } return cachedPostZero; - } - - @Override - protected boolean isMemoryStateRemovable(String memoryVolume) { - return !memoryVolume.isEmpty(); - } - - @Override - public void removeMemoryVolume(String memoryVolumes) { - super.removeMemoryVolume( - MemoryUtils.changeStorageDomainAndPoolInMemoryState( - memoryVolumes, storageDomainId, storagePoolId)); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java index 67b7e38..465e0ec 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java @@ -1,6 +1,7 @@ package org.ovirt.engine.core.bll.memory; import java.util.List; +import java.util.Set; import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; import org.ovirt.engine.core.common.businessentities.Disk; @@ -19,6 +20,10 @@ this.vm = vm; } + public void remove(Set<String> memoryStates) { + removeMemoryVolumes(memoryStates); + } + @Override protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids) { return new DeleteImageGroupVDSCommandParameters( diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java index 7142672..01f34b8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java @@ -2,13 +2,13 @@ import java.util.ArrayList; import java.util.Date; -import java.util.HashSet; import java.util.List; import java.util.Set; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.ImagesHandler; import org.ovirt.engine.core.bll.context.CompensationContext; +import org.ovirt.engine.core.bll.memory.MemoryUtils; import org.ovirt.engine.core.bll.network.VmInterfaceManager; import org.ovirt.engine.core.bll.utils.ClusterUtils; import org.ovirt.engine.core.bll.utils.VmDeviceUtils; @@ -262,13 +262,11 @@ * @return Set of memoryVolumes of the removed snapshots */ public Set<String> removeSnapshots(Guid vmId) { - Set<String> memoryVolumes = new HashSet<String>(); - for (Snapshot snapshot : getSnapshotDao().getAll(vmId)) { - memoryVolumes.add(snapshot.getMemoryVolume()); + final List<Snapshot> vmSnapshots = getSnapshotDao().getAll(vmId); + for (Snapshot snapshot : vmSnapshots) { getSnapshotDao().remove(snapshot.getId()); } - memoryVolumes.remove(StringUtils.EMPTY); - return memoryVolumes; + return MemoryUtils.getMemoryVolumesFromSnapshots(vmSnapshots); } /** -- To view, visit http://gerrit.ovirt.org/17307 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I30f400400eb8233a61c4a9cef54c7a8844255b38 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
