weizhouapache commented on code in PR #8234:
URL: https://github.com/apache/cloudstack/pull/8234#discussion_r1493037323
##########
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:
> > Why has tested it, except the author?
>
> I don't think I understood your question.
>
Sorry typo.
Who has tested it, except the author?
> > Do you think it is a regression of this PR we should address?
>
> The PR does not address changes to method `upgradeVirtualMachine`;
therefore, the topic you are questioning would not be a regression of this PR.
>
This PR updates the vm instance details, but not restore the vm details if
upgrade fails. It should be restored if catch an exception or result is false.
In the past, vm details are not updated so not need to restore in case of
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]
