Copilot commented on code in PR #14108:
URL: https://github.com/apache/cloudstack/pull/14108#discussion_r3975854131
##########
server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java:
##########
@@ -768,6 +794,49 @@ void processPlans() {
* @param plan
* the DRS plan to be executed
*/
+ /**
Review Comment:
There are two consecutive Javadoc blocks here; the first describes
`executeDrsPlan(...)` but is no longer attached to that method (because
`destinationViolatesAffinity(...)` was inserted in between). This leaves an
orphaned/misleading doc block in the source.
##########
server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java:
##########
@@ -768,6 +794,49 @@ void processPlans() {
* @param plan
* the DRS plan to be executed
*/
+ /**
+ * Checks a planned migration against the placement rules as they stand
now.
+ *
+ * A plan is generated once and executed later, so state can have moved
on: VMs may have been
+ * created, migrated or destroyed in between. Anti-affinity in particular
is only meaningful
+ * against current placements, and nothing downstream re-checks it -
migrateVirtualMachine does
+ * not enforce affinity groups.
+ *
+ * The processors also cover dedicated resources and DPDK, so a refusal is
not necessarily about
+ * an affinity group.
+ *
+ * @param vm
+ * the VM the plan wants to move
+ * @param destHost
+ * where the plan wants to move it
+ * @param dispatched
+ * migrations already queued by this run, which the database does
not reflect yet
+ * @param dispatchedSourceHosts
+ * hosts those queued migrations have not actually left yet
+ * @return true when the migration should not go ahead
+ */
+ protected boolean destinationViolatesAffinity(VirtualMachine vm, Host
destHost, List<VirtualMachine> dispatched,
+ List<Long> dispatchedSourceHosts) {
+ if (vm.getHostId() == null) {
+ logger.debug("VM {} is no longer running, so its planned migration
is out of date", vm);
+ return true;
+ }
+ if
(CollectionUtils.isEmpty(affinityGroupVMMapDao.listByInstanceId(vm.getId()))) {
+ return false;
+ }
Review Comment:
`destinationViolatesAffinity` returns early when the VM has no affinity
groups, which skips `managementServer.applyAffinityConstraints(...)`. That
method also applies DPDK and dedicated-resource exclusions even when
`vmGroupCount == 0`, so a migration plan for a VM without affinity groups could
now be executed without re-checking those placement rules.
--
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]
