Arik Hadas has uploaded a new change for review. Change subject: core: fix monitoring of SetNonOperationalVdsCommand ......................................................................
core: fix monitoring of SetNonOperationalVdsCommand This patch resolves a bug in which an exception was thrown when a step for an already completed and cleaned-up job is saved, when SetNonOperationalVdsCommand is invoked and finished and then a step for a migration it triggered is about to save. The solution is to extract the migrations to be monitored as different job, which reflects the process better on this scenario as the host become non-oprational independently of the status of the triggered migrations. The decision whether to monitor a command was changed to receive the parent command, so that we'll be able to return true (to monitor) in case the command is InternalMigrateVm and its parent is SetNonOperationalVds. Change-Id: I06b08108b1c75fe00b3f4dc6336c61a5e8c95d85 Bug-Url: https://bugzilla.redhat.com/888199 Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintananceVdsCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetNonOperationalVdsCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/ExecutionHandler.java M backend/manager/modules/dal/src/main/resources/bundles/ExecutionMessages.properties 6 files changed, 25 insertions(+), 7 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/70/11370/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java index 10ef8a8..3c816b3 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java @@ -344,7 +344,7 @@ CommandBase<?> command = CommandsFactory.CreateCommand(actionType, parameters); command.setInternalExecution(runAsInternal); command.setContext(context); - ExecutionHandler.prepareCommandForMonitoring(command, actionType, runAsInternal, hasCorrelationId); + ExecutionHandler.prepareCommandForMonitoring(command, actionType, parameters.getParentCommand(), runAsInternal, hasCorrelationId); returnValue = command.executeAction(); returnValue.setCorrelationId(parameters.getCorrelationId()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintananceVdsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintananceVdsCommand.java index 2657a55..a97a4c5 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintananceVdsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintananceVdsCommand.java @@ -11,6 +11,7 @@ import org.ovirt.engine.core.bll.job.ExecutionHandler; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.VdcObjectType; +import org.ovirt.engine.core.common.action.InternalMigrateVmParameters; import org.ovirt.engine.core.common.action.MaintananceVdsParameters; import org.ovirt.engine.core.common.action.StoragePoolParametersBase; import org.ovirt.engine.core.common.action.VdcActionType; @@ -106,7 +107,7 @@ if (vm.getStatus() != VMStatus.MigratingFrom && (!HAOnly || (HAOnly && vm.isAutoStartup()))) { VdcReturnValueBase result = Backend.getInstance().runInternalAction(VdcActionType.InternalMigrateVm, - new InternalMigrateVmParameters(vm.getId()), + new InternalMigrateVmParameters(vm.getId(), getActionType()), new CommandContext(createMigrateVmContext(parentContext, vm))); if (!result.getCanDoAction() || !(((Boolean) result.getActionReturnValue()).booleanValue())) { succeeded = false; @@ -118,7 +119,7 @@ return succeeded; } - private ExecutionContext createMigrateVmContext(ExecutionContext parentContext,VM vm) { + protected ExecutionContext createMigrateVmContext(ExecutionContext parentContext,VM vm) { ExecutionContext ctx = new ExecutionContext(); try { Map<String, String> values = new HashMap<String, String>(); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java index 9fb1495..e6dcd9e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java @@ -176,6 +176,7 @@ if (executionContext == null || executionContext.isMonitored()) { ExecutionHandler.prepareCommandForMonitoring(command, command.getActionType(), + command.getParameters().getParentCommand(), command.isInternalExecution(), new Boolean(hasCorrelationIdMap.get(command.getCommandId()))); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetNonOperationalVdsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetNonOperationalVdsCommand.java index 1309d2b..ff87220 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetNonOperationalVdsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetNonOperationalVdsCommand.java @@ -2,10 +2,12 @@ import java.util.Map.Entry; +import org.ovirt.engine.core.bll.job.ExecutionContext; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.SetNonOperationalVdsParameters; import org.ovirt.engine.core.common.businessentities.NonOperationalReason; import org.ovirt.engine.core.common.businessentities.VDSStatus; +import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.vdscommands.SetVdsStatusVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.dal.VdcBllMessages; @@ -24,6 +26,10 @@ setStorageDomainId(parameters.getStorageDomainId()); } + /** + * Note: it's ok that this method isn't marked as async command even though it triggers + * migrations as sub-commands, because those migrations are executed as different jobs + */ @Override protected void executeCommand() { if (getParameters().getSaveToDb()) { @@ -63,6 +69,11 @@ } @Override + protected ExecutionContext createMigrateVmContext(ExecutionContext parentContext, VM vm) { + return new ExecutionContext(); + } + + @Override protected boolean canDoAction() { boolean result = true; if (getVds() == null) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/ExecutionHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/ExecutionHandler.java index 4cacbb2..9e02e33 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/ExecutionHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/ExecutionHandler.java @@ -177,6 +177,7 @@ */ public static void prepareCommandForMonitoring(CommandBase<?> command, VdcActionType actionType, + VdcActionType parentActionType, boolean runAsInternal, boolean hasCorrelationId) { @@ -186,7 +187,7 @@ } try { - boolean isMonitored = shouldMonitorCommand(actionType, runAsInternal, hasCorrelationId); + boolean isMonitored = shouldMonitorCommand(actionType, runAsInternal, hasCorrelationId, parentActionType); // A monitored job is created for monitored external flows if (isMonitored || context.isJobRequired()) { @@ -221,9 +222,11 @@ * Indicates if the current command was executed under a correlation-ID * @return true if the command should be monitored, else false. */ - private static boolean shouldMonitorCommand(VdcActionType actionType, boolean isInternal, boolean hasCorrelationId) { - - return (actionType.isActionMonitored() || hasCorrelationId) && !isInternal; + private static boolean shouldMonitorCommand(VdcActionType actionType, boolean isInternal, boolean hasCorrelationId, VdcActionType parentActionType) { + if (actionType == VdcActionType.InternalMigrateVm && parentActionType == VdcActionType.SetNonOperationalVds) + return true; + else + return (actionType.isActionMonitored() || hasCorrelationId) && !isInternal; } /** diff --git a/backend/manager/modules/dal/src/main/resources/bundles/ExecutionMessages.properties b/backend/manager/modules/dal/src/main/resources/bundles/ExecutionMessages.properties index ece6294..edf34ac 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/ExecutionMessages.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/ExecutionMessages.properties @@ -11,6 +11,8 @@ job.RunVmOnce=Launching VM ${VM} in run-once mode job.MigrateVm=Migrating VM ${VM} job.MigrateVmToServer=Migrating VM ${VM} to Host ${VDS} +# TODO: this is a hack to handle internal migrations triggered from set-non-operational command, should be replaced +job.InternalMigrateVm=Migrating VM ${VM} because previous Host became non-operational job.ExportVm=Exporting VM ${VM} To Export Domain ${Storage} job.ExportVmTemplate=Exporting VM Template ${VmTemplate} To Export Domain ${Storage} job.AddDisk=Adding Disk ${DiskAlias} -- To view, visit http://gerrit.ovirt.org/11370 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I06b08108b1c75fe00b3f4dc6336c61a5e8c95d85 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
