Liron Aravot has uploaded a new change for review. Change subject: [wip] core: re-factor MoveOrCopy revert method ......................................................................
[wip] core: re-factor MoveOrCopy revert method refactor to the revert method that removes the parent command checks and unifies the flow. Change-Id: I92036258512d385b7296e1f75d9dcebfdd129d4a Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.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/MoveOrCopyImageGroupCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveImageParameters.java 7 files changed, 56 insertions(+), 37 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/89/12689/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java index ea76557..1e6ed1c 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java @@ -60,6 +60,8 @@ srcStorageDomainId, diskImage.getId(), diskImage.getImageId(), parentCommandType); + parameters.setShouldAttemptToRevert(true); + parameters.setShouldPerformRevertDbOpsAfterTasksEnded(false); VdcReturnValueBase result = executeChildCopyingCommand(parameters); handleCopyResult(newDiskImage, parameters, result); } 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 a09ea32..e8ad6f8 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 @@ -602,6 +602,8 @@ p.setStoragePoolId(getParameters().getStoragePoolId()); p.setImportEntity(true); p.setEntityId(getVm().getId()); + p.setShouldAttemptToRevert(true); + p.setShouldPerformRevertDbOpsAfterTasksEnded(true); p.setQuotaId(disk.getQuotaId() != null ? disk.getQuotaId() : getParameters().getQuotaId()); if (getParameters().getVm().getDiskMap() != null && getParameters().getVm().getDiskMap().containsKey(diskGuidList.get(i))) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java index 91a9170..3841ff5 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java @@ -176,6 +176,7 @@ if (!getParameters().isImportEntity()) { unLockImage(); } + if (getMoveOrCopyImageOperation() == ImageOperation.Copy) { if (getParameters().getAddImageDomainMapping()) { // remove image-storage mapping @@ -183,6 +184,9 @@ (new ImageStorageDomainMapId(getParameters().getImageId(), getParameters().getStorageDomainId())); } + } + + if (getParameters().isShouldAttemptToRevert()) { revertTasks(); } setSucceeded(true); @@ -190,30 +194,23 @@ @Override protected void revertTasks() { - // Revert should be performed only for AddVmFromSnapshot at this point. - if (getParameters().getParentCommand() == VdcActionType.AddVmFromSnapshot || getParameters().getParentCommand() == VdcActionType.ImportVm) { - Guid destImageId = getParameters().getDestinationImageId(); - RemoveImageParameters removeImageParams = - new RemoveImageParameters(destImageId); - if (getParameters().getParentCommand() == VdcActionType.ImportVm) { - removeImageParams.setParentParameters(removeImageParams); - removeImageParams.setParentCommand(VdcActionType.RemoveImage); - removeImageParams.setRemoveDuringExecution(false); - removeImageParams.setRemoveFromDB(true); - } else { - removeImageParams.setParentParameters(getParameters()); - removeImageParams.setParentCommand(VdcActionType.MoveOrCopyImageGroup); - } - removeImageParams.setEntityId(getDestinationImageId()); - // Setting the image as the monitored entity, so there will not be dependency - VdcReturnValueBase returnValue = - checkAndPerformRollbackUsingCommand(VdcActionType.RemoveImage, removeImageParams); - if (returnValue.getSucceeded()) { - // Starting to monitor the the tasks - RemoveImage is an internal command - // which adds the taskId on the internal task ID list - startPollingAsyncTasks(returnValue.getInternalTaskIdList()); - } + Guid destImageId = getParameters().getDestinationImageId(); + RemoveImageParameters removeImageParams = + new RemoveImageParameters(destImageId); + removeImageParams.setParentParameters(removeImageParams); + removeImageParams.setParentCommand(VdcActionType.RemoveImage); + removeImageParams.setPerformDbOperationsDuringExecution(!getParameters().isShouldPerformRevertDbOpsAfterTasksEnded()); + removeImageParams.setRemoveFromDB(true); + removeImageParams.setEntityId(getDestinationImageId()); + // Setting the image as the monitored entity, so there will not be dependency + VdcReturnValueBase returnValue = + checkAndPerformRollbackUsingCommand(VdcActionType.RemoveImage, removeImageParams); + if (returnValue.getSucceeded()) { + // Starting to monitor the the tasks - RemoveImage is an internal command + // which adds the taskId on the internal task ID list + startPollingAsyncTasks(returnValue.getInternalTaskIdList()); } + } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java index 1e9208a..b7565f5 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java @@ -278,7 +278,7 @@ p.setTransactionScopeOption(TransactionScopeOption.Suppress); p.setDiskImage(diskImage); p.setParentCommand(VdcActionType.RemoveDisk); - p.setRemoveDuringExecution(false); + p.setPerformDbOperationsDuringExecution(false); p.setEntityId(getParameters().getEntityId()); p.setParentParameters(getParameters()); p.setStorageDomainId(getParameters().getStorageDomainId()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java index 34a7132..ed4a395 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java @@ -80,7 +80,7 @@ VdcObjectType.Storage, getParameters().getStorageDomainId())); - if (getParameters().isRemoveDuringExecution() + if (getParameters().isPerformDbOperationsDuringExecution() && getParameters().getParentCommand() != VdcActionType.RemoveVmFromImportExport && getParameters().getParentCommand() != VdcActionType.RemoveVmTemplateFromImportExport) { removeImageFromDB(false); @@ -269,15 +269,15 @@ } private void endCommand() { - if (getParameters().getRemoveFromDB()) { - if (!getParameters().isRemoveDuringExecution()) { + if (!getParameters().isPerformDbOperationsDuringExecution()) { + if (getParameters().getRemoveFromDB()) { removeImageFromDB(true); + } else { + getImageStorageDomainMapDao().remove( + new ImageStorageDomainMapId(getParameters().getImageId(), + getParameters().getStorageDomainId())); + unLockImage(); } - } else { - getImageStorageDomainMapDao().remove( - new ImageStorageDomainMapId(getParameters().getImageId(), - getParameters().getStorageDomainId())); - unLockImage(); } setSucceeded(true); diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java index 039baf8..6578191 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java @@ -18,6 +18,8 @@ private boolean forceOverride; private NGuid sourceDomainId; private Guid destImageGroupId; + private boolean shouldAttemptToRevert; + private boolean shouldPerformRevertDbOpsAfterTasksEnded; public MoveOrCopyImageGroupParameters() { } @@ -121,6 +123,22 @@ forceOverride = value; } + public boolean isShouldAttemptToRevert() { + return shouldAttemptToRevert; + } + + public void setShouldAttemptToRevert(boolean shouldAttemptToRevert) { + this.shouldAttemptToRevert = shouldAttemptToRevert; + } + + public boolean isShouldPerformRevertDbOpsAfterTasksEnded() { + return shouldPerformRevertDbOpsAfterTasksEnded; + } + + public void setShouldPerformRevertDbOpsAfterTasksEnded(boolean shouldPerformRevertDbOpsAfterTasksEnded) { + this.shouldPerformRevertDbOpsAfterTasksEnded = shouldPerformRevertDbOpsAfterTasksEnded; + } + public NGuid getSourceDomainId() { return sourceDomainId; } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveImageParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveImageParameters.java index 22b2197..0493a98 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveImageParameters.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveImageParameters.java @@ -8,7 +8,7 @@ private DiskImage diskImage; private boolean removeFromDB; - private boolean removeDuringExecution = true; + private boolean performDbOperationsDuringExecution = true; public RemoveImageParameters(Guid imageId) { super(imageId, null); @@ -27,12 +27,12 @@ diskImage = value; } - public boolean isRemoveDuringExecution() { - return removeDuringExecution; + public boolean isPerformDbOperationsDuringExecution() { + return performDbOperationsDuringExecution; } - public void setRemoveDuringExecution(boolean removeDuringExecution) { - this.removeDuringExecution = removeDuringExecution; + public void setPerformDbOperationsDuringExecution(boolean performDbOperationsDuringExecution) { + this.performDbOperationsDuringExecution = performDbOperationsDuringExecution; } public void setRemoveFromDB(boolean removeFromDB) { -- To view, visit http://gerrit.ovirt.org/12689 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I92036258512d385b7296e1f75d9dcebfdd129d4a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
