Repository navigation
Fix block commit wait when using libvirt events - #13914
JoaoJandre wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13914 +/- ##
============================================
+ Coverage 19.65% 20.67% +1.01%
- Complexity 19798 21722 +1924
============================================
Files 6368 6492 +124
Lines 574943 590746 +15803
Branches 70355 72448 +2093
============================================
+ Hits 113015 122108 +9093
- Misses 449658 455364 +5706
- Partials 12270 13274 +1004
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:
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Events are not correlated to the target disk, and the central no-event behavior lacks regression coverage.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Fixes block-commit completion handling by requiring the corresponding Libvirt event before reporting success.
Changes:
- Waits for block-job events within the remaining merge timeout.
- Uses a semaphore to signal terminal block-commit events.
- Updates merge tests for the listener API.
| File | Description |
|---|---|
BlockCommitListener.java |
Adds event waiting and timeout handling. |
LibvirtComputingResource.java |
Passes remaining timeout to the listener. |
LibvirtComputingResourceTest.java |
Updates listener mocks for the new method signature. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@JoaoJandre |
|
|
||
| checkBlockCommitProgress(vm, diskLabel, vmName, snapshotName, topFilePath, baseFilePath); | ||
| remainingTimeout = checkBlockCommitProgress(vm, diskLabel, vmName, snapshotName, topFilePath, baseFilePath); | ||
| mergeResult = blockCommitListener.getResult(remainingTimeout); |
There was a problem hiding this comment.
what happens if the job finishes on the last poll? the wait left is 0 here, so an event that shows up a moment late gets reported as a failed commit
There was a problem hiding this comment.
@Damans227 If I understood your comment correctly, it should be fixed now. Now we get the result after removing the blockCommitListener, so it is impossible for us to get a failed result and for the listener to process the event after.
|
@blueorangutan package |
|
@JoaoJandre 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 19781 |
Description
There is a bug in the VM volume block-commit process that affects the KBOSS backup, volume snapshot, and VM snapshot processes, where if the waiting process finishes without Libvirt throwing an event, ACS assumes the process finished successfully.
This PR fixes this situation, so that the process is only considered successful when the correct event is thrown by Libvirt.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
In the
agent.propertiesfile, I changedlibvirt.events.enabledto true andqcow2.delta.merge.timeoutto 1.I took a (non-incremental) volume snapshot of a VM and while the snapshot was being copied to the secondary storage, I wrote 1GB of data into the volume, so that the block commit would not be instantaneous.