Arik Hadas has uploaded a new change for review. Change subject: core: fix the hierarchy of memory images removers ......................................................................
core: fix the hierarchy of memory images removers The "RAM Snapshots" feature introduced the MemoryImageRemover and MemoryImageRemoverFromExportDomain classes. The second inherits the first, taking advantage of common code and making small adjustments that are required when it comes to operating on the export domain. We end up with a wrong relationship between the two classes as there is no relation of "kind of" between them, thus the inheritence is wrong. This patch fix the hierarchy between them: 1. MemoryImageRemover become an abstract class, containing the common code for subclasses that remove memory states 2. introduce MemoryImageRemoverOnDataDomain class which extends MemoryImageRemover and implements the code that is relevant for removing memory states from data domains 3. The MemoryImageRemoverFromExportDomain now extends MemoryImageRemover and implements the code which is relevant for removing memory states from export domains ImportVmCommand is changed to use MemoryImageRemoverOnDataDomain instead of using MemoryImageRemover. Change-Id: Id7cada7b6f9a3356afcac0dd2856d6a02cf1f9fd 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/memory/MemoryImageRemover.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java 4 files changed, 82 insertions(+), 43 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/26/16826/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 6f13511..483a576 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 @@ -13,7 +13,7 @@ import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.job.ExecutionContext; import org.ovirt.engine.core.bll.job.ExecutionHandler; -import org.ovirt.engine.core.bll.memory.MemoryImageRemover; +import org.ovirt.engine.core.bll.memory.MemoryImageRemoverOnDataDomain; import org.ovirt.engine.core.bll.memory.MemoryUtils; import org.ovirt.engine.core.bll.network.MacPoolManager; import org.ovirt.engine.core.bll.network.VmInterfaceManager; @@ -1077,7 +1077,7 @@ private void removeVmSnapshots(VM vm) { Set<String> memoriesOfRemovedSnapshots = snapshotsManager.removeSnapshots(vm.getId()); - new MemoryImageRemover(vm, this).removeMemoryVolumes(memoriesOfRemovedSnapshots); + new MemoryImageRemoverOnDataDomain(vm, this).removeMemoryVolumes(memoriesOfRemovedSnapshots); } protected void removeVmNetworkInterfaces() { 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 545dfe2..d9d3f2f 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 @@ -8,7 +8,6 @@ import org.ovirt.engine.core.bll.VmCommand; import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; import org.ovirt.engine.core.common.VdcObjectType; -import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; @@ -17,7 +16,7 @@ import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.utils.GuidUtils; -public class MemoryImageRemover { +public abstract class MemoryImageRemover { private TaskHandlerCommand<?> enclosingCommand; protected VM vm; @@ -27,6 +26,12 @@ this.enclosingCommand = enclosingCommand; this.vm = vm; } + + protected abstract boolean shouldRemoveMemorySnapshotVolumes(String memoryVolume); + + protected abstract DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids); + + protected abstract DeleteImageGroupVDSCommandParameters buildDeleteMemoryConfParams(List<Guid> guids); public void removeMemoryVolume(String memoryVolumes) { if (shouldRemoveMemorySnapshotVolumes(memoryVolumes)) { @@ -38,19 +43,6 @@ for (String memoryVols : memoryVolumes) { removeMemoryVolume(memoryVols); } - } - - protected DbFacade getDbFacade() { - return DbFacade.getInstance(); - } - - /** - * There is a one to many relation between memory volumes and snapshots, so memory - * volumes should be removed only if the only snapshot that points to them is removed - */ - protected boolean shouldRemoveMemorySnapshotVolumes(String memoryVolume) { - return !memoryVolume.isEmpty() && - getDbFacade().getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1; } protected boolean removeMemoryVolumes(String memVols, boolean startPollingTasks) { @@ -107,30 +99,5 @@ } return true; - } - - protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids) { - return new DeleteImageGroupVDSCommandParameters( - guids.get(1), guids.get(0), guids.get(2), isPostZero(), false); - } - - protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryConfParams(List<Guid> guids) { - return new DeleteImageGroupVDSCommandParameters( - guids.get(1), guids.get(0), guids.get(4), isPostZero(), false); - } - - protected boolean isPostZero() { - if (cachedPostZero == null) { - // check if one of the disks is marked with wipe_after_delete - cachedPostZero = - getDbFacade().getDiskDao().getAllForVm(vm.getId()).contains( - new Object() { - @Override - public boolean equals(Object obj) { - return ((Disk) obj).isWipeAfterDelete(); - } - }); - } - return cachedPostZero; } } 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 413531d..66d5605 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 @@ -1,8 +1,11 @@ package org.ovirt.engine.core.bll.memory; +import java.util.List; + import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters; import org.ovirt.engine.core.compat.Guid; public class MemoryImageRemoverFromExportDomain extends MemoryImageRemover { @@ -17,6 +20,18 @@ this.storageDomainId = storageDomainId; } + @Override + protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids) { + return new DeleteImageGroupVDSCommandParameters( + guids.get(1), guids.get(0), guids.get(2), isPostZero(), false); + } + + @Override + protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryConfParams(List<Guid> guids) { + return new DeleteImageGroupVDSCommandParameters( + guids.get(1), guids.get(0), guids.get(4), isPostZero(), false); + } + /** * We set the post zero field on memory image deletion from export domain as we do * when it is deleted from data domain even though the export domain is NFS and NFS @@ -24,7 +39,6 @@ * code that do the same, and to be prepared for supporting export domains which * are not NFS. */ - @Override protected boolean isPostZero() { if (cachedPostZero == null) { // check if one of the disks is marked with wipe_after_delete 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 new file mode 100644 index 0000000..9f3229c --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java @@ -0,0 +1,58 @@ +package org.ovirt.engine.core.bll.memory; + +import java.util.List; + +import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; +import org.ovirt.engine.core.common.businessentities.Disk; +import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; + +public class MemoryImageRemoverOnDataDomain extends MemoryImageRemover { + + public MemoryImageRemoverOnDataDomain(VM vm, TaskHandlerCommand<?> enclosingCommand) { + super(vm, enclosingCommand); + } + + @Override + protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids) { + return new DeleteImageGroupVDSCommandParameters( + guids.get(1), guids.get(0), guids.get(2), isPostZero(), false); + } + + @Override + protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryConfParams(List<Guid> guids) { + return new DeleteImageGroupVDSCommandParameters( + guids.get(1), guids.get(0), guids.get(4), isPostZero(), false); + } + + protected boolean isPostZero() { + if (cachedPostZero == null) { + // check if one of the disks is marked with wipe_after_delete + cachedPostZero = + getDbFacade().getDiskDao().getAllForVm(vm.getId()).contains( + new Object() { + @Override + public boolean equals(Object obj) { + return ((Disk) obj).isWipeAfterDelete(); + } + }); + } + return cachedPostZero; + } + + /** + * There is a one to many relation between memory volumes and snapshots, so memory + * volumes should be removed only if the only snapshot that points to them is removed + */ + @Override + protected boolean shouldRemoveMemorySnapshotVolumes(String memoryVolume) { + return !memoryVolume.isEmpty() && + getDbFacade().getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1; + } + + protected DbFacade getDbFacade() { + return DbFacade.getInstance(); + } +} -- To view, visit http://gerrit.ovirt.org/16826 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id7cada7b6f9a3356afcac0dd2856d6a02cf1f9fd 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
