Skip to content

improved errorlog message by adding null check #13263

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

Open
saurabhkushwaha438 wants to merge 2 commits into
base: main
Choose a base branch
from
+928 −481
Open

improved errorlog message by adding null check #13263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
93876ee
improved errorlog message by adding null check
saurabhkushwaha438 May 27, 2026
5f54d85
ui: centralize importVMTask null guard in task manager and add warnin…
saurabhkushwaha438 May 27, 2026
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
39 changes: 27 additions & 12 deletions server/src/main/java/org/apache/cloudstack/vm/ImportVmTasksManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ public ListResponse<ImportVMTaskResponse> listImportVMTasks(ListImportVMTasksCmd
Long pageSizeVal = cmd.getPageSizeVal();

ImportVmTask.TaskState state = getStateFromFilter(cmd.getTasksFilter());
Pair<List<ImportVMTaskVO>, Integer> result = importVMTaskDao.listImportVMTasks(zoneId, accountId, vcenter, convertHostId, state, startIndex, pageSizeVal);
Pair<List<ImportVMTaskVO>, Integer> result = importVMTaskDao.listImportVMTasks(zoneId, accountId, vcenter,
convertHostId, state, startIndex, pageSizeVal);
List<ImportVMTaskVO> tasks = result.first();

List<ImportVMTaskResponse> responses = new ArrayList<>();
Expand All @@ -99,14 +100,16 @@ private ImportVmTask.TaskState getStateFromFilter(String tasksFilter) {
try {
return ImportVmTask.TaskState.getValue(tasksFilter);
} catch (IllegalArgumentException e) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Invalid value for task state: %s", tasksFilter));
throw new ServerApiException(ApiErrorCode.PARAM_ERROR,
String.format("Invalid value for task state: %s", tasksFilter));
}
}

@Override
public ImportVmTask createImportVMTaskRecord(DataCenter zone, Account owner, long userId, String displayName, String vcenter, String datacenterName, String sourceVMName, Host convertHost, Host importHost) {
public ImportVmTask createImportVMTaskRecord(DataCenter zone, Account owner, long userId, String displayName,
String vcenter, String datacenterName, String sourceVMName, Host convertHost, Host importHost) {
logger.debug("Creating import VM task entry for VM: {} for account {} on zone {} " +
"from the vCenter: {} / datacenter: {} / source VM: {}",
"from the vCenter: {} / datacenter: {} / source VM: {}",
sourceVMName, owner.getAccountName(), zone.getName(), displayName, vcenter, datacenterName);
Copy link
Copy Markdown
Author

@saurabhkushwaha438 saurabhkushwaha438 May 28, 2026

Choose a reason for hiding this comment

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

this is not actual change that i made , it is just a formatting to pass CI build because maven build enforces strict checkstyle rules.

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland May 28, 2026

Choose a reason for hiding this comment

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

Suggested change
sourceVMName, owner.getAccountName(), zone.getName(), displayName, vcenter, datacenterName);
sourceVMName, owner.getAccountName(), zone.getName(), vcenter, datacenterName, displayName);

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland May 28, 2026

Choose a reason for hiding this comment

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

don’t worry about it @saurabhkushwaha438 . you can apply the suggestion (mine is better than co-pilot’s ;) ) or just resolve.

ImportVMTaskVO importVMTaskVO = new ImportVMTaskVO(zone.getId(), owner.getAccountId(), userId, displayName,
vcenter, datacenterName, sourceVMName, convertHost.getId(), importHost.getId());
Expand All @@ -115,21 +118,26 @@ public ImportVmTask createImportVMTaskRecord(DataCenter zone, Account owner, lon
}

private String getStepDescription(ImportVMTaskVO importVMTaskVO, Host convertHost, Host importHost,
ImportVMTaskVO.Step step, Date updatedDate) {
ImportVMTaskVO.Step step, Date updatedDate) {
String sourceVMName = importVMTaskVO.getSourceVMName();
String vcenter = importVMTaskVO.getVcenter();
String datacenter = importVMTaskVO.getDatacenter();

StringBuilder stringBuilder = new StringBuilder();
if (Completed == step) {
stringBuilder.append("Completed at ").append(DateUtil.getDateDisplayString(TimeZone.getTimeZone("GMT"), updatedDate));
stringBuilder.append("Completed at ")
.append(DateUtil.getDateDisplayString(TimeZone.getTimeZone("GMT"), updatedDate));
} else {
if (CloningInstance == step) {
stringBuilder.append(String.format("Cloning source instance: %s on vCenter: %s / datacenter: %s", sourceVMName, vcenter, datacenter));
stringBuilder.append(String.format("Cloning source instance: %s on vCenter: %s / datacenter: %s",
sourceVMName, vcenter, datacenter));
} else if (ConvertingInstance == step) {
stringBuilder.append(String.format("Converting the cloned VMware instance to a KVM instance on the host: %s", convertHost.getName()));
stringBuilder
.append(String.format("Converting the cloned VMware instance to a KVM instance on the host: %s",
convertHost.getName()));
} else if (Importing == step) {
stringBuilder.append(String.format("Importing the converted KVM instance on the host: %s", importHost.getName()));
stringBuilder.append(
String.format("Importing the converted KVM instance on the host: %s", importHost.getName()));
} else if (Prepare == step) {
stringBuilder.append("Preparing to convert Vmware instance");
}
Expand All @@ -139,11 +147,12 @@ private String getStepDescription(ImportVMTaskVO importVMTaskVO, Host convertHos

@Override
public void updateImportVMTaskStep(ImportVmTask importVMTask, DataCenter zone, Account owner, Host convertHost,
Host importHost, Long vmId, ImportVmTask.Step step) {
Host importHost, Long vmId, ImportVmTask.Step step) {
ImportVMTaskVO importVMTaskVO = (ImportVMTaskVO) importVMTask;
logger.debug("Updating import VM task entry for VM: {} for account {} on zone {} " +
"from the vCenter: {} / datacenter: {} / source VM: {} to step: {}",
importVMTaskVO.getSourceVMName(), owner.getAccountName(), zone.getName(), importVMTaskVO.getDisplayName(),
"from the vCenter: {} / datacenter: {} / source VM: {} to step: {}",
importVMTaskVO.getSourceVMName(), owner.getAccountName(), zone.getName(),
importVMTaskVO.getDisplayName(),
importVMTaskVO.getVcenter(), importVMTaskVO.getDatacenter(), step);
Comment on lines +155 to 156
Date updatedDate = DateUtil.now();
String description = getStepDescription(importVMTaskVO, convertHost, importHost, step, updatedDate);
Expand All @@ -161,6 +170,12 @@ public void updateImportVMTaskStep(ImportVmTask importVMTask, DataCenter zone, A

@Override
public void updateImportVMTaskErrorState(ImportVmTask importVMTask, ImportVmTask.TaskState state, String errorMsg) {
if (importVMTask == null) {
logger.warn(
"Skipping import VM task error state update because importVMTask is null; state: {}, errorMsg: {}",
state, errorMsg);
return;
}
ImportVMTaskVO importVMTaskVO = (ImportVMTaskVO) importVMTask;
Date updatedDate = DateUtil.now();
importVMTaskVO.setUpdated(updatedDate);
Expand Down
Loading
Loading