Copilot commented on code in PR #13914:
URL: https://github.com/apache/cloudstack/pull/13914#discussion_r4193713623
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BlockCommitListener.java:
##########
@@ -55,17 +71,20 @@ public void onEvent(Domain domain, String diskPath,
BlockJobType type, BlockJobS
switch (status) {
case COMPLETED:
result = null;
+ semaphore.release();
Review Comment:
The semaphore is released for any `COMMIT`/`ACTIVE_COMMIT` event on this
domain, without verifying that `diskPath` belongs to the block commit being
awaited. If two VM disks have overlapping block jobs, an event for the other
disk can set this listener's result and permit, so a later missing event for
this disk is still reported as success—the bug this change is intended to
prevent. Pass the expected disk label/path into the listener and ignore events
for other disks before updating `result` or releasing the semaphore.
##########
plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java:
##########
@@ -6696,9 +6697,11 @@ public void
mergeSnapshotIntoBaseFileTestActiveAndDeleteFlags() throws Exception
threadContextMockedStatic.when(() ->
ThreadContext.get(Mockito.anyString())).thenReturn("logid");
-
Mockito.doNothing().when(domainMock).addBlockJobListener(Mockito.any());
+
Mockito.doReturn(blockCommitListenerMock).when(libvirtComputingResourceSpy).getBlockCommitListener(Mockito.any());
+
Mockito.doReturn(null).when(blockCommitListenerMock).getResult(anyInt());
Review Comment:
These stubs force `getResult(...)` to report success, so none of the tests
exercise the regression this PR fixes. A production regression that again
treats a missing Libvirt event as success would therefore still pass. Please
add focused coverage using a real `BlockCommitListener` for both a `COMPLETED`
event and the no-event timeout/error path (and the `READY`/pivot path if
applicable).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
