Omer Frenkel has posted comments on this change. Change subject: core: Hot set number of CPUs using update VM ......................................................................
Patch Set 10: (2 comments) http://gerrit.ovirt.org/#/c/22757/10/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotSetNumberOfCpusCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotSetNumberOfCpusCommand.java: Line 35: if (getVm() == null) { Line 36: canDo = failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_EXIST); Line 37: } Line 38: if (getVm().getStatus() != VMStatus.Up) { Line 39: canDo = failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL, personally i prefer quick return on canDo, because it keep things simple, for example, you need to handle the case getVm() is null, as you continue and there is possible NPE here and below. i dont mind having it like this, just handle the NPE Line 40: LocalizedVmStatus.from(getVm().getStatus())); Line 41: } Line 42: if (getParameters().getVm().getNumOfCpus() > SlaValidator.getEffectiveCpuCores(getVds())) { Line 43: canDo = failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_CPUS); http://gerrit.ovirt.org/#/c/22757/10/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 115: } Line 116: Line 117: private void auditLogHotSetCpusCandos() { Line 118: if (!setNumberOfCpusResult.getCanDoAction()) { Line 119: AuditLogableBase logable = new HotSetNumberOfCpusCommand<>(new VmManagementParametersBase(newVmStatic)); why not new AuditLogableBase? and then set vmId, its more standard Line 120: List<String> canDos = getBackend().getErrorsTranslator(). Line 121: TranslateErrorText(setNumberOfCpusResult.getCanDoActionMessages()); Line 122: logable.addCustomValue(HotSetNumberOfCpusCommand.LOGABLE_FIELD_ERROR_MESSAGE, StringUtils.join(canDos, ",")); Line 123: AuditLogDirector.log(logable, AuditLogType.FAILED_HOT_SET_NUMBER_OF_CPUS); -- To view, visit http://gerrit.ovirt.org/22757 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I464a3dda5f143d1fcef63fd903eb615b01efe081 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Roy Golan <[email protected]> Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa <[email protected]> Gerrit-Reviewer: Leonardo Bianconi <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Vitor de Lima <[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
