Yair Zaslavsky has posted comments on this change. Change subject: aaa: more fixes to command context propgation ......................................................................
Patch Set 6: (20 comments) http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java: Line 190 Line 191 Line 192 Line 193 Line 194 > without instead of null? Done http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java: Line 675: for (VdcActionParametersBase p : getParameters().getImagesParameters()) { Line 676: p.setTaskGroupSuccess(false); Line 677: Backend.getInstance().endAction(VdcActionType.CreateImageTemplate, Line 678: p, Line 679: cloneContext().withoutCompensationContext().withoutExecutionContext().withoutLock()); > detach? Done Line 680: } Line 681: Line 682: // if template exist in db remove it Line 683: if (getVmTemplate() != null) { http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachUserToVmFromPoolAndRunCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AttachUserToVmFromPoolAndRunCommand.java: Line 245: protected void endSuccessfully() { Line 246: if (getVm() != null) { Line 247: if (DbFacade.getInstance().getSnapshotDao().exists(getVm().getId(), SnapshotType.STATELESS)) { Line 248: setSucceeded(Backend.getInstance().endAction(VdcActionType.RunVm, Line 249: getParameters().getImagesParameters().get(0), cloneContext().withoutLock().withExecutionContext(null)).getSucceeded()); > withoutCompensationContext? see my answer under :) Line 250: Line 251: if (!getSucceeded()) { Line 252: log.warn("EndSuccessfully: endAction of RunVm failed, detaching user from Vm"); Line 253: detachUserFromVmFromPool(); // just in case. Line 273: @Override Line 274: protected void endWithFailure() { Line 275: setSucceeded(Backend.getInstance().endAction(VdcActionType.RunVm, Line 276: getParameters().getImagesParameters().get(0), Line 277: cloneContext().withExecutionContext(null).withoutLock()).getSucceeded()); > withoutCompensationContext? No, you meant probably withoutExecutionContext() :) Sometimes we do not "without" compensation context. Line 278: if (!getSucceeded()) { Line 279: log.warn("AttachUserToVmFromPoolAndRunCommand::EndWitFailure: endAction of RunVm Failed"); Line 280: } Line 281: detachUserFromVmFromPool(); http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommonVmPoolWithVmsCommand.java: Line 218: ExecutionMessageDirector.resolveStepMessage(StepEnum.ADD_VM_TO_POOL, values)); Line 219: ExecutionContext ctx = new ExecutionContext(); Line 220: ctx.setStep(addVmStep); Line 221: ctx.setMonitored(true); Line 222: commandCtx = cloneContext().withExecutionContext(ctx).withoutLock().withoutCompensationContext(); > detach().withExecutionContext(ctx) ? yes. Line 223: } catch (RuntimeException e) { Line 224: log.errorFormat("Failed to create command context of adding VM {0} to Pool {1}", Line 225: currentVmName, Line 226: getParameters().getVmPool().getName(), http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java: Line 658: Line 659: @Override Line 660: public CommandContext cloneContextAndDetachFromParent() { Line 661: return super.cloneContextAndDetachFromParent(); Line 662: } > why is this needed? i have previously explained. we discussed it, and you suggested that cloneContextAndDetachFromParent will be public at CommandBase, and I agreed. I will fix accordingly. Line 663: http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportRepoImageCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportRepoImageCommand.java: Line 112: Line 113: @Override Line 114: public CommandContext cloneContextAndDetachFromParent() { Line 115: return super.cloneContextAndDetachFromParent(); Line 116: } > why is this needed? Done Line 117: Line 118: @Override Line 119: public List<PermissionSubject> getPermissionCheckSubjects() { Line 120: List<PermissionSubject> permissionSubjects = new ArrayList<>(); http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java: Line 1363: Line 1364: @Override Line 1365: public CommandContext cloneContextAndDetachFromParent() { Line 1366: return super.cloneContextAndDetachFromParent(); Line 1367: } > why is it needed? Done http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmFromConfigurationCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmFromConfigurationCommand.java: Line 50: Line 51: @Override Line 52: public CommandContext cloneContextAndDetachFromParent() { Line 53: return super.cloneContextAndDetachFromParent(); Line 54: } > why is this needed? Done Line 55: Line 56: @Override Line 57: protected boolean canDoAction() { Line 58: if (isImagesAlreadyOnTarget() && !validateUnregisteredEntity(vmFromConfiguration, ovfEntityData)) { http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceVdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MaintenanceVdsCommand.java: Line 147: ctx.setMonitored(true); Line 148: } catch (RuntimeException e) { Line 149: log.error("Failed to create ExecutionContext for MigrateVmCommand", e); Line 150: } Line 151: return cloneContext().withExecutionContext(ctx).withoutCompensationContext().withoutLock(); > detach().withExecutionContext(ctx) ? Done Line 152: } Line 153: Line 154: @Override Line 155: protected boolean canDoAction() { http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotsCommand.java: Line 49: Line 50: @Override Line 51: public CommandContext cloneContextAndDetachFromParent() { Line 52: return super.cloneContextAndDetachFromParent(); Line 53: } > why is this needed? Done Line 54: Line 55: @Override Line 56: protected void init(T parameters) { Line 57: super.init(parameters); http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java: Line 81: Line 82: @Override Line 83: public CommandContext cloneContextAndDetachFromParent() { Line 84: return super.cloneContextAndDetachFromParent(); Line 85: } > why is this needed? Done Line 86: Line 87: @Override Line 88: public void taskEndSuccessfully() { Line 89: // Not implemented http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java: Line 410: Line 411: @Override Line 412: public CommandContext cloneContextAndDetachFromParent() { Line 413: return super.cloneContextAndDetachFromParent(); Line 414: } > why is this needed? Done Line 415: http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java: Line 146: Line 147: @Override Line 148: public CommandContext cloneContextAndDetachFromParent() { Line 149: return super.cloneContextAndDetachFromParent(); Line 150: } > why is this needed? Done Line 151: Line 152: /* Line 153: * get vm from export domain Line 154: */ http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmHibernationVolumesCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmHibernationVolumesCommand.java: Line 81: Line 82: @Override Line 83: public CommandContext cloneContextAndDetachFromParent() { Line 84: return super.cloneContextAndDetachFromParent(); Line 85: } > why is this needed? Done Line 86: Line 87: @Override Line 88: public Guid createTask(Guid taskId, Line 89: AsyncTaskCreationInfo asyncTaskCreationInfo, http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestartVdsCommand.java: Line 126: } Line 127: Line 128: public CommandContext getContext() { Line 129: if (commandContext == null) { Line 130: commandContext = cloneContext().withExecutionContext(getExecutionContext()).withLock(new EngineLock(getExclusiveLocks(), null)); > isn't this execution context part of the clone? done. Line 131: } Line 132: return commandContext; Line 133: } Line 134: http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java: Line 952: runStatelessVmCtx.setJob(job); Line 953: // Since run stateless step involves invocation of command, we should set the run stateless vm step as Line 954: // the "beginning step" of the child command. Line 955: runStatelessVmCtx.setStep(runStatelessStep); Line 956: return cloneContext().withExecutionContext(runStatelessVmCtx).withoutCompensationContext().withoutLock(); > deach().withExecutionContext() ? Done Line 957: } Line 958: Line 959: @Override Line 960: protected void endWithFailure() { http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java: Line 98: Line 99: @Override Line 100: public CommandContext cloneContextAndDetachFromParent() { Line 101: return super.cloneContextAndDetachFromParent(); Line 102: } > why is this needed? Done Line 103: Line 104: @Override Line 105: public VdcActionType getActionType() { Line 106: return super.getActionType(); http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java: Line 110: Line 111: @Override Line 112: public CommandContext cloneContextAndDetachFromParent() { Line 113: return super.cloneContextAndDetachFromParent(); Line 114: } > why is this needed? Done Line 115: Line 116: public Guid persistAsyncTaskPlaceHolder() { Line 117: return super.persistAsyncTaskPlaceHolder(getActionType()); Line 118: } http://gerrit.ovirt.org/#/c/29290/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/NetworkConfigurator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/NetworkConfigurator.java: Line 261: } Line 262: Line 263: private CommandContext cloneOnlyEngineContext() { Line 264: return commandContext.clone().withoutCompensationContext().withoutExecutionContext().withoutLock(); Line 265: } > leftover? ok, should be renamd, but in general this does not extend CommandBase as you can see. Line 266: Line 267: private DbFacade getDbFacade() { Line 268: return DbFacade.getInstance(); Line 269: } -- To view, visit http://gerrit.ovirt.org/29290 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icbf9e0216c83d981d11bb2d8d2a939fd2a279d9d Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
