Arik Hadas has posted comments on this change. Change subject: core: New internal command, VmSlaPolicyCommand ......................................................................
Patch Set 8: (6 comments) http://gerrit.ovirt.org/#/c/27646/8/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 28: public VmSlaPolicyCommand(T parameters) { Line 29: super(parameters); Line 30: setVmId((Guid) (getParameters().getSlaParameter(VmSlaPolicyParameters.Params.VM_ID))); Line 31: setVdsId((Guid) getParameters().getSlaParameter(VmSlaPolicyParameters.Params.VDS_ID)); Line 32: setVdsGroupId(getVds().getVdsGroupId()); getVds() might return null.. Line 33: } Line 34: Line 35: @Override Line 36: protected boolean canDoAction() { Line 34: Line 35: @Override Line 36: protected boolean canDoAction() { Line 37: Line 38: if (!FeatureSupported.vmSlaPolicy(getVdsGroup().getcompatibility_version())) { getVdsGroup might be null Line 39: return failCanDoAction(VdcBllMessages.VM_SLA_POLICY_NOT_SUPPORTED); Line 40: } Line 41: Line 42: if (getVm() == null) { Line 43: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_EXIST); Line 44: } Line 45: if (getVm() == null) { Line 46: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_EXIST); Line 47: } one of the two checks above should be removed Line 48: if (getVm().getStatus() != VMStatus.Up) { Line 49: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL, Line 50: LocalizedVmStatus.from(getVm().getStatus())); Line 51: } Line 69: } else { Line 70: VdcFault fault = new VdcFault(); Line 71: fault.setError(vdsReturnValue.getVdsError().getCode()); Line 72: fault.setMessage(vdsReturnValue.getVdsError().getMessage()); Line 73: getReturnValue().setFault(fault); why this weird handling? Line 74: } Line 75: Line 76: } Line 77: http://gerrit.ovirt.org/#/c/27646/8/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/UpdateVmPolicyVDSCommand.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/UpdateVmPolicyVDSCommand.java: Line 19: status = build(); Line 20: proceedProxyReturnValue(); Line 21: } catch (RuntimeException e) { Line 22: setVdsRuntimeError(e); Line 23: // prevent exception handler from re-throwing an exception I don't understand why not.. Line 24: getVDSReturnValue().setExceptionString(null); Line 25: } Line 26: } Line 27: Line 28: protected StatusOnlyReturnForXmlRpc build() { Line 29: Map<String, Object> struct = new HashMap<>(); Line 30: struct.put("vmId", getParameters().getVmId().toString()); Line 31: struct.put("vcpuLimit", String.valueOf(getParameters().getCpuLimit())); Line 32: return getBroker().updateVmPolicy(struct); this call should not be in this method Line 33: } Line 34: -- 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: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Kobi Ianko <k...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Jiří Moskovčák <jmosk...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@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
