[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 7 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.94% 18.94%
- Complexity 18363 18366 +3
=========================================
Files 6192 6195 +3
Lines 556361 556451 +90
Branches 67908 67916 +8
=========================================
+ Hits 105397 105421 +24
- Misses 439393 439455 +62
- Partials 11571 11575 +4
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 |
|
[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 |
|
[SF] Trillian test result (tid-16395)
|
|
@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 18356 |
|
@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 18367 |
|
@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 18370 |
|
@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?