Roy Golan has posted comments on this change.

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


Patch Set 8:

(5 comments)

https://gerrit.ovirt.org/#/c/39864/8/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 93: 
Line 94:     /**
Line 95:      * Asynch method to publish updated KSM policy to all active hosts.
Line 96:      */
Line 97:     private void publishUpdatedKsmPolicyToHosts() {
helper methods on the bottom, by order of apearance
Line 98:          // if ksm-enabled updated or ksm-merge-across-nodes updated
Line 99:         if ((getVdsGroup().isKsmMergeAcrossNumaNodes() != 
getPrevVdsGroup().isKsmMergeAcrossNumaNodes()) ||
Line 100:                 (getVdsGroup().isEnableKsm() != 
getPrevVdsGroup().isEnableKsm())) {
Line 101:             momPolicyUpdatedEvent.fire(getVdsGroup());


Line 144:         }
Line 145: 
Line 146:         if (getParameters().isForceResetEmulatedMachine()) {
Line 147:             
getParameters().getVdsGroup().setDetectEmulatedMachine(true);
Line 148:         }
add a check if succeeded() and out it in the end - after we make sure 
everything is saved and updated
Line 149:         publishUpdatedKsmPolicyToHosts();
Line 150: 
Line 151:         getVdsGroupDAO().update(getParameters().getVdsGroup());
Line 152:         addOrUpdateAddtionalClusterFeatures();


https://gerrit.ovirt.org/#/c/39864/8/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 534: getVdsDao()
please inject the VdsDao. see private members on the top of the class


Line 554:  return retVa
no one is using is so better off. unless you intend to do something with it?


Line 559: callables.size() > 0
btw, ThreadPoolUtil will do nothing if the list is empty so you can spare the 
check here.


-- 
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: 8
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

Reply via email to