Copilot commented on code in PR #12793:
URL: https://github.com/apache/cloudstack/pull/12793#discussion_r2919164952
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -9509,6 +9509,13 @@ public UserVm importVM(final DataCenter zone, final Host
host, final VirtualMach
throw new InvalidParameterValueException("Unable to import
virtual machine with invalid host");
}
+ // Ensure template details are loaded so that commitUserVm can
copy them into the VM's details map
+ VMTemplateVO vmTemplateVO = null;
+ if (template != null) {
+ vmTemplateVO = _templateDao.findById(template.getId());
+ _templateDao.loadDetails(vmTemplateVO);
+ }
Review Comment:
`vmTemplateVO` can remain null (when `template == null`), but it’s used
unconditionally in `checkIfDynamicScalingCanBeEnabled(...)`,
`commitUserVm(...)`, and `vmTemplateVO.getFormat()`, which will throw a
NullPointerException. If `template` is required for VM import, fail fast with a
clear exception when it’s null and/or when `_templateDao.findById(...)` returns
null; otherwise, keep using `template` for the ISO check (and avoid passing
null into `commitUserVm`) by deriving an `isIso` boolean safely.
```suggestion
if (template == null) {
throw new InvalidParameterValueException("Unable to import
virtual machine without a template");
}
// Ensure template details are loaded so that commitUserVm can
copy them into the VM's details map
VMTemplateVO vmTemplateVO = null;
vmTemplateVO = _templateDao.findById(template.getId());
if (vmTemplateVO == null) {
throw new InvalidParameterValueException("Unable to find
template with id " + template.getId() + " for virtual machine import");
}
_templateDao.loadDetails(vmTemplateVO);
```
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -9521,10 +9528,10 @@ public UserVm importVM(final DataCenter zone, final
Host host, final VirtualMach
final String uuidName = _uuidMgr.generateUuid(UserVm.class, null);
final Host lastHost = powerState !=
VirtualMachine.PowerState.PowerOn ? host : null;
- final Boolean dynamicScalingEnabled =
checkIfDynamicScalingCanBeEnabled(null, serviceOffering, template,
zone.getId());
- return commitUserVm(true, zone, host, lastHost, template,
hostName, displayName, owner,
+ final Boolean dynamicScalingEnabled =
checkIfDynamicScalingCanBeEnabled(null, serviceOffering, vmTemplateVO,
zone.getId());
+ return commitUserVm(true, zone, host, lastHost, vmTemplateVO,
hostName, displayName, owner,
null, null, userData, null, null, isDisplayVm, keyboard,
- accountId, userId, serviceOffering,
template.getFormat().equals(ImageFormat.ISO), sshPublicKeys, networkNicMap,
+ accountId, userId, serviceOffering,
vmTemplateVO.getFormat().equals(ImageFormat.ISO), sshPublicKeys, networkNicMap,
Review Comment:
`vmTemplateVO` can remain null (when `template == null`), but it’s used
unconditionally in `checkIfDynamicScalingCanBeEnabled(...)`,
`commitUserVm(...)`, and `vmTemplateVO.getFormat()`, which will throw a
NullPointerException. If `template` is required for VM import, fail fast with a
clear exception when it’s null and/or when `_templateDao.findById(...)` returns
null; otherwise, keep using `template` for the ISO check (and avoid passing
null into `commitUserVm`) by deriving an `isIso` boolean safely.
--
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]
