Prevent NPE when removing NIC from a stopped VM using service offering with CPU cap set #12232
Conversation
…g with CPU cap set
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12232 +/- ##
============================================
+ Coverage 16.21% 16.24% +0.02%
- Complexity 13353 13476 +123
============================================
Files 5657 5657
Lines 498647 501942 +3295
Branches 60533 61754 +1221
============================================
+ Hits 80863 81522 +659
- Misses 408771 411364 +2593
- Partials 9013 9056 +43
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 fixes a NullPointerException that occurs when removing a NIC from a stopped VM that uses a service offering with CPU cap set. The fix gracefully handles the scenario where the host is not available instead of throwing an exception.
Key Changes:
- Modified error handling in
setVmQuotaPercentageto log a warning and skip CPU quota calculation when host is unavailable - Updated corresponding test to verify the new graceful degradation behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| server/src/main/java/com/cloud/hypervisor/KVMGuru.java | Changed from throwing CloudRuntimeException to logging warning and returning early when host is null, preventing NPE |
| server/src/test/java/com/cloud/hypervisor/KVMGuruTest.java | Updated test to verify that CPU quota is not set when host is unavailable, rather than expecting an exception |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@blueorangutan package |
|
@Pearl1594 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 16007 |
|
@Pearl1594 , user/reporter has tested are you satisfied as is? |
There was a problem hiding this comment.
Tested the fix:
- Create a compute offering with CPU cap
- Create an Instance and attach multiple isolated networks
- Stop the Instance
- Remove all non-default NICs.
Also performed some lifecycle operation on the above Instance: Start, Stop, Add-Delete more Nics, setup firewall rules and port forwarding.
LGTM.
|
@Pearl1594 can this be merged? |
Co-authored-by: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com>
|
yes @abh1sar - ready for merging. Thanks for testing. |
|
6 participants
Description
This PR fixes: #12225
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?