Skip to content

CLOUDSTACK-8599: [VMware] Successful migration was reported as failure when vCenter session timed out#2092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sateesh-chodapuneedi merged 1 commit into from Nov 7, 2017
Merged

CLOUDSTACK-8599: [VMware] Successful migration was reported as failure when vCenter session timed out #2092

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 40 additions & 7 deletions vmware-base/src/com/cloud/hypervisor/vmware/util/VmwareClient.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,6 +37,7 @@
import com.vmware.vim25.InvalidPropertyFaultMsg;
import com.vmware.vim25.LocalizedMethodFault;
import com.vmware.vim25.ManagedObjectReference;
import com.vmware.vim25.MethodFault;
import com.vmware.vim25.ObjectContent;
import com.vmware.vim25.ObjectSpec;
import com.vmware.vim25.ObjectUpdate;
Expand All@@ -46,9 +47,11 @@
import com.vmware.vim25.PropertyFilterSpec;
import com.vmware.vim25.PropertyFilterUpdate;
import com.vmware.vim25.PropertySpec;
import com.vmware.vim25.RequestCanceled;
import com.vmware.vim25.RuntimeFaultFaultMsg;
import com.vmware.vim25.SelectionSpec;
import com.vmware.vim25.ServiceContent;
import com.vmware.vim25.TaskInfo;
import com.vmware.vim25.TaskInfoState;
import com.vmware.vim25.TraversalSpec;
import com.vmware.vim25.UpdateSet;
Expand DownExpand Up@@ -347,16 +350,46 @@ public boolean waitForTask(ManagedObjectReference task) throws InvalidPropertyFa
Object[] result = waitForValues(task, new String[] { "info.state", "info.error" }, new String[] { "state" }, new Object[][] { new Object[] {
TaskInfoState.SUCCESS, TaskInfoState.ERROR } });

if (result[0].equals(TaskInfoState.SUCCESS)) {
retVal = true;
if (result != null && result.length == 2) { //result for 2 properties: info.state, info.error
if (result[0].equals(TaskInfoState.SUCCESS)) {
retVal = true;
}
if (result[1] instanceof LocalizedMethodFault) {
throw new RuntimeException(((LocalizedMethodFault)result[1]).getLocalizedMessage());
}
}
if (result[1] instanceof LocalizedMethodFault) {
throw new RuntimeException(((LocalizedMethodFault) result[1]).getLocalizedMessage());
} catch (WebServiceException we) {
s_logger.warn("Session to vCenter failed with: " + we.getLocalizedMessage());

TaskInfo taskInfo = (TaskInfo)getDynamicProperty(task, "info");
if (!taskInfo.isCancelable()) {
s_logger.warn("vCenter task: " + taskInfo.getName() + "(" + taskInfo.getKey() + ")" + " will continue to run on vCenter because the task cannot be cancelled");
throw new RuntimeException(we.getLocalizedMessage());
}
} catch(WebServiceException we) {
s_logger.debug("Cancelling vCenter task because task failed with " + we.getLocalizedMessage());

s_logger.debug("Cancelling vCenter task: " + taskInfo.getName() + "(" + taskInfo.getKey() + ")");
getService().cancelTask(task);
throw new RuntimeException("vCenter task failed due to " + we.getLocalizedMessage());

// Since task cancellation is asynchronous, wait for the task to be cancelled
Object[] result = waitForValues(task, new String[] {"info.state", "info.error"}, new String[] {"state"},

@yadvryadvrMay 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a check for length of result to avoid array-bound exceptions when accessing result[0], result[1] etc.

@sureshanapartisureshanapartiMay 16, 2017

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhtyd The properties list to filter in the above waitForValues() are "info.state" and "info.error". The result size returned from waitForValues() would be same as the properties list size, which is 2 and so the result Object array would hold two objects: result[0], result[1].

@yadvryadvrMay 16, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sureshanaparti I see that however my comment aimed to encourage defensive programming. Maybe now the code works, but it can still improve upon overall elegance and include additional checks to defend future bugs [1].

[1] https://en.wikipedia.org/wiki/Defensive_programming

@sureshanapartisureshanapartiMay 16, 2017

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhtyd ok. will check and update. Thanks for the review.

@sureshanapartisureshanapartiMay 17, 2017•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhtyd Updated the code. Please take a look. Thanks.

@yadvryadvrMay 17, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

new Object[][] {new Object[] {TaskInfoState.SUCCESS, TaskInfoState.ERROR}});

if (result != null && result.length == 2) { //result for 2 properties: info.state, info.error
if (result[0].equals(TaskInfoState.SUCCESS)) {
s_logger.warn("Failed to cancel vCenter task: " + taskInfo.getName() + "(" + taskInfo.getKey() + ")" + " and the task successfully completed");
retVal = true;
}

if (result[1] instanceof LocalizedMethodFault) {
MethodFault fault = ((LocalizedMethodFault)result[1]).getFault();
if (fault instanceof RequestCanceled) {
s_logger.debug("vCenter task " + taskInfo.getName() + "(" + taskInfo.getKey() + ")" + " was successfully cancelled");
throw new RuntimeException(we.getLocalizedMessage());
}
} else {
throw new RuntimeException(((LocalizedMethodFault)result[1]).getLocalizedMessage());
}
}
}
return retVal;
}
Expand Down