Arik Hadas has uploaded a new change for review. Change subject: core: handle 'image does not exists' error on remove hibernation vols ......................................................................
core: handle 'image does not exists' error on remove hibernation vols Introducing HibernationVolumesRemover class which is responsible for removing hibernation volumes, with proper handling of 'image does not exists' error which is not really an error, because the result is that the images don't exists, as we wanted. HibernationVolumesRemover is now being used by the stop VM commands (stop and shutdown) and by run VM, fixing the race described in bz 952603. Change-Id: Ief52a20315b29660c7038eab1cc8dbbb4c4ded3e Bug-Url: https://bugzilla.redhat.com/952603 Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/HibernationVolumesRemover.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 5 files changed, 215 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/74/17474/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java index 45fcb80..024532d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java @@ -15,10 +15,16 @@ import org.ovirt.engine.core.bll.job.ExecutionContext.ExecutionMethod; import org.ovirt.engine.core.bll.job.ExecutionHandler; import org.ovirt.engine.core.bll.job.JobRepositoryFactory; +import org.ovirt.engine.core.bll.memory.HibernationVolumesRemover; import org.ovirt.engine.core.bll.scheduling.RunVmDelayer; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.storage.StorageHelperDirector; +import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; +import org.ovirt.engine.core.common.VdcObjectType; +import org.ovirt.engine.core.common.action.VdcActionType; +import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.action.VmOperationParameterBase; +import org.ovirt.engine.core.common.asynctasks.AsyncTaskCreationInfo; import org.ovirt.engine.core.common.businessentities.IVdsAsyncCommand; import org.ovirt.engine.core.common.businessentities.LUNs; import org.ovirt.engine.core.common.businessentities.LunDisk; @@ -36,6 +42,7 @@ import org.ovirt.engine.core.common.vdscommands.UpdateVmDynamicDataVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.compat.NotImplementedException; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; @@ -47,7 +54,7 @@ * Base class for asynchronous running process handling */ public abstract class RunVmCommandBase<T extends VmOperationParameterBase> extends VmCommand<T> implements - IVdsAsyncCommand, RunVmDelayer { + IVdsAsyncCommand, RunVmDelayer, TaskHandlerCommand<T> { private static Log log = LogFactory.getLog(RunVmCommandBase.class); protected static final HashMap<Guid, Integer> _vds_pending_vm_count = new HashMap<Guid, Integer>(); @@ -162,7 +169,9 @@ getVm().setLastVdsRunOn(getCurrentVdsId()); } if (StringUtils.isNotEmpty(getVm().getHibernationVolHandle())) { - removeMemoryVolumes(getVm().getHibernationVolHandle(), getActionType(), true); + new HibernationVolumesRemover(getVm().getHibernationVolHandle(), + getVmId(), this, true).remove(); + // In order to prevent a race where VdsUpdateRuntimeInfo saves the Vm Dynamic as UP prior to execution of // this method (which is a part of the cached VM command, // so the state this method is aware to is RESTORING, in case of RunVmCommand after the VM got suspended. @@ -318,4 +327,59 @@ return Collections.singletonMap(getVmId().toString(), LockMessagesMatchUtil.makeLockingPair(LockingGroup.VM, VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED)); } + + /////////////////////////////////////// + // TaskHandlerCommand Implementation // + /////////////////////////////////////// + + public T getParameters() { + return super.getParameters(); + } + + public VdcActionType getActionType() { + return super.getActionType(); + } + + public VdcReturnValueBase getReturnValue() { + return super.getReturnValue(); + } + + public ExecutionContext getExecutionContext() { + return super.getExecutionContext(); + } + + public void setExecutionContext(ExecutionContext executionContext) { + super.setExecutionContext(executionContext); + } + + public Guid createTask(Guid taskId, + AsyncTaskCreationInfo asyncTaskCreationInfo, + VdcActionType parentCommand, + VdcObjectType entityType, + Guid... entityIds) { + return super.createTaskInCurrentTransaction(taskId, asyncTaskCreationInfo, parentCommand, entityType, entityIds); + } + + public Guid createTask(Guid taskId, + AsyncTaskCreationInfo asyncTaskCreationInfo, + VdcActionType parentCommand) { + return super.createTask(taskId, asyncTaskCreationInfo, parentCommand); + } + + public ArrayList<Guid> getTaskIdList() { + return super.getTaskIdList(); + } + + public void preventRollback() { + throw new NotImplementedException(); + } + + public Guid persistAsyncTaskPlaceHolder() { + return super.persistAsyncTaskPlaceHolder(getActionType()); + } + + public Guid persistAsyncTaskPlaceHolder(String taskKey) { + return super.persistAsyncTaskPlaceHolder(getActionType(), taskKey); + } + } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java index 235bb5a..c2a364e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java @@ -4,14 +4,20 @@ import java.util.List; import org.apache.commons.lang.StringUtils; +import org.ovirt.engine.core.bll.job.ExecutionContext; +import org.ovirt.engine.core.bll.memory.HibernationVolumesRemover; import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageDependent; import org.ovirt.engine.core.bll.quota.QuotaVdsDependent; import org.ovirt.engine.core.bll.quota.QuotaVdsGroupConsumptionParameter; +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.VdcActionType; +import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.action.VmOperationParameterBase; +import org.ovirt.engine.core.common.asynctasks.AsyncTaskCreationInfo; import org.ovirt.engine.core.common.asynctasks.EntityInfo; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.VMStatus; @@ -21,13 +27,14 @@ import org.ovirt.engine.core.common.vdscommands.UpdateVmDynamicDataVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.compat.NotImplementedException; import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; import org.ovirt.engine.core.utils.transaction.TransactionMethod; import org.ovirt.engine.core.utils.transaction.TransactionSupport; public abstract class StopVmCommandBase<T extends VmOperationParameterBase> extends VmOperationCommandBase<T> - implements QuotaVdsDependent, QuotaStorageDependent { + implements QuotaVdsDependent, QuotaStorageDependent, TaskHandlerCommand<T> { private boolean suspendedVm; protected StopVmCommandBase(Guid guid) { @@ -83,11 +90,25 @@ boolean removedHibernationVolumes = false; if (StringUtils.isNotEmpty(getVm().getHibernationVolHandle())) { removedHibernationVolumes = removeHibernationVolumes(); + + if (!removedHibernationVolumes) { + log.errorFormat("Could not remove hibernation volumes {0}, when stoping VM {1} ({2})", + getVm().getHibernationVolHandle(), + getVm().getName(), + getVm().getId()); + } } if (getVm().getStatus() == VMStatus.Suspended) { suspendedVm = true; + setSucceeded(removedHibernationVolumes); + + // it is possible that no task was created and the hibernation volumes removal + // was succeeded, in case the hibernation images don't exist + if (getSucceeded() && getTaskIdList().isEmpty()) { + endVmCommand(); + } } else { super.executeVmCommand(); @@ -114,9 +135,15 @@ // Set the VM to image locked to decrease race condition. updateVmStatus(VMStatus.ImageLocked); - // We call removeMemoryVolumes with startPollingTasks = false because this call - // is part of the exeuction stage so the task polling will be made anyway.. - if (!removeMemoryVolumes(getVm().getHibernationVolHandle(), getActionType(), false)) { + // We don't need the remover to poll the tasks because we are in the execution + // method so the task polling will be made later anyway.. + HibernationVolumesRemover hibernationVolumesRemover = + new HibernationVolumesRemover( + getVm().getHibernationVolHandle(), + getVmId(), + this); + + if (!hibernationVolumesRemover.remove()) { updateVmStatus(vmStatus); return false; } @@ -210,4 +237,58 @@ public void addQuotaPermissionSubject(List<PermissionSubject> quotaPermissionList) { // } + + /////////////////////////////////////// + // TaskHandlerCommand Implementation // + /////////////////////////////////////// + + public T getParameters() { + return super.getParameters(); + } + + public VdcActionType getActionType() { + return super.getActionType(); + } + + public VdcReturnValueBase getReturnValue() { + return super.getReturnValue(); + } + + public ExecutionContext getExecutionContext() { + return super.getExecutionContext(); + } + + public void setExecutionContext(ExecutionContext executionContext) { + super.setExecutionContext(executionContext); + } + + public Guid createTask(Guid taskId, + AsyncTaskCreationInfo asyncTaskCreationInfo, + VdcActionType parentCommand, + VdcObjectType entityType, + Guid... entityIds) { + return super.createTaskInCurrentTransaction(taskId, asyncTaskCreationInfo, parentCommand, entityType, entityIds); + } + + public Guid createTask(Guid taskId, + AsyncTaskCreationInfo asyncTaskCreationInfo, + VdcActionType parentCommand) { + return super.createTask(taskId, asyncTaskCreationInfo, parentCommand); + } + + public ArrayList<Guid> getTaskIdList() { + return super.getTaskIdList(); + } + + public void preventRollback() { + throw new NotImplementedException(); + } + + public Guid persistAsyncTaskPlaceHolder() { + return super.persistAsyncTaskPlaceHolder(getActionType()); + } + + public Guid persistAsyncTaskPlaceHolder(String taskKey) { + return super.persistAsyncTaskPlaceHolder(getActionType(), taskKey); + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/HibernationVolumesRemover.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/HibernationVolumesRemover.java new file mode 100644 index 0000000..206d4dc --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/HibernationVolumesRemover.java @@ -0,0 +1,59 @@ +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.vdscommands.DeleteImageGroupVDSCommandParameters; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; + +public class HibernationVolumesRemover extends MemoryImageRemover { + + private Boolean cachedPostZero; + private Guid vmId; + private String memoryState; + + public HibernationVolumesRemover(String memoryState, Guid vmId, + TaskHandlerCommand<?> enclosingCommand) { + this(memoryState, vmId, enclosingCommand, false); + } + + public HibernationVolumesRemover(String memoryState, Guid vmId, + TaskHandlerCommand<?> enclosingCommand, boolean startPollingTasks) { + super(enclosingCommand, startPollingTasks); + this.memoryState = memoryState; + this.vmId = vmId; + } + + public boolean remove() { + return removeMemoryVolume(memoryState); + } + + @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 = + DbFacade.getInstance().getDiskDao().getAllForVm(vmId).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/MemoryImageRemover.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java index b203311..50080fa 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 @@ -45,10 +45,11 @@ return !memoryVolume.isEmpty(); } - protected void removeMemoryVolume(String memoryVolumes) { + protected boolean removeMemoryVolume(String memoryVolumes) { if (isMemoryStateRemovable(memoryVolumes)) { - removeMemoryVolumes(memoryVolumes); + return removeMemoryVolumes(memoryVolumes); } + return true; } protected void removeMemoryVolumes(Set<String> memoryVolumes) { 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 32b5145..57195a7 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 @@ -28,8 +28,8 @@ } @Override - protected void removeMemoryVolume(String memoryVolumes) { - super.removeMemoryVolume( + protected boolean removeMemoryVolume(String memoryVolumes) { + return super.removeMemoryVolume( MemoryUtils.changeStorageDomainAndPoolInMemoryState( memoryVolumes, storageDomainId, storagePoolId)); } -- To view, visit http://gerrit.ovirt.org/17474 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ief52a20315b29660c7038eab1cc8dbbb4c4ded3e 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
