make sure virtual machine params exist#12771
make sure virtual machine params exist#12771DaanHoogland wants to merge 2 commits intoapache:4.22from
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #12771 +/- ##
=========================================
Coverage 17.60% 17.61%
- Complexity 15659 15665 +6
=========================================
Files 5917 5917
Lines 531394 531395 +1
Branches 64970 64967 -3
=========================================
+ Hits 93575 93595 +20
+ Misses 427269 427245 -24
- Partials 10550 10555 +5
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:
|
There was a problem hiding this comment.
Pull request overview
This PR aims to ensure the VM start/deploy parameter map is always initialized (non-null) during UserVmManagerImpl.startVirtualMachine, addressing failures when parameters are expected to exist (notably in password-enabled template flows, per #11941).
Changes:
- Initialize
paramsas a newHashMap<>instead ofnullwhen starting a VM.
Comments suppressed due to low confidence (2)
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:5803
- Initializing
paramsas an emptyHashMapchanges behavior:createParameterInParameterMaponly copiesparameterMap(i.e.,additionalParams) intoparamswhenparams == null. With this change, whenvm.isUpdateParameters()is true,paramswill end up containing only the explicitly added entries (e.g.,VmPassword), and will no longer include otheradditionalParamssuch as UEFI boot options (UefiFlag/BootType/BootMode) orConsiderLastHost, which are expected downstream (e.g., logged/used during start). Consider initializingparamsas a copy ofadditionalParams(or adjustingcreateParameterInParameterMapto always mergeparameterMap) so all start parameters are preserved while still guaranteeingparamsis non-null.
// Set parameters
Map<VirtualMachineProfile.Param, Object> params = new HashMap<>();
if (vm.isUpdateParameters()) {
_vmDao.loadDetails(vm);
String password = getCurrentVmPasswordOrDefineNewPassword(String.valueOf(additionalParams.getOrDefault(VirtualMachineProfile.Param.VmPassword, "")), vm, template);
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:5803
- This change alters how the start parameter map is built; please add/adjust a unit test to assert the returned
paramsis non-null and still includes entries fromadditionalParams(e.g., UEFI/boot mode params orConsiderLastHost) whenvm.isUpdateParameters()is true, to prevent regressions in the start/deploy flow.
// Set parameters
Map<VirtualMachineProfile.Param, Object> params = new HashMap<>();
if (vm.isUpdateParameters()) {
_vmDao.loadDetails(vm);
String password = getCurrentVmPasswordOrDefineNewPassword(String.valueOf(additionalParams.getOrDefault(VirtualMachineProfile.Param.VmPassword, "")), vm, template);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@blueorangutan package |
|
@DaanHoogland 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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 17046 |
|
@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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17051 |
|
@blueorangutan test |
|
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Description
This PR...
Fixes: #11941
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?