Copilot commented on code in PR #13131:
URL: https://github.com/apache/cloudstack/pull/13131#discussion_r3207649925


##########
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:
##########
@@ -1464,7 +1464,7 @@ private boolean isClusterWideMigrationPossible(Host host, 
List<VMInstanceVO> vms
      */
     private void migrateAwayVmWithVolumes(HostVO host, VMInstanceVO vm) {
         final DataCenterDeployment plan = new 
DataCenterDeployment(host.getDataCenterId(), host.getPodId(), 
host.getClusterId(), null, null, null);
-        ServiceOfferingVO offeringVO = 
serviceOfferingDao.findById(vm.getServiceOfferingId());
+        ServiceOfferingVO offeringVO = serviceOfferingDao.findById(vm.getId(), 
vm.getServiceOfferingId());
         final VirtualMachineProfile profile = new 
VirtualMachineProfileImpl(vm, null, offeringVO, null, null);

Review Comment:
   This change fixes dynamic/customized service offerings by using the 
vmId-aware DAO method, but there’s no unit test guarding against regressions 
(e.g., customized offering with NULL cpu/ram in service_offering should still 
yield a compute offering with non-null cpu/ram via user_vm_details during 
maintenance migration). Consider adding/adjusting a test in 
ResourceManagerImplTest to cover migrate-away during maintenance with a dynamic 
offering so the NPE can’t reappear.



-- 
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]

Reply via email to