| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
@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. |
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 35.74144% with 169 lines in your changes missing coverage. Please review. @@ 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.
|
Sorry, something went wrong.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17829 |
Sorry, something went wrong.
|
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Sorry, something went wrong.
|
[SF] Trillian test result (tid-16076)
|
Sorry, something went wrong.
There was a problem hiding this comment.
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:
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.
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
There was a problem hiding this comment.
code LGTM. @nvazquez please check if we can add some unit tests here.
Sorry, something went wrong.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17841 |
Sorry, something went wrong.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18334 |
Sorry, something went wrong.
|
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Sorry, something went wrong.
|
[SF] Trillian test result (tid-16395)
|
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18356 |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 18367 |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18370 |
Sorry, something went wrong.
|
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
Sorry, something went wrong.
|
[SF] Trillian test result (tid-16428)
|
Sorry, something went wrong.
|
@nvazquez |
Sorry, something went wrong.
|
@nvazquez (4.22)# patch -p1 <13151.diff patching file core/src/main/java/com/cloud/agent/api/CleanupConvertedInstanceDisksCommand.java patching file plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBaseConvertCommandWrapper.java patching file plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupConvertedInstanceDisksCommandWrapper.java patching file plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtImportConvertedInstanceCommandWrapper.java patching file plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtImportConvertedInstanceCommandWrapperTest.java patching file server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java Hunk #2 succeeded at 1721 (offset 3 lines). Hunk #3 succeeded at 2219 (offset 3 lines). Could you rebase with 4.22 ? I am moving it to 4.22.2 milestone |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM
Executed the full test plan for FR307: VMWare-2-KVM part II - Cleanup after failure - 19/19 test cases Pass (100%).
Summary:
Testing covered cleanup of temporary artifacts (disks and XML descriptors) after failed VMware-to-KVM imports: temporary locations on the primary pool and NFS secondary storage, XML handling (present / absent / corrupt with fallback to prefix listing), forceConvertToPool positive and negative paths, mid-move multi-disk failure (customer repro), destination-pool orphan check, and correct cleanup scoping (concurrent imports and the convert-stage boundary remain untouched). Negative scenarios confirmed no crashes when there is nothing to clean or when cleanup itself fails, with the original error preserved. Failed conversions are correctly reflected in the UI and in listImportVmTasks, and the successful import regression passed with temporary artifacts fully cleaned.
Executed test cases:
Sorry, something went wrong.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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?