bhouse-nexthop commented on code in PR #14108:
URL: https://github.com/apache/cloudstack/pull/14108#discussion_r4017476362
##########
api/src/main/java/org/apache/cloudstack/affinity/AffinityProcessorBase.java:
##########
@@ -41,6 +43,23 @@ public void process(VirtualMachineProfile vm, DeploymentPlan
plan, ExcludeList a
}
+
+ /**
+ * Indexes placements supplied by the caller. Callers such as DRS build a
plan of several moves
+ * in memory and persist it only at the end, so during planning the
database still shows the old
+ * host for every VM the plan has already moved.
+ */
+ protected Map<Long, VirtualMachine> getVmIdVmMap(List<VirtualMachine>
vmList) {
Review Comment:
Good catch, done in f0ebe8c.
The two were not quite identical, which makes removing it a small fix rather
than only a tidy-up: `HostAffinityProcessor`'s copy iterates `vmList`
unguarded, so a null list throws, while the base class returns an empty map.
Nothing reaches it with null today - the no-placements callers go through
`getHostIdSet(vmIds)`, which passes `Collections.emptyList()` - but having the
two differ is the kind of thing that stops being harmless once another caller
appears.
Also dropped the now-unused `java.util.HashMap` import. Full `mvn test` on
`api`, `server` and the four affinity processor plugins: 0 failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
