Arik Hadas has uploaded a new change for review. Change subject: core: managed removal of memory volumes in negative flows ......................................................................
core: managed removal of memory volumes in negative flows On failure while importing VM or creating snapshot with memory, we need to remove the memory volumes that were copied/created. The remove image operation is asynchronous, i.e it is based on tasks. On negative flows such as the ones mentioned above, we need to remove the memory image in the end-action phase. Since our infrastructure for commands doesn't support scenarios where tasks are created in the end-action phase well, we used to create the tasks for the remove operation without polling them. The problem was that without polling the tasks they remained in VDSM forever. The solution is to invoke the command that removes memory volumes as a stand-alone command, and not as a child of the failed command. It means that the tasks will be created as tasks of RemoveMemoryVolumesCommand and thus its end-action methods will be called and not the ones of the failed command. Change-Id: Ib4b270ec0e1ab41cae34459dde9f9cf47b1b5bdf Bug-Url: https://bugzilla.redhat.com/1019394 Signed-off-by: Arik Hadas <aha...@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/ImportVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.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/MemoryImageRemoverOnDataDomain.java 6 files changed, 69 insertions(+), 39 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/22/23222/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 fa9f0e0..4de449b 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 @@ -309,14 +309,9 @@ } private void removeMemoryVolumesOfSnapshot(Snapshot snapshot) { - RemoveMemoryVolumesParameters parameters = new RemoveMemoryVolumesParameters(snapshot.getMemoryVolume(), getVmId()); - parameters.setParentCommand(getActionType()); - parameters.setEntityInfo(getParameters().getEntityInfo()); - parameters.setParentParameters(getParameters()); - VdcReturnValueBase retVal = getBackend().runInternalAction( VdcActionType.RemoveMemoryVolumes, - parameters, + new RemoveMemoryVolumesParameters(snapshot.getMemoryVolume(), getVmId()), ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); if (!retVal.getSucceeded()) { 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 ad8d2dd..1c28434 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 @@ -1172,14 +1172,9 @@ } private void removeMemoryVolumes(String memoryVolume, Guid vmId) { - RemoveMemoryVolumesParameters parameters = new RemoveMemoryVolumesParameters(memoryVolume, vmId); - parameters.setParentCommand(getActionType()); - parameters.setEntityInfo(getParameters().getEntityInfo()); - parameters.setParentParameters(getParameters()); - VdcReturnValueBase retVal = getBackend().runInternalAction( VdcActionType.RemoveMemoryVolumes, - parameters, + new RemoveMemoryVolumesParameters(memoryVolume, vmId), ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); if (!retVal.getSucceeded()) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java index da03968..3a942de 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java @@ -4,13 +4,14 @@ import java.util.Collections; import java.util.List; -import org.ovirt.engine.core.bll.memory.HibernationVolumesRemover; +import org.ovirt.engine.core.bll.memory.MemoryImageRemoverOnDataDomain; import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.RemoveMemoryVolumesParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.asynctasks.AsyncTaskCreationInfo; +import org.ovirt.engine.core.common.asynctasks.AsyncTaskType; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.NotImplementedException; @@ -22,8 +23,6 @@ @NonTransactiveCommandAttribute @InternalCommandAttribute public class RemoveMemoryVolumesCommand<T extends RemoveMemoryVolumesParameters> extends CommandBase<T> implements TaskHandlerCommand<T> { - /** fictive list of task IDs, used when we don't want to add tasks */ - private static final ArrayList<Guid> dummyTaskIdList = new ArrayList<>(); public RemoveMemoryVolumesCommand(T parameters) { super(parameters); @@ -35,13 +34,18 @@ @Override protected void executeCommand() { - HibernationVolumesRemover hibernationVolumesRemover = - new HibernationVolumesRemover( - getParameters().getMemoryVolumes(), + MemoryImageRemoverOnDataDomain memoryImageRemover = + new MemoryImageRemoverOnDataDomain( getParameters().getVmId(), this); - setSucceeded(hibernationVolumesRemover.remove()); + setSucceeded(memoryImageRemover.remove( + Collections.singleton(getParameters().getMemoryVolumes()))); + } + + @Override + protected AsyncTaskType getTaskType() { + return AsyncTaskType.deleteImage; } ////////////////////////////////// @@ -50,7 +54,8 @@ @Override public VdcActionType getActionType() { - return super.getActionType(); + return getParameters().getParentCommand() == VdcActionType.Unknown ? + super.getActionType() : getParameters().getParentCommand(); } /** @@ -58,7 +63,7 @@ */ @Override public Guid createTask(Guid taskId, AsyncTaskCreationInfo asyncTaskCreationInfo, VdcActionType parentCommand) { - return Guid.Empty; + return super.createTask(taskId, asyncTaskCreationInfo, parentCommand); } /** @@ -70,7 +75,7 @@ VdcActionType parentCommand, VdcObjectType vdcObjectType, Guid... entityIds) { - return Guid.Empty; + return super.createTask(taskId, asyncTaskCreationInfo, parentCommand, vdcObjectType, entityIds); } /** @@ -78,7 +83,7 @@ */ @Override public ArrayList<Guid> getTaskIdList() { - return dummyTaskIdList; + return super.getTaskIdList(); } @Override @@ -91,7 +96,7 @@ */ @Override public Guid persistAsyncTaskPlaceHolder() { - return Guid.Empty; + return super.persistAsyncTaskPlaceHolder(getActionType()); } /** @@ -99,7 +104,7 @@ */ @Override public Guid persistAsyncTaskPlaceHolder(String taskKey) { - return Guid.Empty; + return super.persistAsyncTaskPlaceHolder(getActionType(), taskKey); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java index 6cccb55..11f1876 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java @@ -10,7 +10,6 @@ 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.MemoryImageRemoverOnDataDomain; import org.ovirt.engine.core.bll.network.ExternalNetworkManager; import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter; @@ -25,6 +24,7 @@ import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.RemoveAllVmImagesParameters; +import org.ovirt.engine.core.common.action.RemoveMemoryVolumesParameters; import org.ovirt.engine.core.common.action.RemoveVmParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; @@ -117,11 +117,35 @@ } } - new MemoryImageRemoverOnDataDomain(getVm(), this).remove(memoryStates); + removeMemoryVolumes(); return true; } + private void removeMemoryVolumes() { + for (String memoryState : memoryStates) { + VdcReturnValueBase retVal = getBackend().runInternalAction( + VdcActionType.RemoveMemoryVolumes, + buildRemoveMemoryVolumesParameters(memoryState, getVmId()), + ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); + + if (!retVal.getSucceeded()) { + log.errorFormat("Failed to remove memory volumes whie removing vm {0} (volumes: {1})", + getVmId(), memoryState); + } + } + } + + private RemoveMemoryVolumesParameters buildRemoveMemoryVolumesParameters(String memoryState, Guid vmId) { + RemoveMemoryVolumesParameters parameters = + new RemoveMemoryVolumesParameters(memoryState, getVmId()); + parameters.setParentCommand(getActionType()); + parameters.setParentParameters(getParameters()); + parameters.setEntityInfo(getParameters().getEntityInfo()); + + return parameters; + } + @Override protected boolean canDoAction() { if (getVm() == null) { 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 3750a91..54322b2 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 @@ -60,13 +60,17 @@ if (isMemoryStateRemovable(memoryVolumes)) { return removeMemoryVolumes(memoryVolumes); } + return true; } - protected void removeMemoryVolumes(Set<String> memoryVolumes) { + protected boolean removeMemoryVolumes(Set<String> memoryVolumes) { + boolean result = true; for (String memoryVols : memoryVolumes) { - removeMemoryVolume(memoryVols); + result &= removeMemoryVolume(memoryVols); } + + return result; } private boolean removeMemoryVolumes(String memVols) { 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 ac48c3e..0a4ec61 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 @@ -4,7 +4,7 @@ import java.util.Set; import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; -import org.ovirt.engine.core.common.businessentities.VM; +import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; @@ -13,15 +13,15 @@ public class MemoryImageRemoverOnDataDomain extends MemoryImageRemover { protected Boolean cachedPostZero; - private VM vm; + private Guid vmId; - public MemoryImageRemoverOnDataDomain(VM vm, TaskHandlerCommand<?> enclosingCommand) { - super(enclosingCommand); - this.vm = vm; + public MemoryImageRemoverOnDataDomain(Guid vmId, TaskHandlerCommand<?> enclosingCommand) { + super(enclosingCommand, false); + this.vmId = vmId; } - public void remove(Set<String> memoryStates) { - removeMemoryVolumes(memoryStates); + public boolean remove(Set<String> memoryStates) { + return removeMemoryVolumes(memoryStates); } @Override @@ -38,15 +38,22 @@ protected boolean isPostZero() { if (cachedPostZero == null) { - cachedPostZero = isDiskWithWipeAfterDeleteExist(getDiskDao().getAllForVm(vm.getId())); + cachedPostZero = isDiskWithWipeAfterDeleteExist(getDiskDao().getAllForVm(vmId)); } return cachedPostZero; } @Override protected boolean isMemoryStateRemovable(String memoryVolume) { - return !memoryVolume.isEmpty() && - getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 0; + if (memoryVolume.isEmpty()) { + return false; + } + + // from RemoveVmCommand this command is called before the snapshot + // is removed from the DB (because it is transactive command) + return enclosingCommand.getParameters().getParentCommand() == VdcActionType.RemoveVm ? + getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 0 + : getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1; } protected SnapshotDao getSnapshotDao() { -- To view, visit http://gerrit.ovirt.org/23222 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib4b270ec0e1ab41cae34459dde9f9cf47b1b5bdf Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
