Arik Hadas has posted comments on this change. Change subject: core: New internal command, VmSlaPolicyCommand ......................................................................
Patch Set 11: (5 comments) http://gerrit.ovirt.org/#/c/27646/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmSlaPolicyCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmSlaPolicyCommand.java: Line 26: Line 27: public VmSlaPolicyCommand(T parameters) { Line 28: super(parameters); Line 29: setVmId((Guid) (getParameters().getSlaParameter(VmSlaPolicyParameters.Params.VM_ID))); Line 30: setVdsId((Guid) getParameters().getSlaParameter(VmSlaPolicyParameters.Params.VDS_ID)); should be removed (and use getRunOnVds in the execute method instead) Line 31: if (getVds() != null) { Line 32: setVdsGroupId(getVds().getVdsGroupId()); Line 33: } Line 34: } Line 28: super(parameters); Line 29: setVmId((Guid) (getParameters().getSlaParameter(VmSlaPolicyParameters.Params.VM_ID))); Line 30: setVdsId((Guid) getParameters().getSlaParameter(VmSlaPolicyParameters.Params.VDS_ID)); Line 31: if (getVds() != null) { Line 32: setVdsGroupId(getVds().getVdsGroupId()); I don't see any usage of the cluster in this command, can we just not set it? Line 33: } Line 34: } Line 35: Line 36: @Override Line 35: Line 36: @Override Line 37: protected boolean canDoAction() { Line 38: if (getVdsGroup() == null) { Line 39: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CLUSTER_CAN_NOT_BE_EMPTY); if the last comment is correct, this one should be removed Line 40: } Line 41: if (!FeatureSupported.vmSlaPolicy(getVdsGroup().getcompatibility_version())) { Line 42: return failCanDoAction(VdcBllMessages.VM_SLA_POLICY_NOT_SUPPORTED); Line 43: } Line 74: Line 75: if (getSucceeded()) { Line 76: return AuditLogType.VM_SLA_POLICY; Line 77: } else { Line 78: addCustomValue(LOGABLE_FIELD_ERROR_MESSAGE, getReturnValue().getFault().getMessage()); the Fault in getReturnValue() is not set in the execute method now, so we should hold it in class member or something.. Line 79: return AuditLogType.FAILED_VM_SLA_POLICY; Line 80: } Line 81: Line 82: } http://gerrit.ovirt.org/#/c/27646/11/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmSlaPolicyParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmSlaPolicyParameters.java: Line 14: public VmSlaPolicyParameters() { Line 15: } Line 16: Line 17: public VmSlaPolicyParameters(Guid vdsId, Guid vmId, int cpuLimit) { Line 18: params.put(Params.VDS_ID, vdsId); we should not pass the vdsId to the command using the parameters, the command should use getVm().getRunOnVds() so it could be used from UpdateVmCommand later Line 19: params.put(Params.VM_ID, vmId); Line 20: params.put(Params.CPU_LIMIT, cpuLimit); Line 21: } Line 22: -- To view, visit http://gerrit.ovirt.org/27646 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I26b0794c52cfe1d352f26392a835023afcd9efa0 Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Kobi Ianko <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Jiří Moskovčák <[email protected]> Gerrit-Reviewer: Martin Sivák <[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
