Roy Golan has posted comments on this change.

Change subject: engien: Add KSM policy to NUMA hosts
......................................................................


Patch Set 1:

(6 comments)

https://gerrit.ovirt.org/#/c/39864/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsGroupCommand.java:

Line 145: 
Line 146:         setSucceeded(true);
Line 147:     }
Line 148: 
Line 149:     private void publishMomPolicyOnKsmPolicyChange() {
I want this out of the responsibility of the command. 

I think its fair to fire a CDI event for that.

at the top of the command create a 


 @Inject
 Event<VDSGroup> policyUpdated;


and in line 125 just do:


 policyChanged.fire(vdsCluster);


next, create a class with the code below which has a listener or observer method


 public class MomPolicyListener {

   public void policyChanged(@Observes VdsGroup cluster) {
     ...
   }
 }
Line 150:         // collect all Active hosts into a callable list
Line 151:         List<Callable<VDSReturnValue>> callables = new LinkedList<>();
Line 152:         final VDSGroup cluster = getVdsGroup();
Line 153:         for (final VDS vds : allForVdsGroup) {


Line 150:         // collect all Active hosts into a callable list
Line 151:         List<Callable<VDSReturnValue>> callables = new LinkedList<>();
Line 152:         final VDSGroup cluster = getVdsGroup();
Line 153:         for (final VDS vds : allForVdsGroup) {
Line 154:             if (vds.getStatus() == VDSStatus.Up) {
nice, as InitVdsOnUp covers us.
Line 155:                 callables.add(new Callable<VDSReturnValue>() {
Line 156:                     @Override
Line 157:                     public VDSReturnValue call() {
Line 158:                         return runUpdateMomPolicy(cluster, vds);


Line 166:     }
Line 167: 
Line 168:     private VDSReturnValue runUpdateMomPolicy(final VDSGroup cluster, 
final VDS vds) {
Line 169:         VDSReturnValue returnValue = new VDSReturnValue();
Line 170:         if (cluster.getCompatibilityVersion().compareTo(Version.v3_5) 
>= 0) {
should we also update when the compat version is also 3.4?
Line 171:             try {
Line 172:                 returnValue = 
runVdsCommand(VDSCommandType.SetMOMPolicyParameters,
Line 173:                                 new MomPolicyVDSParameters(vds,
Line 174:                                         cluster.isEnableBallooning(),


Line 180: returnValue.setSucceeded(false)
if a VdcBllException was thrown then the return value is false


Line 213:         boolean hasVms = false;
Line 214:         boolean hasVmOrHost = false;
Line 215:         boolean sameCpuNames = false;
Line 216:         boolean allVdssInMaintenance = false;
Line 217: 
need to add a canDo for 3.5 (as ksm and numa are supported from 3.5)
Line 218:         List<VM> vmList = null;
Line 219: 
Line 220:         oldGroup = getVdsGroupDAO().get(getVdsGroup().getId());
Line 221:         if (oldGroup == null) {


https://gerrit.ovirt.org/#/c/39864/1/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/MomPolicyVDSParameters.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/MomPolicyVDSParameters.java:

Line 13:     public MomPolicyVDSParameters() {
Line 14:     }
Line 15: 
Line 16:     public MomPolicyVDSParameters(VDS vds,
Line 17:             boolean enableBallooning2,
pls remove the "2" postfix
Line 18:             boolean enableKsm2,
Line 19:             boolean ksmMergeAcrossNumaNodes2) {
Line 20:         super(vds.getId());
Line 21:         this.enableBalloon = enableBallooning2;


-- 
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <[email protected]>
Gerrit-Reviewer: Dudi Maroshi <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to