GutoVeronezi commented on code in PR #8234:
URL: https://github.com/apache/cloudstack/pull/8234#discussion_r1493885250


##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -1892,7 +1925,11 @@ public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) 
throws ResourceUnavailableEx
         }
         CallContext.current().setEventDetails("Vm Id: " + vm.getUuid());
 
-        boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, 
cmd.getDetails());
+        Map<String, String> cmdDetails = cmd.getDetails();
+
+        updateInstanceDetails(cmdDetails, vm, newServiceOfferingId);
+
+        boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, 
cmdDetails);

Review Comment:
   > Who has tested it, except the author?
   
   I tested it; I do not know if anybody else has tested it too.
   
   > please ignore my question regarding the vm details, They are not saved in 
database before upgrade, so no need to restore.
   
   Yes. The only thing this PR does is facilitate calling the API. For 
instance, without this PR, if one has a VM with 1 vCPU and 1 GB RAM and wants 
to scale it to 2 GB RAM, one has to call the API with both the current vCPU 
count and the new RAM size. With this PR, one will only have to pass the new 
RAM size in the parameters; ACS will retrieve the current VM's vCPU count and 
will add it to the `details` map to be updated; thus, passing the current vCPU 
count and the new RAM size will have the same behavior as just passing the new 
RAM size. It just updates the `details` map (if necessary) before scaling the 
VM. This applies to RAM, frequency and vCPU.
   
   I created PR #8673 to improve the methods names and docs. 



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