Copilot commented on code in PR #12062:
URL: https://github.com/apache/cloudstack/pull/12062#discussion_r2522533627
##########
server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java:
##########
@@ -384,13 +384,26 @@ public DeployDestination
planDeployment(VirtualMachineProfile vmProfile, Deploym
boolean considerLastHost = vm.getLastHostId() != null && haVmTag ==
null &&
(considerLastHostStr == null ||
Boolean.TRUE.toString().equalsIgnoreCase(considerLastHostStr));
if (considerLastHost) {
+ logger.debug("This VM has last host_id: {}", vm.getLastHostId());
HostVO host = _hostDao.findById(vm.getLastHostId());
- logger.debug("This VM has last host_id specified, trying to choose
the same host: " + host);
- lastHost = host;
+ if (host == null) {
+ if
(Boolean.TRUE.toString().equalsIgnoreCase(considerLastHostStr)) {
+ throw new CloudRuntimeException("Failed to deploy VM, last
host doesn't exists");
Review Comment:
The error message contains a grammatical error. "doesn't exists" should be
"doesn't exist" (without the 's').
```suggestion
throw new CloudRuntimeException("Failed to deploy VM,
last host doesn't exist");
```
##########
server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java:
##########
@@ -843,7 +843,7 @@ public Long migrate(final HaWorkVO work) {
logger.info(String.format("VM %s is running on a different host
%s, skipping migration", vm, vm.getHostId()));
return null;
}
- logger.info("Migration attempt: for VM " + vm.getUuid() + "from host
id " + srcHostId +
+ logger.info("Migration attempt: for VM " + vm.getUuid() + " from host
id " + srcHostId +
". Starting attempt: " + (1 + work.getTimesTried()) + "/" +
_maxRetries + " times.");
Review Comment:
There are duplicate log messages at lines 836 and 846-847 that log the same
migration attempt information. This appears to be redundant code that should be
consolidated into a single log statement to avoid duplicate log entries.
```suggestion
```
##########
server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java:
##########
@@ -384,13 +384,26 @@ public DeployDestination
planDeployment(VirtualMachineProfile vmProfile, Deploym
boolean considerLastHost = vm.getLastHostId() != null && haVmTag ==
null &&
(considerLastHostStr == null ||
Boolean.TRUE.toString().equalsIgnoreCase(considerLastHostStr));
if (considerLastHost) {
+ logger.debug("This VM has last host_id: {}", vm.getLastHostId());
HostVO host = _hostDao.findById(vm.getLastHostId());
- logger.debug("This VM has last host_id specified, trying to choose
the same host: " + host);
- lastHost = host;
+ if (host == null) {
+ if
(Boolean.TRUE.toString().equalsIgnoreCase(considerLastHostStr)) {
+ throw new CloudRuntimeException("Failed to deploy VM, last
host doesn't exists");
+ }
+ } else {
+ if (host.isInMaintenanceStates()) {
+ if
(Boolean.TRUE.toString().equalsIgnoreCase(considerLastHostStr)) {
+ throw new CloudRuntimeException("Failed to deploy VM,
last host is in maintenance state");
+ }
+ } else {
+ logger.debug("VM's last {}, trying to choose the same
host", host);
Review Comment:
The log message is incomplete or unclear. It appears to be missing text
after "VM's last". Should be something like "VM's last host is {}, trying to
choose the same host" to match the pattern of other similar messages in the
codebase.
```suggestion
logger.debug("VM's last host is {}, trying to choose the
same host", host);
```
--
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]
