Roy Golan has posted comments on this change. Change subject: core: automatically update vms on new version ......................................................................
Patch Set 7: (9 comments) http://gerrit.ovirt.org/#/c/23188/7/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 561: } Line 562: Line 563: private void endDefaultOperations() { Line 564: endUnlockOps(); Line 565: not a show stopper - do we want to copy the permission of the template over to the new one? Line 566: // in case of new version of a template, update vms marked to use latest Line 567: if (getParameters().getBaseTemplateId() != null) { Line 568: for (Guid vmId : getVmDAO().getVmIdsForVersionUpdate(getParameters().getBaseTemplateId())) { Line 569: VmManagementParametersBase params = new VmManagementParametersBase(); http://gerrit.ovirt.org/#/c/23188/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreStatelessVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreStatelessVmCommand.java: Line 41: // if it fail because of canDoAction, its safe to restore the snapshot Line 42: // and the vm will still be usable with previous version Line 43: if (!result.getSucceeded() && !result.getCanDoAction()) { Line 44: log.warnFormat("Couldn't update VM {0} ({1}) version, continue with restoring stateless snapshot.", Line 45: getVm().getName(), "...update VM {0} {1} from its template" - or something similar - just to make sure its an update caused by the template ? Line 46: getVmId()); Line 47: Line 48: boolean returnVal = true; Line 49: Guid snapshotId = DbFacade.getInstance().getSnapshotDao().getId(getVmId(), SnapshotType.STATELESS); http://gerrit.ovirt.org/#/c/23188/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmVersionCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmVersionCommand.java: Line 29: import org.ovirt.engine.core.common.utils.VmDeviceType; Line 30: import org.ovirt.engine.core.compat.Guid; Line 31: Line 32: /** Line 33: * This class updates VM to the latest template version for stateless vms that has newer template version there is no requierment to go back to a specific version. only latest? Line 34: */ Line 35: @InternalCommandAttribute Line 36: @LockIdNameAttribute Line 37: public class UpdateVmVersionCommand<T extends VmManagementParametersBase> extends VmCommand<T> { Line 56: } Line 57: Line 58: @Override Line 59: protected boolean canDoAction() { Line 60: if (getVm() == null) { VM should also be DOWN Line 61: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND); Line 62: } Line 63: Line 64: if (getVm().getTemplateVersionNumber() != null) { Line 109: setSucceeded(true); Line 110: } Line 111: } Line 112: Line 113: private void addUpdatedVm() { we have to code share this nice method somehow, otherwise it will be very quickly be outdated and cause bugs Line 114: VmManagementParametersBase addVmParams = new VmManagementParametersBase(getParameters().getVmStaticData()); Line 115: Line 116: addVmParams.setUseLatestVersion(true); Line 117: addVmParams.setDiskInfoDestinationMap(new HashMap<Guid, DiskImage>()); Line 133: addVmParams.setVmPayload(payload); Line 134: } Line 135: Line 136: // when this initiated from down vm event (restore stateless vm) Line 137: // then there is no session, so using the current user. nice one. in case of compensation its ok this will be blank? Line 138: if (StringUtils.isEmpty(getParameters().getSessionId())) { Line 139: addVmParams.setParametersCurrentUser(getCurrentUser()); Line 140: } Line 141: getBackend().runInternalAction(VdcActionType.AddVm, addVmParams, Line 164: log.errorFormat("Failed to copy field {0} of new version to VM {1} ({2}), error: {3}", Line 165: srcFld.getName(), Line 166: source.getName(), Line 167: source.getId(), Line 168: exp.getMessage()); if we fail here we have a stale VM version in hand. we should roleback Line 169: } Line 170: } Line 171: } Line 172: Line 177: Line 178: @Override Line 179: protected Map<String, Pair<String, String>> getExclusiveLocks() { Line 180: return Collections.singletonMap(getVmId().toString(), Line 181: LockMessagesMatchUtil.makeLockingPair(LockingGroup.VM, VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED)); should you lock the template too to make sure nobody is messing with it? Line 182: } Line 183: Line 184: @Override Line 185: protected void endVmCommand() { Line 188: } Line 189: Line 190: @Override Line 191: protected void endWithFailure() { Line 192: // nothing to do not safe for non stateless VMs - we should consider different rollback approach Line 193: } -- To view, visit http://gerrit.ovirt.org/23188 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5449eee954bd6096b6f2e423a75a8cac3e5bcc3b Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roy Golan <[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
