Kobi Ianko has posted comments on this change. Change subject: db: Adding Cpu Qos DB config ......................................................................
Patch Set 2: (7 comments) http://gerrit.ovirt.org/#/c/27686/2//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-05-14 15:38:40 +0300 Line 4: Commit: Kobi Ianko <k...@redhat.com> Line 5: CommitDate: 2014-06-23 14:58:14 +0300 Line 6: Line 7: db: Adding Cpu Qos DB config > component: core, db Done Line 8: Line 9: Adding the Cpu Qos db config, Line 10: the Cpu Qos consists of one parameter:cpu_limit. Line 11: The parameter will be added to the qos table. http://gerrit.ovirt.org/#/c/27686/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/CpuQos.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/qos/CpuQos.java: Line 22: } Line 23: Line 24: public void setCpuLimit(Integer cpuLimit) { Line 25: this.cpuLimit = cpuLimit; Line 26: } > not related to the patch Done Line 27: Line 28: @Override Line 29: public int valuesHashCode() { Line 30: final int prime = 31; http://gerrit.ovirt.org/#/c/27686/2/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/qos/CpuQosDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/qos/CpuQosDaoDbFacadeImpl.java: Line 11: public class CpuQosDaoDbFacadeImpl extends QosBaseDaoFacadeImpl<CpuQos> implements CpuQosDao { Line 12: Line 13: private static final String CPU_QOS = "CpuQos"; Line 14: private static final String SAVE_PROCEDURE_FORMAT = "Insert" + CPU_QOS; Line 15: private static final String UPDATE_PROCEDURE_FORMAT = "Update" + CPU_QOS; > above 3 lines should be handled in QosBaseDaoFacadeImpl - in a separate pat you do not want to write cpuQos or storageQos in a base class Line 16: Line 17: private static final CpuDaoDbFacadaeImplMapper MAPPER = Line 18: new CpuDaoDbFacadaeImplMapper(); Line 19: http://gerrit.ovirt.org/#/c/27686/2/packaging/dbscripts/qos_sp.sql File packaging/dbscripts/qos_sp.sql: Line 34: VALUES(v_id, v_qos_type, v_name, v_description, v_storage_pool_id, v_cpu_limit); Line 35: END; $procedure$ Line 36: LANGUAGE plpgsql; Line 37: Line 38: > why the separation?, please add sections. The cpu qos has different parameters then the other qos type, you cannot call the SP with missing parameters and adding dummy parameters at the caller side(engine) will be a bad ood Line 39: Create or replace FUNCTION UpdateQos(v_id uuid, Line 40: v_qos_type SMALLINT, Line 41: v_name VARCHAR(50), Line 42: v_description TEXT, http://gerrit.ovirt.org/#/c/27686/2/packaging/dbscripts/upgrade/03_05_0720_qos_and_storage_impl.sql File packaging/dbscripts/upgrade/03_05_0720_qos_and_storage_impl.sql: > related? it's a name change because of the conflict merges, once we have this file with a final name merge it will not be a problem Line 1: -- ---------------------------------------------------------------------- Line 2: -- table qos Line 3: -- ---------------------------------------------------------------------- Line 4: http://gerrit.ovirt.org/#/c/27686/2/packaging/dbscripts/upgrade/03_05_0730_add_cpu_limit_to_qos.sql File packaging/dbscripts/upgrade/03_05_0730_add_cpu_limit_to_qos.sql: Line 1: select fn_db_add_column('qos', 'cpu_limit', 'smallint default null'); > add comment (-- cpu_limit is...) I don't see any real documentation on the qos table or in other add column scripts so I doubt anyone will look for it here, but it does not hurt anyone so I'll add a sentence http://gerrit.ovirt.org/#/c/27686/2/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql File packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql: Line 231: select fn_db_add_config_value('StorageQosSupported','false','3.4'); Line 232: select fn_db_add_config_value('CpuQosSupported','false','3.0'); Line 233: select fn_db_add_config_value('CpuQosSupported','false','3.1'); Line 234: select fn_db_add_config_value('CpuQosSupported','false','3.3'); Line 235: select fn_db_add_config_value('CpuQosSupported','false','3.4'); > 3.5? not needed Line 236: select fn_db_add_config_value('HostNetworkQosSupported', 'false', '3.0'); Line 237: select fn_db_add_config_value('HostNetworkQosSupported', 'false', '3.1'); Line 238: select fn_db_add_config_value('HostNetworkQosSupported', 'false', '3.2'); Line 239: select fn_db_add_config_value('HostNetworkQosSupported', 'false', '3.3'); -- To view, visit http://gerrit.ovirt.org/27686 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I533bcc601ac44289c51f5d56a6da0dd2455116d6 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Kobi Ianko <k...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Kobi Ianko <k...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@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
