Arik Hadas has posted comments on this change. Change subject: core: add support to update running vm ......................................................................
Patch Set 7: (5 comments) http://gerrit.ovirt.org/#/c/26758/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java: Line 558: getVm minor: can we check the isNextRunConfigurationExists first? Line 633: ACTION_TYPE_FAILED_OBJECT_LOCKED should be short, I know, but don't you prefer to have an informative message anyway? http://gerrit.ovirt.org/#/c/26758/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolHandler.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolHandler.java: Line 59: Line 60: QuotaManager.getInstance().rollbackQuotaByVmId(vmId); Line 61: VmHandler.removeStatelessVmUnmanagedDevices(vmId); Line 62: Line 63: ApplyNextRunConfiguration(vmId); it reminds me the stateless snapshot handling, don't we need to handle a case where the VM went down and we didn't 'catch it' so the VM is starting while there is some next run configuration existing for the VM? Line 64: } Line 65: Line 66: /** Line 67: * Update vm configuration with NEXT_RUN configuration, if exists Line 118: updateVmParams.setVmPayload(new VmPayload(VmDeviceType.getByName(device.getDevice()), device.getSpecParams())); Line 119: } Line 120: break; Line 121: case CONSOLE: Line 122: updateVmParams.setConsoleEnabled(true); need to add empty 'default' clause Line 123: } Line 124: } Line 125: return updateVmParams; Line 126: } http://gerrit.ovirt.org/#/c/26758/7/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ObjectIdentityChecker.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ObjectIdentityChecker.java: Line 132: } Line 133: srcCls = srcCls.getSuperclass(); Line 134: dstCls = dstCls.getSuperclass(); Line 135: } Line 136: return true; I think we can use only one Class object, assuming both objects are from the same Class (and we assume it is true since the condition in the while is only on the srcCls), don't we? Line 137: } Line 138: Line 139: public final boolean IsUpdateValid(Object source, Object destination) { Line 140: if (source.getClass() != destination.getClass()) { -- To view, visit http://gerrit.ovirt.org/26758 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie433352852f53f62e58349ce85475ee89e37ce89 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
