BryanMLima commented on code in PR #9844:
URL: https://github.com/apache/cloudstack/pull/9844#discussion_r1863796892
##########
server/src/main/java/com/cloud/resource/RollingMaintenanceManagerImpl.java:
##########
@@ -666,11 +682,36 @@ private Pair<Boolean, String>
performCapacityChecksBeforeHostInMaintenance(Host
return new Pair<>(true, "OK");
}
+ protected Ternary<Integer, Integer, Integer>
getComputeResourcesCpuSpeedAndRamSize(VMInstanceVO runningVM) {
+ ServiceOfferingVO serviceOffering =
serviceOfferingDao.findById(runningVM.getServiceOfferingId());
+ Integer cpu = serviceOffering.getCpu();
+ Integer speed = serviceOffering.getSpeed();
+ Integer ramSize = serviceOffering.getRamSize();
+ if (serviceOffering.isDynamic()) {
+ List<UserVmDetailVO> vmDetails =
userVmDetailsDao.listDetails(runningVM.getId());
+ if (CollectionUtils.isNotEmpty(vmDetails)) {
+ for (UserVmDetailVO vmDetail : vmDetails) {
+ if (vmDetail.getName() != null && vmDetail.getValue() !=
null) {
+ if (cpu == null &&
VmDetailConstants.CPU_NUMBER.equals(vmDetail.getName())) {
+ cpu = Integer.valueOf(vmDetail.getValue());
+ } else if (speed == null &&
VmDetailConstants.CPU_SPEED.equals(vmDetail.getName())) {
+ speed = Integer.valueOf(vmDetail.getValue());
+ } else if (ramSize == null &&
VmDetailConstants.MEMORY.equals(vmDetail.getName())) {
+ ramSize = Integer.valueOf(vmDetail.getValue());
+ }
+ }
+ }
+ }
+ }
Review Comment:
I agree with @JoaoJandre on this one, inverting the `ifs` makes for a more
readable code, and easier to understand its logic. Could even invert another
if, like the suggestion below.
If these changes were on a code that this patch did not initially create, I
would agree with @rohityadavcloud, but as it has been introduced with this
patch, I think it is better to address this before merging.
```suggestion
if (!serviceOffering.isDynamic()) {
return new Ternary<>(cpu, speed, ramSize);
}
List<UserVmDetailVO> vmDetails =
userVmDetailsDao.listDetails(runningVM.getId());
if (!CollectionUtils.isNotEmpty(vmDetails)) {
return new Ternary<>(cpu, speed, ramSize);
}
for (UserVmDetailVO vmDetail : vmDetails) {
if (vmDetail.getName() == null || vmDetail.getValue() == null) {
continue;
}
if (cpu == null &&
VmDetailConstants.CPU_NUMBER.equals(vmDetail.getName())) {
cpu = Integer.valueOf(vmDetail.getValue());
} else if (speed == null &&
VmDetailConstants.CPU_SPEED.equals(vmDetail.getName())) {
speed = Integer.valueOf(vmDetail.getValue());
} else if (ramSize == null &&
VmDetailConstants.MEMORY.equals(vmDetail.getName())) {
ramSize = Integer.valueOf(vmDetail.getValue());
}
}
```
--
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]
