Roy Golan has posted comments on this change. Change subject: engien: Add KSM policy to NUMA hosts ......................................................................
Patch Set 7: (3 comments) https://gerrit.ovirt.org/#/c/39864/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java: Line 523: * An internal parameters carrier class. A constraint from JBoss 7.1 - can't pass parameterized class in CDI event Line 524: * Preferred to use type safe object rather then Object[] or List<Object> Line 525: */ Line 526: public static class MomPolicyUpdateParams { Line 527: final VDSGroup cluster; 1. inner classes are fine but this class is and API class and I'd like to keep it as lean as possible. 2. all fields are publicly exposed to mutation. why? 3.alternatively you can send only the cluster as payload and skip the wrapper class and fetch all active hosts with onUpdate(@Observers VdsGroup vdsGroup) { vdsDAO.getAllForVdsGroupWithStatus( vdsGroup.id, VdsStatus.UP } Line 528: final List<VDS> allHostsInCluster; Line 529: Line 530: MomPolicyUpdateParams(VDSGroup cluster, List<VDS> allHostsInCluster) { Line 531: this.cluster = cluster; Line 544: callables.add(new Callable<VDSReturnValue>() { Line 545: @Override Line 546: public VDSReturnValue call() { Line 547: VDSReturnValue returnValue = new VDSReturnValue(); Line 548: if (policyParams.cluster.getCompatibilityVersion().compareTo(Version.v3_4) >= 0) { why not put this check on line 538? Line 549: try { Line 550: returnValue = Backend.getInstance().getResourceManager() Line 551: .RunVdsCommand(VDSCommandType.SetMOMPolicyParameters, Line 552: new MomPolicyVDSParameters(vds, Line 550: getResourceManager() use resourceManagerProvider.get() , its an instance member -- To view, visit https://gerrit.ovirt.org/39864 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9efa046547fb3732f0fb059092e5f444d72d589c Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Dudi Maroshi <d...@redhat.com> Gerrit-Reviewer: Dudi Maroshi <d...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
