Roy Golan has posted comments on this change. Change subject: engien: Add KSM policy to NUMA hosts ......................................................................
Patch Set 6: (5 comments) https://gerrit.ovirt.org/#/c/39864/6/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 134: Line 135: if (getParameters().isForceResetEmulatedMachine()) { Line 136: getParameters().getVdsGroup().setDetectEmulatedMachine(true); Line 137: } Line 138: // if ksm-enabled updated or ksm-merge-across-nodes updated this section must be called if and when the command ended successfully. see endSuccesfully() method Line 139: if ((getVdsGroup().isKsmMergeAcrossNumaNodes() != getPrevVdsGroup().isKsmMergeAcrossNumaNodes()) || Line 140: (getVdsGroup().isEnableKsm() != getPrevVdsGroup().isEnableKsm())) { Line 141: MomPolicyUpdateParams eventParams = new MomPolicyUpdateParams(getVdsGroup(), allForVdsGroup); Line 142: https://gerrit.ovirt.org/#/c/39864/6/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 532: this.allHostsInCluster = allHostsInCluster; Line 533: } Line 534: } Line 535: Line 536: public void onMomPolicyChange(@Observes @MomPolicyUpdate final MomPolicyUpdateParams policyParams) { TODO this event handling must be called in a separate thread. I need to make the event to be async with some infra glue code. Line 537: if (policyParams == null || policyParams.cluster == null || policyParams.allHostsInCluster == null) Line 538: return; Line 539: // collect all Active hosts into a callable list Line 540: List<Callable<VDSReturnValue>> callables = new LinkedList<>(); Line 558: if (cluster.getCompatibilityVersion().compareTo(Version.v3_4) >= I'd move this check up to the parent method to spare all iteration and threads https://gerrit.ovirt.org/#/c/39864/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsGroupOperationCommandBase.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsGroupOperationCommandBase.java: Line 52: * Get the cluster object as it is in database before update Line 53: * Line 54: * @return Current cluster object before database update, or null if not existing Line 55: */ Line 56: public VDSGroup getPrevVdsGroup() { shouldn't you justcompare getVdsGroup() vs getParameters().getVdsGropup? Line 57: return super.getVdsGroup(); Line 58: } Line 59: Line 60: @Override https://gerrit.ovirt.org/#/c/39864/6/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/common/qualifiers/MomPolicyUpdate.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/common/qualifiers/MomPolicyUpdate.java: Line 1: package org.ovirt.engine.core.common.qualifiers; Line 2: missing some declerations on the qualifier import javax.inject.Qualifier; import java.lang.annotation.Retention; import java.lang.annotation.Target; import static java.lang.annotation.ElementType.FIELD; import static java.lang.annotation.ElementType.PARAMETER; import static java.lang.annotation.RetentionPolicy.RUNTIME; @Qualifier @Retention(RUNTIME) @Target({ FIELD, PARAMETER }) Line 3: import javax.inject.Qualifier; Line 4: Line 5: @Qualifier Line 6: public @interface MomPolicyUpdate { -- 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: 6 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
