Gilad Chaplik has posted comments on this change.

Change subject: core: Collect vm numa node runtime information
......................................................................


Patch Set 9:

(3 comments)

http://gerrit.ovirt.org/#/c/28096/9/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java:

Line 210:             final List<VmNumaNode> vmNumaNodesToUpdate = new 
ArrayList<>();
Line 211:             for(VmStatistics vmStats : _vmStatisticsToSave.values()) {
Line 212:                 
vmNumaNodesToUpdate.addAll(vmStats.getvNumaNodeStatisticsList());
Line 213:             }
Line 214:             if (!vmNumaNodesToUpdate.isEmpty()) {
> new TX on monitoring needs a very good reason. 
Done
Line 215:                 
TransactionSupport.executeInScope(TransactionScopeOption.Required,
Line 216:                         new TransactionMethod<Void>() {
Line 217:                             @Override
Line 218:                             public Void runInTransaction() {


Line 2076: 
Line 2077:     private void updateVmNumaNodeRuntimeInfo(VmStatistics 
statistics, VM vm) {
Line 2078:         if (vm.getRunOnVds() == null) {
Line 2079:             return;
Line 2080:         }
> do we have to save these numa stats on any VM status?
will change line 2078 to check if status is running.
Line 2081:         List<VdsNumaNode> runOnVdsAllNumaNodes = 
getDbFacade().getVdsNumaNodeDAO().getAllVdsNumaNodeByVdsId(vm.getRunOnVds());
Line 2082:         List<VmNumaNode> vmAllNumaNodes = 
getDbFacade().getVmNumaNodeDAO().getAllVmNumaNodeByVmId(vm.getId());
Line 2083:         Map<Integer, VdsNumaNode> runOnVdsAllNumaNodesMap = new 
HashMap<>();
Line 2084:         Map<Integer, VmNumaNode> vmAllNumaNodesMap = new HashMap<>();


Line 2102:                         pair.getSecond().setFirst(false);
Line 2103:                         runTimePinList.add(pair);
Line 2104:                     }
Line 2105:                 }
Line 2106:                 vNode.setVdsNumaNodeList(runTimePinList);
> confusing a bit 
Done
Line 2107:                 if (!vNode.getVdsNumaNodeList().isEmpty()) {
Line 2108:                     vmNumaNodesNeedUpdate.add(vNode);
Line 2109:                 }
Line 2110:             }


-- 
To view, visit http://gerrit.ovirt.org/28096
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9d2c1dd03e0cc2f7d0ed9593e6ff889712894ac7
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Xiaolei Shi <xiao-lei....@hp.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Shahar Havivi <shav...@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

Reply via email to