Skip to content

server: fix NPE when migrate vm away with volumes#13131

Open
weizhouapache wants to merge 1 commit into
apache:4.20from
weizhouapache:4.20-fix-vm-migrate-away-custom-offering
Open

server: fix NPE when migrate vm away with volumes#13131
weizhouapache wants to merge 1 commit into
apache:4.20from
weizhouapache:4.20-fix-vm-migrate-away-custom-offering

Conversation

@weizhouapache
Copy link
Copy Markdown
Member

@weizhouapache weizhouapache commented May 8, 2026

Description

This PR fixes #13130

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@weizhouapache weizhouapache changed the title fix 13130 server: fix NPE when migrate vm away with volumes May 8, 2026
@weizhouapache weizhouapache linked an issue May 8, 2026 that may be closed by this pull request
PrepareHostForMaintenance throws NPE when vm has custom restrained service offering #13130
Open
@weizhouapache weizhouapache added this to the 4.20.4 milestone May 8, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 16.26%. Comparing base (a7c2a05) to head (ad56bf7).

Files with missing lines Patch % Lines
...n/java/com/cloud/resource/ResourceManagerImpl.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               4.20   #13131      +/-   ##
============================================
- Coverage     16.27%   16.26%   -0.01%     
+ Complexity    13440    13438       -2     
============================================
  Files          5665     5665              
  Lines        500555   500555              
  Branches      60789    60789              
============================================
- Hits          81445    81438       -7     
- Misses       410004   410012       +8     
+ Partials       9106     9105       -1
Flag Coverage Δ
uitests 4.15% <ø> (ø)
unittests 17.12% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@weizhouapache weizhouapache requested a review from Copilot May 8, 2026 09:10
Copilot AI reviewed May 8, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a NullPointerException during host maintenance VM migration when the VM uses a customized/dynamic compute offering (where service_offering.cpu/ram_size may be NULL) by fetching the VM-specific computed service offering.

Changes:

  • Use the ServiceOfferingDao.findById(vmId, offeringId) overload when migrating a VM away with volumes so dynamic offering values are populated from user_vm_details.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
Comment on lines 1466 to 1468
final DataCenterDeployment plan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), null, null, null);
ServiceOfferingVO offeringVO = serviceOfferingDao.findById(vm.getServiceOfferingId());
ServiceOfferingVO offeringVO = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId());
final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offeringVO, null, null);
sureshanaparti
sureshanaparti approved these changes May 8, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

clgtm

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented May 18, 2026

@blueorangutan package

@weizhouapache weizhouapache marked this pull request as ready for review May 18, 2026 10:44
@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 18, 2026

@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.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 18, 2026

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17893

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented May 18, 2026

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 18, 2026

@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented May 19, 2026

[SF] Trillian test result (tid-16104)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 83936 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13131-t16104-kvm-ol8.zip
Smoke tests completed. 137 look OK, 4 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
ContextSuite context=TestSharedFSLifecycle>:setup Error 0.00 test_sharedfs_lifecycle.py
test_10_attachAndDetach_iso Failure 606.88 test_vm_life_cycle.py
test_01_create_vm_snapshots Failure 603.70 test_vm_snapshots.py
test_02_revert_vm_snapshots Failure 600.60 test_vm_snapshots.py
test_03_delete_vm_snapshots Failure 0.02 test_vm_snapshots.py
test_01_create_volume Failure 608.20 test_volumes.py
test_01_root_volume_encryption Failure 714.98 test_volumes.py
test_02_data_volume_encryption Failure 664.89 test_volumes.py
test_03_root_and_data_volume_encryption Failure 688.15 test_volumes.py
test_02_attach_volume Failure 1263.82 test_volumes.py
test_02_attach_volume Failure 1263.83 test_volumes.py
test_03_download_attached_volume Failure 662.09 test_volumes.py
test_04_delete_attached_volume Failure 660.02 test_volumes.py
test_05_detach_volume Failure 754.24 test_volumes.py
test_06_download_detached_volume Failure 842.40 test_volumes.py
test_07_resize_fail Failure 657.10 test_volumes.py
test_08_resize_volume Failure 662.19 test_volumes.py
test_09_delete_detached_volume Failure 661.27 test_volumes.py
test_10_list_volumes Failure 657.04 test_volumes.py
test_11_attach_volume_with_unstarted_vm Failure 757.60 test_volumes.py
test_12_resize_volume_with_only_size_parameter Failure 661.04 test_volumes.py
test_13_migrate_volume_and_change_offering Failure 790.33 test_volumes.py
test_14_delete_volume_delete_protection Failure 660.15 test_volumes.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@sureshanaparti sureshanaparti sureshanaparti approved these changes

Assignees

No one assigned

Labels

status:needs-testing

Projects

None yet

Milestone

4.20.4

Development

Successfully merging this pull request may close these issues.

PrepareHostForMaintenance throws NPE when vm has custom restrained service offering

4 participants

@weizhouapache @blueorangutan @sureshanaparti