Gilad Chaplik has posted comments on this change.

Change subject: core: Adding an option to get a list of partial VDS
......................................................................


Patch Set 11:

(3 comments)

Without knowing weather a field is IN or OUT is very problematic; and who 
decides weather a field should be there or not? and what if we decide to add 
fields from other tables as well... probably this partial will become full 
again someday, after a lot of solved bugs.

maybe we need to skip this phase and go straight to caching, don't know :/

http://gerrit.ovirt.org/#/c/22810/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/SchedulingManager.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/SchedulingManager.java:

Line 375:             List<Guid> vdsWhiteList,
Line 376:             Guid destVdsId,
Line 377:             List<String> messages) {
Line 378:         List<VDS> vdsList = getPartialVdsDAO()
Line 379:                 .getPartialVdsForVdsGroupWithStatus(cluster.getId(), 
VDSStatus.Up);
I think we shouldn't change this code (maybe add a parameter), the underline 
impl should change.
Line 380:         updateInitialHostList(vdsList, vdsBlackList, true);
Line 381:         updateInitialHostList(vdsList, vdsWhiteList, false);
Line 382:         ClusterPolicy policy = 
policyMap.get(cluster.getClusterPolicyId());
Line 383:         Map<String, String> parameters = 
createClusterPolicyParameters(cluster);


http://gerrit.ovirt.org/#/c/22810/11/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/PartialVdsDAODbFacadeImpl.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/PartialVdsDAODbFacadeImpl.java:

Line 183:             entity.setMemAvailable(rs.getLong("mem_available"));
Line 184:             entity.setMemShared(rs.getLong("mem_shared"));
Line 185:             entity.calculateFreeVirtualMemory();
Line 186: 
Line 187:             return entity;
IMO you should somehow use existing mappers, this is bug prone.
Line 188:         }
Line 189:     }
Line 190: 


http://gerrit.ovirt.org/#/c/22810/11/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDAO.java:

Line 181
Line 182
Line 183
Line 184
Line 185
remove from patch


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I20eabaf4531431b0adbaa85966d2cb095c70ec2d
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liran Zelkha <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Liran Zelkha <[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