Gilad Chaplik has posted comments on this change. Change subject: core: move vm numa fields to VmBase ......................................................................
Patch Set 10: (6 comments) http://gerrit.ovirt.org/#/c/28341/10/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmBase.java: Line 869 Line 870 Line 871 Line 872 Line 873 > is that a redundant bracket we have on master? yes. Line 224: @EditableField Line 225: private NumaTuneMode numaTuneMode; Line 226: Line 227: @EditableField Line 228: private List<VmNumaNode> vNumaNodeList; > I know its not your code, just minor naming convention comment - if its plu will post a later patch that does that... currently I have some patches depends on it :-) Line 229: Line 230: public VmBase() { Line 231: name = ""; Line 232: interfaces = new ArrayList<VmNetworkInterface>(); Line 246: ssoMethod = SsoMethod.GUEST_AGENT; Line 247: singleQxlPci = true; Line 248: spiceFileTransferEnabled = true; Line 249: spiceCopyPasteEnabled = true; Line 250: setNumaTuneMode(NumaTuneMode.INTERLEAVE); > wouldn't you want it in the field init? so other constructors would have th this is required for GWT code (as you can see, all other fields does it) Line 251: vNumaNodeList = new ArrayList<VmNumaNode>(); Line 252: } Line 253: Line 254: @EditableField Line 889: && bootMenuEnabled == other.bootMenuEnabled Line 890: && spiceFileTransferEnabled == other.spiceFileTransferEnabled Line 891: && spiceCopyPasteEnabled == other.spiceCopyPasteEnabled Line 892: && ObjectUtils.objectsEqual(cpuProfileId, other.cpuProfileId) Line 893: && ObjectUtils.objectsEqual(numaTuneMode.getValue(), other.numaTuneMode.getValue()) > possible NPE when one of the members could be null will remove the NumaTuneMode c'tor that requires value parameter, the value is initialized in the default ctor, so no worried (for now). about NPE, other than that, numaTuneMode is set in ctor. Line 894: && ObjectUtils.objectsEqual(vNumaNodeList, other.vNumaNodeList)); Line 895: } Line 896: Line 897: public Guid getQuotaId() { http://gerrit.ovirt.org/#/c/28341/10/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmBaseDaoDbFacade.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmBaseDaoDbFacade.java: Line 124: entity.setDedicatedVmForVds(getGuid(rs, "dedicated_vm_for_vds")); Line 125: entity.setMinAllocatedMem(rs.getInt("min_allocated_mem")); Line 126: entity.setQuotaId(getGuid(rs, "quota_id")); Line 127: entity.setCpuProfileId(getGuid(rs, "cpu_profile_id")); Line 128: entity.setNumaTuneMode(NumaTuneMode.forValue(rs.getString("numatune_mode"))); > why aren't we using an integer id as value as the the rest of our enums? Not quite accurate (git grep '.forValue(rs')). I suggest to leave it for now. Line 129: } Line 130: } http://gerrit.ovirt.org/#/c/28341/10/packaging/dbscripts/vm_templates_sp.sql File packaging/dbscripts/vm_templates_sp.sql: Line 59: v_is_boot_menu_enabled BOOLEAN, Line 60: v_is_spice_file_transfer_enabled BOOLEAN, Line 61: v_is_spice_copy_paste_enabled BOOLEAN, Line 62: v_cpu_profile_id UUID, Line 63: v_numatune_mode VARCHAR(20)) > again id? line 53 -> v_template_type VARCHAR(40). same as previous comment can be discussed for a later refactoring. Line 64: Line 65: RETURNS VOID Line 66: AS $procedure$ Line 67: DECLARE -- To view, visit http://gerrit.ovirt.org/28341 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icc69f8167b8468becef728617ec850dc9034d88f Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Xiaolei Shi <xiao-lei....@hp.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Xiaolei Shi <xiao-lei....@hp.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
