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


##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -2870,10 +2870,18 @@ private void updateVmStateForFailedVmCreation(Long 
vmId, Long hostId) {
                         volumeMgr.destroyVolume(volume);
                     }
                 }
-                String subject = String.format("Failed to deploy Instance [ID: 
%s]", vm.getUuid());
+                String subject = String.format("Failed to deploy Instance 
[%s]", vm);
+                String hostDesc;
+                if (host != null) {
+                    hostDesc = String.format(" on host [%s]", host);
+                } else if (hostId != null) {
+                    hostDesc = String.format(" on host [id: %s]", hostId);

Review Comment:
   In updateVmStateForFailedVmCreation, the alert body/subject now supports a 
null hostId, but the method still unconditionally calls 
_hostDao.findById(hostId). In production this can throw 
(GenericDaoBase.findById ultimately calls lockRow with the null ID), so the new 
null-hostId path can fail before sending the alert.



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