-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[KVM] Fix VM migration error due to VNC password on libvirt limiting versions #6404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@blueorangutan package |
|
@nvazquez a 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. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3466 |
|
@blueorangutan package |
|
@nvazquez a 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. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3468 |
|
@blueorangutan test |
|
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
It works.
|
@leolleeooleo |
@weizhouapache |
|
Yes, was a mistake on the PR description, have updated it - thanks for testing @leolleeooleo. Please submit your review under Files changed -> Review |
weizhouapache
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm
...vm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java
Outdated
Show resolved
Hide resolved
"then upgrade host 1" is right.
|
...vm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java
Outdated
Show resolved
Hide resolved
...vm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java
Outdated
Show resolved
Hide resolved
|
Trillian test result (tid-4228)
|
|
Thanks for the review @leolleeooleo @weizhouapache - I have refactored it to a simpler approach |
|
@blueorangutan package |
1 similar comment
|
@blueorangutan package |
|
@nvazquez a 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. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3471 |
|
@blueorangutan package |
|
@weizhouapache a 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. |
weizhouapache
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 3472 |
|
@blueorangutan test |
|
@weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
weizhouapache
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manually tested OK
DaanHoogland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm
| // Limit the VNC password in case the length is greater than 8 characters | ||
| // Since libvirt version 8 VNC passwords are limited to 8 characters | ||
| String vncPassword = org.apache.commons.lang3.StringUtils.truncate(to.getVncPassword(), 8); | ||
| xmlDesc = replaceIpForVNCInDescFileAndNormalizePassword(xmlDesc, target, vncPassword); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
separate methos?
| // Limit the VNC password in case the length is greater than 8 characters | |
| // Since libvirt version 8 VNC passwords are limited to 8 characters | |
| String vncPassword = org.apache.commons.lang3.StringUtils.truncate(to.getVncPassword(), 8); | |
| xmlDesc = replaceIpForVNCInDescFileAndNormalizePassword(xmlDesc, target, vncPassword); | |
| /** | |
| * Limit the VNC password in case the length is greater than 8 characters | |
| * Since libvirt version 8 VNC passwords are limited to 8 characters | |
| */ | |
| private XmlDescOrSuchAThing getXmlDesc(....) { | |
| String vncPassword = org.apache.commons.lang3.StringUtils.truncate(to.getVncPassword(), 8); | |
| return replaceIpForVNCInDescFileAndNormalizePassword(xmlDesc, target, vncPassword); | |
| } |
|
Trillian test result (tid-4229)
|
…versions (apache#6404) * [KVM] Fix VM migration error due to VNC password on libvirt limiting versions * Fix passwd value * Simplify implementation
* Extract the IO_URING configuration into the agent.properties (apache#6253) When using advanced virtualization the IO Driver is not supported. The admin will decide if want to enable/disable this configuration from agent.properties file. The default value is true * kvm: truncate vnc password to 8 chars (apache#6244) This PR truncates the vnc password of kvm vms to 8 chars to support latest versions of libvirt. * merge fix Signed-off-by: Abhishek Kumar <[email protected]> * [KVM] Enable IOURING only when it is available on the host (apache#6399) * [KVM] Disable IOURING by default on agents * Refactor * Remove agent property for iouring * Restore property * Refactor suse check and enable on ubuntu by default * Refactor irrespective of guest OS * Improvement * Logs and new path * Refactor condition to enable iouring * Improve condition * Refactor property check * Improvement * Doc comment * Extend comment * Move method * Add log * [KVM] Fix VM migration error due to VNC password on libvirt limiting versions (apache#6404) * [KVM] Fix VM migration error due to VNC password on libvirt limiting versions * Fix passwd value * Simplify implementation Co-authored-by: slavkap <[email protected]> Co-authored-by: Wei Zhou <[email protected]> Co-authored-by: Nicolas Vazquez <[email protected]>
…versions (apache#6404) * [KVM] Fix VM migration error due to VNC password on libvirt limiting versions * Fix passwd value * Simplify implementation (cherry picked from commit b1c8b5a) Signed-off-by: Rohit Yadav <[email protected]>
Description
This PR fixes a VM migration issue described on #6402 (comment)
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?