server: copy template details to vm instance details when import VM#12793
server: copy template details to vm instance details when import VM#12793weizhouapache wants to merge 1 commit intoapache:4.22from
Conversation
|
@blueorangutan package |
|
@weizhouapache a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
There was a problem hiding this comment.
Pull request overview
This PR aims to fix VM import so that a template’s details are loaded and then copied into the imported VM’s instance details.
Changes:
- Fetches the template VO from the DAO and explicitly loads its details prior to
commitUserVm. - Switches downstream calls (
checkIfDynamicScalingCanBeEnabled,commitUserVm, ISO format check) to use the DAO-loadedVMTemplateVO.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // 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); | ||
| } |
There was a problem hiding this 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.
| // 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); | |
| } | |
| 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); |
| 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, |
There was a problem hiding this 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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #12793 +/- ##
=========================================
Coverage 17.61% 17.61%
- Complexity 15661 15663 +2
=========================================
Files 5917 5917
Lines 531402 531406 +4
Branches 64971 64972 +1
=========================================
+ Hits 93586 93595 +9
+ Misses 427262 427257 -5
Partials 10554 10554
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17101 |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Description
This PR fixes #12594
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?