Identify Guest Agent connection error by the ordinal instead of message (KBOSS) - #13789
Open
GaOrtiga wants to merge 1 commit into
Open
Identify Guest Agent connection error by the ordinal instead of message (KBOSS)#13789GaOrtiga wants to merge 1 commit into
GaOrtiga 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:
|
Contributor
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())) { |
| public static final String CGROUP_V2 = "cgroup2fs"; | ||
|
|
||
| public static final String AGENT_IS_NOT_CONNECTED = "QEMU guest agent is not connected"; | ||
| public static final Integer AGENT_UNRESPONSIVE_ERROR_ORDINAL = 86; |
| 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())) { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.