DaanHoogland commented on code in PR #7383:
URL: https://github.com/apache/cloudstack/pull/7383#discussion_r1378561856
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -6454,21 +6448,53 @@ public VirtualMachine migrateVirtualMachine(Long vmId,
Host destinationHost) thr
throw new InvalidParameterValueException("Cannot migrate VM, host
with id: " + srcHostId + " for VM not found");
}
- DeployDestination dest = null;
- if (destinationHost == null) {
- dest = chooseVmMigrationDestination(vm, srcHost, null);
- } else {
- dest = checkVmMigrationDestination(vm, srcHost, destinationHost);
- }
+ DeployDestination dest;
+ int retries = 1;
+ VirtualMachine migratedVm = null;
- // If no suitable destination found then throw exception
- if (dest == null) {
- throw new CloudRuntimeException("Unable to find suitable
destination to migrate VM " + vm.getInstanceName());
+ do {
+ if (vm.getState() != State.Running) {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("VM is not Running, unable to migrate the
vm " + vm);
+ }
+ InvalidParameterValueException ex = new
InvalidParameterValueException("VM is not Running, unable to migrate the vm
with specified id");
+ ex.addProxyObject(vm.getUuid(), "vmId");
+ throw ex;
+ }
+
+ checkIfHostOfVMIsInPrepareForMaintenanceState(vm.getHostId(),
vmId, "Migrate");
+
+ s_logger.debug(String.format("Trying to migrate the VM, attempt
#%d out of %d" , retries , MigrateRetry.value()));
+
+ dest = (destinationHost == null) ?
chooseVmMigrationDestination(vm, srcHost, null) :
checkVmMigrationDestination(vm, srcHost, destinationHost);
+
+ if (dest != null) {
+ collectVmDiskAndNetworkStatistics(vmId, State.Running);
+ _itMgr.migrate(vm.getUuid(), srcHostId, dest);
+ migratedVm = findMigratedVm(vm.getId(), vm.getType());
+ }
+
+ if (migratedVm == null){
+ s_logger.debug(String.format("Failed to migrate the VM on
attempt %d out of %d.", retries , MigrateRetry.value() ));
+ retries++;
+ try {
+ s_logger.debug(String.format("Waiting %d seconds before
trying another migration." , MigrateRetryDelay.value()));
+ Thread.sleep(MigrateRetryDelay.value());
+ } catch (InterruptedException ignored) {
+ }
+ }
+ } while ( (MigrateRetry.value() != 1) && (retries <=
MigrateRetry.value()) && migratedVm==null);
+
+ if (migratedVm == null){
+ s_logger.error("Failed to migrate the VM after " +
MigrateRetry.value() + " attempts");
+ if(dest == null) { // If no suitable destination hasn't been found
then throw exception
+ throw new CloudRuntimeException("Unable to find suitable
destination to migrate VM " + vm.getInstanceName());
+ } else {
+ throw new CloudRuntimeException("Failed to migrate the VM
after " + MigrateRetry.value() + " attempts");
+ }
}
Review Comment:
this error checking should be a separate method
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -6454,21 +6448,53 @@ public VirtualMachine migrateVirtualMachine(Long vmId,
Host destinationHost) thr
throw new InvalidParameterValueException("Cannot migrate VM, host
with id: " + srcHostId + " for VM not found");
}
- DeployDestination dest = null;
- if (destinationHost == null) {
- dest = chooseVmMigrationDestination(vm, srcHost, null);
- } else {
- dest = checkVmMigrationDestination(vm, srcHost, destinationHost);
- }
+ DeployDestination dest;
+ int retries = 1;
+ VirtualMachine migratedVm = null;
- // If no suitable destination found then throw exception
- if (dest == null) {
- throw new CloudRuntimeException("Unable to find suitable
destination to migrate VM " + vm.getInstanceName());
+ do {
Review Comment:
I think the contents of this do..while() loop should be a separate method.
--
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]