DaanHoogland commented on code in PR #7383:
URL: https://github.com/apache/cloudstack/pull/7383#discussion_r1180137609
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -6374,7 +6380,43 @@ public boolean isVMUsingLocalStorage(VMInstanceVO vm) {
@Override
@ActionEvent(eventType = EventTypes.EVENT_VM_MIGRATE, eventDescription =
"migrating VM", async = true)
- public VirtualMachine migrateVirtualMachine(Long vmId, Host
destinationHost) throws ResourceUnavailableException,
ConcurrentOperationException, ManagementServerException,
+ public VirtualMachine migrateVirtualMachine(Long vmId, Host
destinationHost) throws ResourceUnavailableException,
ConcurrentOperationException, ManagementServerException,
VirtualMachineMigrationException {
+ final int retry = MigrateRetry.value();
+ int currentRetry = 0;
+ VirtualMachine migratedVm = null;
+ while (currentRetry < retry) {
+ int num_retries = currentRetry + 1;
+ try {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Trying to migrate the VM, attempt #" +
num_retries + " out of " + retry);
+ }
+ migratedVm = _migrateVirtualMachine(vmId, destinationHost);
+ break; // Successfully migrated the VM
+ } catch (Exception e) {
+ // Failed to migrate the VM, retry after a delay
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Failed to migrate the VM on attempt " +
num_retries + " out of " + retry);
+ }
+ currentRetry++;
+ if (currentRetry < retry) {
+ try {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("Waiting " +
MigrateRetryDelay.value() + " seconds before trying another migration");
+ }
+ Thread.sleep(MigrateRetryDelay.value());
+ } catch (InterruptedException ie) {
+ throw new ManagementServerException("Interrupted while
waiting to retry VM migration", ie);
+ }
+ } else {
+ s_logger.error("Failed to migrate the VM after " + retry +
" attempts");
+ throw e; // Rethrow the exception if all retries have
failed
+ }
+ }
Review Comment:
@benj-n , I think the do-while is certainly a candidat for further
dissection, it is part of a (too) large method, contains nested ifs and a
try-catch in an if.
Also, do you think any tests can be added for (parts of) this code?
--
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]
