Roy Golan has posted comments on this change. Change subject: core: need vds id when build vm numa properties ......................................................................
Patch Set 7: (3 comments) http://gerrit.ovirt.org/#/c/27830/7/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CreateVDSCommand.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/CreateVDSCommand.java: Line 18: super(parameters, parameters.getVm().getId()); Line 19: vm = parameters.getVm(); Line 20: //Use this field to pass the vdsId into builder which will be used by buildVmNumaProperties Line 21: //It will be set back to null in buildVmNumaProperties since the actual runOnVds value will Line 22: //be set in CreateVmVDSCommand.executeVdsIdCommand(). instead of using that vm field please just pass it to the builder constructor new VmInfoBuilder(vm, vdsId, createInfo) Line 23: vm.setRunOnVds(getParameters().getVdsId()); Line 24: createInfo = new HashMap<String, Object>(); Line 25: builder = createBuilder(); Line 26: } http://gerrit.ovirt.org/#/c/27830/7/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilder.java: Line 1039: * have almost the same libvirt version support Line 1040: * Line 1041: * @param compatibilityVersion Line 1042: */ Line 1043: private void addNumaSetting(final String compatibilityVersion) { lots of stuff to unit test here. please create a numa settings factory and add test to it. this is not straight forward to follow. make sure the factory is receiving the inputs as arguments and don't call dbfacade from within the factory should create the numa structure we want to append to the createInfo as a hunch I think it should look like new NumaSturcture( compatVersion, vmNodes, vdsNumaNodes) Line 1044: if (Boolean.TRUE.equals(Config.<Boolean> getValue(ConfigValues.CpuPinningEnabled, Line 1045: compatibilityVersion))) { Line 1046: NumaTuneMode numaTune = vm.getNumaTuneMode() == null ? NumaTuneMode.INTERLEAVE : vm.getNumaTuneMode(); Line 1047: List<VmNumaNode> vmNumaNodes = DbFacade.getInstance().getVmNumaNodeDAO().getAllVmNumaNodeByVmId(vm.getId()); http://gerrit.ovirt.org/#/c/27830/7/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilderBase.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VmInfoBuilderBase.java: Line 318: protected abstract void buildVmVirtioScsi(); Line 319: Line 320: protected abstract void buildVmRngDevice(); Line 321: Line 322: protected abstract void buildVmNumaProperties(); thanks Line 323: Line 324: protected static enum VNIC_PROFILE_PROPERTIES { Line 325: PORT_MIRRORING("Port Mirroring"), Line 326: CUSTOM_PROPERTIES("Custom Properties"), -- To view, visit http://gerrit.ovirt.org/27830 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I19be54812d0ebbb54145f33e6b036af84ff1d8ae Gerrit-PatchSet: 7 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: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@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