{{ message }}
Ensure the detail value is not null when updating the VNC access details of the VMs with failed migrations on Host maintenance#13492
Open
sureshanaparti wants to merge 1 commit into
Conversation
…ils of the vms with failed migrations on host maintenance - Fixes the Constraint error Column 'value' cannot be null
Contributor
Author
|
@blueorangutan package |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a host-maintenance edge case where updating KVM VNC access VM details can attempt to persist a null value, triggering DB constraint failures (e.g., Column 'value' cannot be null). It does so by adding validation around the GetVncPortAnswer before writing kvm.vnc.* details, plus small supporting updates in tests and KVM agent-side logging.
Changes:
- Refactors VNC detail persistence in
ResourceManagerImplinto a helper that validates theGetVncPortAnswerbefore writing VM details. - Updates
ResourceManagerImplTestmocks to reflect the newgetResult()-gated behavior. - Adds an error log in the KVM Libvirt
GetVncPortcommand wrapper when libvirt throws.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java |
Adds guarded VNC detail update logic to avoid persisting null/invalid values from GetVncPortAnswer. |
server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java |
Adjusts test setup to return successful GetVncPortAnswer results under the new logic. |
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVncPortCommandWrapper.java |
Adds logging when libvirt fails to resolve VNC port details for a VM. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1838
to
1851
| if (!vmVncPortAnswer.getResult()) { | ||
| logger.warn("Failed to get VNC port for VM {} on host {}. Details: {}", vm, hostId, vmVncPortAnswer.getDetails()); | ||
| return; | ||
| } | ||
|
|
||
| String vncAddress = vmVncPortAnswer.getAddress(); | ||
| String vncPort = String.valueOf(vmVncPortAnswer.getPort()); | ||
| if (org.apache.commons.lang3.StringUtils.isNotBlank(vmVncPortAnswer.getAddress()) && org.apache.commons.lang3.StringUtils.isNotBlank(vncPort)) { | ||
| logger.info("Setting VNC access details for VM {} on host {} to address: {}, port: {}", vm, hostId, vncAddress, vncPort); | ||
| vmInstanceDetailsDao.addDetail(vm.getId(), VmDetailConstants.KVM_VNC_ADDRESS, vncAddress, true); | ||
| vmInstanceDetailsDao.addDetail(vm.getId(), VmDetailConstants.KVM_VNC_PORT, vncPort, true); | ||
| } else { | ||
| logger.warn("Unable to set VNC access details for VM {} on host {} as the address or port is blank. Address: {}, Port: {}", vm, hostId, vncAddress, vncPort); | ||
| } |
Comment on lines
+44
to
45
| logger.error("Failed to get vnc port for the vm: {} due to {}", command.getName(), e.getMessage()); | ||
| return new GetVncPortAnswer(command, e.toString()); |
Contributor
Author
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
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
This PR ensures the detail value is not null when updating the VNC access details of the VMs with failed migrations on Host maintenance. It fixes the Constraint error "Column 'value' cannot be null" while updating the VNC access details.
Failed error log below (noticed with 4.18).
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?