Arik Hadas has posted comments on this change. Change subject: core: New internal command, VmSlaPolicyCommand ......................................................................
Patch Set 1: (6 comments) where is the new patch-set? http://gerrit.ovirt.org/#/c/27646/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java: Line 987: // External scheduler Line 988: EXTERNAL_SCHEDULER_PLUGIN_ERROR(10500, AuditLogSeverity.ERROR), Line 989: EXTERNAL_SCHEDULER_ERROR(10501, AuditLogSeverity.ERROR), Line 990: Line 991: VM_SLA_POLICY(10550), > followed the hot set convention, since sla policy cannot be added or delete ok Line 992: FAILED_VM_SLA_POLICY(10551, AuditLogSeverity.ERROR); Line 993: Line 994: private int intValue; Line 995: // indicates time interval in seconds on which identical events from same instance are suppressed. Line 988: EXTERNAL_SCHEDULER_PLUGIN_ERROR(10500, AuditLogSeverity.ERROR), Line 989: EXTERNAL_SCHEDULER_ERROR(10501, AuditLogSeverity.ERROR), Line 990: Line 991: VM_SLA_POLICY(10550), Line 992: FAILED_VM_SLA_POLICY(10551, AuditLogSeverity.ERROR); > same as above ok Line 993: Line 994: private int intValue; Line 995: // indicates time interval in seconds on which identical events from same instance are suppressed. Line 996: private int eventFloodRate; http://gerrit.ovirt.org/#/c/27646/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java: Line 41: DetachDiskFromVm(181, ActionGroup.CONFIGURE_VM_STORAGE, false, QuotaDependency.NONE), Line 42: HotPlugDiskToVm(182, ActionGroup.CONFIGURE_VM_STORAGE, false, QuotaDependency.NONE), Line 43: HotUnPlugDiskFromVm(183, ActionGroup.CONFIGURE_VM_STORAGE, false, QuotaDependency.NONE), Line 44: HotSetNumberOfCpus(184, ActionGroup.EDIT_VM_PROPERTIES, false, QuotaDependency.NONE), Line 45: VmSlaPolicy(185, ActionGroup.EDIT_VM_PROPERTIES, false, QuotaDependency.NONE), > since sla policy cannot be added or deleted only updated, we can skip that. ok Line 46: ChangeFloppy(35, QuotaDependency.NONE), Line 47: ImportVm(36, ActionGroup.IMPORT_EXPORT_VM, QuotaDependency.STORAGE), Line 48: RemoveVmFromImportExport(37, ActionGroup.DELETE_VM, QuotaDependency.NONE), Line 49: RemoveVmTemplateFromImportExport(38, ActionGroup.DELETE_TEMPLATE, QuotaDependency.NONE), http://gerrit.ovirt.org/#/c/27646/1/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 2: Line 3: import org.ovirt.engine.core.common.businessentities.VmStatic; Line 4: Line 5: Line 6: public class VmSlaPolicyParameters extends VmManagementParametersBase { > parameters will be added soon... the sla policy should grow... in that case, ok Line 7: Line 8: private static final long serialVersionUID = 3918909396931144459L; Line 9: Line 10: http://gerrit.ovirt.org/#/c/27646/1/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties: Line 795: USER_REMOVED_AFFINITY_GROUP=Affinity Group ${affinityGroupName} was removed. (User: ${UserName}) Line 796: USER_FAILED_TO_REMOVE_AFFINITY_GROUP=Failed to remove Affinity Group ${affinityGroupName}. (User: ${UserName}) Line 797: USER_SET_HOSTED_ENGINE_MAINTENANCE=Hosted Engine HA maintenance mode was updated on host ${VdsName}. Line 798: USER_FAILED_TO_SET_HOSTED_ENGINE_MAINTENANCE=Hosted Engine HA maintenance mode could not be updated on host ${VdsName}. Line 799: VM_SLA_POLICY=VM ${vmName} CPU limit is set to ${cpuLimit} > I should change that to a more general message since sla policy will be mor I think it will be useful for the user to know the problematic SLA attribute, so maybe you can generalize it but the message should still contain the name (and value probably) of the attribute Line 800: FAILED_VM_SLA_POLICY= Failed to set CPU limit to VM ${vmName}. Underlying error message: ${ErrorMessage} Line 801: Line 802: ISCSI_BOND_ADD_SUCCESS=iSCSI bond '${IscsiBondName}' was successfully created in Data Center '${StoragePoolName}'. Line 803: ISCSI_BOND_ADD_FAILED=Failed to create iSCSI bond '${IscsiBondName}' in Data Center '${StoragePoolName}'. http://gerrit.ovirt.org/#/c/27646/1/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 30: Line 31: protected void init() { Line 32: struct.put("vmId", getParameters().getVmId().toString()); Line 33: struct.put("vcpuLimit", String.valueOf(getParameters().getCpuLimit())); Line 34: } > Does it matter? I copied it from another place in the code. I don't think it will be useful in those simple vdsbroker commands, but I understand it is a matter of style. It used to be like that in the vds command of migration and I changed it: http://gerrit.ovirt.org/#/c/24798/6/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/MigrateBrokerVDSCommand.java,cm. just a suggestion.. Line 35: public static class Params extends VdsAndVmIDVDSParametersBase{ Line 36: Line 37: private int cpuLimit; Line 38: -- 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: 1 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: Kobi Ianko <k...@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
