Check for null host before proceeding with VM volume operations in managed storage while restoring VM#12879
Conversation
…naged storage while restoring VM
|
@blueorangutan package |
|
@sureshanaparti 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. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #12879 +/- ##
============================================
- Coverage 17.61% 17.61% -0.01%
Complexity 15665 15665
============================================
Files 5917 5917
Lines 531461 531464 +3
Branches 64977 64978 +1
============================================
- Hits 93608 93607 -1
- Misses 427295 427298 +3
- Partials 10558 10559 +1
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
Fixes a restore-VM failure mode in managed storage where vm.getLastHostId() points to a deleted host, causing host-dependent volume operations to NPE and leaving an extra ROOT volume behind.
Changes:
- Add a null-host guard in managed-storage handling during VM restore to safely skip host-dependent operations when the host record no longer exists.
- Minor formatting-only updates (whitespace/brace style) across
UserVmManagerImpl.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (Exception ex) { | ||
| throw new CloudRuntimeException("Router start failed due to" + ex); |
There was a problem hiding this comment.
The exception message concatenation is missing a space ("due to" + ex) and the thrown CloudRuntimeException drops the original cause/stack trace. Consider including a separating space and passing the caught exception as the cause so callers/logs preserve details.
| // default findById() won't search entries with removed field not null | ||
| Host host = _hostDao.findById(hostId); | ||
| if (host == null) { | ||
| logger.warn("Host {} not found", hostId); |
There was a problem hiding this comment.
This new log line only prints the hostId; when debugging restore/re-image issues it would be helpful to include the VM and root volume identifiers (e.g., vm UUID/id and root volume UUID/id) in the warning so operators can correlate the skip with the affected resources.
| logger.warn("Host {} not found", hostId); | |
| logger.warn("Host {} not found for vm id: {}, uuid: {}, root volume id: {}, uuid: {}", hostId, vm.getId(), vm.getUuid(), root.getId(), root.getUuid()); |
| if (hostId != null) { | ||
| VolumeInfo volumeInfo = volFactory.getVolume(root.getId()); | ||
| // default findById() won't search entries with removed field not null | ||
| Host host = _hostDao.findById(hostId); | ||
| if (host == null) { | ||
| logger.warn("Host {} not found", hostId); | ||
| return; | ||
| } |
There was a problem hiding this comment.
There are unit tests for restoreVirtualMachine() in server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java, but none appear to cover the managed-storage restore path where vm.getLastHostId() is set and _hostDao.findById(hostId) returns null (deleted host). Adding a test for this scenario would help prevent regressions (e.g., ensure restore proceeds without leaving an extra ROOT volume in Allocated state).
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17228 |
Description
This PR checks for null host before proceeding with VM volume operations in managed storage while restoring VM.
During restore VM, when VM last host id returns null when the Host was deleted, the VM ends up with additional ROOT Volume in
Allocatedstate and the later re-image operation will be failing with validation error: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?