Identify Guest Agent connection error by the ordinal instead of message (KBOSS) - #13789
Identify Guest Agent connection error by the ordinal instead of message (KBOSS)#13789GaOrtiga wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13789 +/- ##
============================================
- Coverage 19.65% 19.65% -0.01%
+ Complexity 19792 19791 -1
============================================
Files 6368 6368
Lines 574881 574882 +1
Branches 70351 70351
============================================
- Hits 112970 112969 -1
- Misses 449639 449641 +2
Partials 12272 12272
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.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Updates Guest Agent connection-failure detection during KBOSS VM validation/backup so the VM isn’t incorrectly marked BackupError when the guest agent is unreachable, by matching on libvirt error identity rather than message text.
Changes:
- Replace libvirt exception message matching with an error “ordinal” comparison in KBOSS VM validation.
- Replace snapshot/backup consistency detection message matching with the same ordinal-based check.
- Introduce a constant for the guest-agent-unresponsive error ordinal.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtValidateKbossVmCommandWrapper.java | Switches Guest Agent failure detection from message matching to ordinal-based comparison. |
| plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java | Adds ordinal constant and uses it to mark VM consistent on guest-agent connection failures during snapshot creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } catch (LibvirtException ex) { | ||
| if (!ex.getMessage().contains(LibvirtComputingResource.AGENT_IS_NOT_CONNECTED)) { | ||
| if (!LibvirtComputingResource.AGENT_UNRESPONSIVE_ERROR_ORDINAL.equals(ex.getError().getCode().ordinal())) { |
There was a problem hiding this comment.
I have to agree with the robot in this. Using the enum is much better then using the ordinal.
| String errorMsg = String.format("Creation of disk-only VM snapshot for VM [%s] failed due to %s.", vmName, e.getMessage()); | ||
| boolean isVmConsistent = false; | ||
| if (e.getMessage().contains(AGENT_IS_NOT_CONNECTED)) { | ||
| if (AGENT_UNRESPONSIVE_ERROR_ORDINAL.equals(e.getError().getCode().ordinal())) { |
Description
When a backup fails because connection with the VM's guest agent failed, the VM remains consistent, therefore, there is no need to place it in the
BackupErrorstate. This validation, however is done by verifying the error message sent by Libvirt, which can be different on older versions. It also does not cover every case of Guest Agent connection failures, as Libvirt's code contains multiple places where this error is thrown, with different messages.This PR changes this validation to use the errors # (ordinal) instead of the message, to avoid these issues.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
I disabled the Guest Agent on the VM and tried creating a backup, and verified that the backup failed but the VM was no placed in the
BackupErrorstate.