[VMware to KVM] Cleanup leftover migrated volumes in case of migration failures#13151
[VMware to KVM] Cleanup leftover migrated volumes in case of migration failures#13151nvazquez wants to merge 4 commits into
Conversation
nvazquez
requested review from
DaanHoogland,
NuxRo,
Pearl1594,
harikrishna-patnala,
kiranchavala,
sureshanaparti,
vishesh92 and
weizhouapache
|
@blueorangutan package |
|
@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13151 +/- ##
============================================
+ Coverage 18.09% 18.88% +0.79%
- Complexity 16723 18224 +1501
============================================
Files 6037 6177 +140
Lines 542580 555291 +12711
Branches 66427 67779 +1352
============================================
+ Hits 98155 104876 +6721
- Misses 433399 438894 +5495
- Partials 11026 11521 +495
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17829 |
|
@blueorangutan test |
|
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-16076)
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a best-effort cleanup path for temporary converted volumes when VMware→KVM import fails, and refactors KVM conversion wrappers to share common helper logic.
Changes:
- Trigger cleanup of temporary converted disks on import failures/timeouts.
- Introduce
CleanupConvertedInstanceDisksCommandand a corresponding KVM resource wrapper to delete leftover conversion artifacts. - Refactor
LibvirtImportConvertedInstanceCommandWrapperto reuse a new sharedLibvirtBaseConvertCommandWrapper.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java | Sends a new cleanup command to remove temporary converted disks when import fails. |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtImportConvertedInstanceCommandWrapper.java | Refactors wrapper to inherit shared convert/import helpers and updates cleanup call signature. |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupConvertedInstanceDisksCommandWrapper.java | Adds KVM-side handler that locates and deletes temporary conversion disks (and XML when present). |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBaseConvertCommandWrapper.java | Introduces shared helper methods previously embedded in the import wrapper. |
| core/src/main/java/com/cloud/agent/api/CleanupConvertedInstanceDisksCommand.java | Adds agent command to request cleanup of converted disks by store + prefix. |
| core/src/main/java/com/cloud/agent/api/CleanupConvertedInstanceDisksAnswer.java | Adds a new Answer type for the cleanup command (currently empty). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@blueorangutan package |
|
@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
harikrishna-patnala
left a comment
harikrishna-patnala
left a comment
There was a problem hiding this comment.
code LGTM. @nvazquez please check if we can add some unit tests here.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17841 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-16113) |
|
[SF] Trillian test result (tid-16155)
|
|
[SF] Trillian Build Failed (tid-16175) |
|
@blueorangutan test |
|
@RosiKyu a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-16334)
|
|
@blueorangutan package |
|
@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18334 |
|
@blueorangutan test |
|
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Description
This PR includes a cleanup mechanism for migrated volumes which are leftover in case the VMware to KVM migrations have failed unexpectedly.
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?