Add support for vTPM for XenServer and XCP-ng 8.3/8.4 #12263
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12263 +/- ##
============================================
- Coverage 17.51% 17.50% -0.01%
Complexity 15585 15585
============================================
Files 5914 5914
Lines 529867 529925 +58
Branches 64722 64733 +11
============================================
+ Hits 92782 92789 +7
- Misses 426635 426685 +50
- Partials 10450 10451 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pearl1594
force-pushed
the
support-vtpm-xen-xcp
branch
from
876e735 to
cc8ea1e
Compare
|
@blueorangutan package |
|
@Pearl1594 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. |
…ort-vtpm-xen-xcp
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16133 |
|
@blueorangutan test ol8 xcpng83 |
|
@Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + xcpng83) has been kicked to run smoke tests |
|
@blueorangutan test ol8 xenserver-84 |
|
@Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + xenserver-84) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-15049)
|
|
[SF] Trillian test result (tid-15050)
|
|
@blueorangutan test ol8 xenserver-84 |
|
@Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + xenserver-84) has been kicked to run smoke tests |
|
@blueorangutan test ol8 xcpng83 |
|
@Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + xcpng83) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-15060)
|
|
[SF] Trillian test result (tid-15059)
|
|
@Pearl1594 |
Pearl1594
changed the title
|
@blueorangutan test |
|
@RosiKyu a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-15162) |
|
@blueorangutan package |
|
@vladimirpetrov a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16446 |
vladimirpetrov
left a comment
vladimirpetrov
left a comment
There was a problem hiding this comment.
LGTM, tested on XCPNG 8.3 and XenServer 8.4 with Windows 11 and Ubuntu 24 server.
|
Here is the vtpm support for the XenServer 8.4 with Ubuntu 24 server VM: Check for TMP in the VM: And Ubuntu 24 VM on XCPNG 8.3: Check for TMP in the VM: The Windows 11 VM was successfully installed on both environments. |
|
@blueorangutan test matrix |
|
@borisstoyanov a [SL] Trillian-Jenkins matrix job (EL8 mgmt + EL8 KVM, Ubuntu22 mgmt + Ubuntu22 KVM, EL8 mgmt + VMware 7.0u3, EL9 mgmt + XCP-ng 8.2 ) has been kicked to run smoke tests |
There was a problem hiding this comment.
Pull request overview
This pull request adds support for virtual TPM (Trusted Platform Module) 2.0 for virtual machines running on XenServer 8.4 and XCP-ng 8.3/8.4. The vTPM feature enables VM security capabilities like Secure Boot and disk encryption by providing a virtual TPM device to guest operating systems.
Changes:
- Adds vTPM configuration option to XenServer hypervisor settings in QueryManagerImpl
- Implements vTPM creation logic via XenServer xe CLI commands in Python plugin script
- Integrates vTPM configuration into VM startup workflow with graceful error handling
- Adds version detection to ensure vTPM is only attempted on compatible XenServer/XCP-ng versions
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/main/java/com/cloud/api/query/QueryManagerImpl.java | Adds VIRTUAL_TPM_ENABLED option to VM detail options for XenServer hypervisor type |
| scripts/vm/hypervisor/xenserver/xenserver84/vmops | Implements create_vtpm Python function to create vTPM devices via xe CLI commands |
| plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStartCommandWrapper.java | Integrates vTPM configuration call into VM start workflow with exception handling |
| plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixReadyCommandWrapper.java | Refactors exception handling to separate UEFI support check from other operations |
| plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java | Adds core vTPM configuration logic including version checking, UEFI validation, and vTPM creation via host plugin |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return false; | ||
| } | ||
| ComparableVersion currentVersion = new ComparableVersion(productVersion); | ||
| ComparableVersion minVersion = new ComparableVersion("8.2.0"); |
There was a problem hiding this comment.
The minimum version check is inconsistent with the PR description and documentation. The PR title and description state support for XenServer 8.4 / XCP-ng 8.3/8.4, and the documentation comment at line 5836 states "XenServer/XCP-ng 8.3 (and above)", but this code checks for version 8.2.0 or higher. This version mismatch could enable vTPM on versions that don't properly support it. The minimum version should be updated to "8.3.0" to match the stated requirements.
| if (platform != null) { | ||
| final String guestRequiresVtpm = platform.get("vtpm"); | ||
| if (guestRequiresVtpm != null && Boolean.parseBoolean(guestRequiresVtpm) && !Boolean.parseBoolean(vtpmEnabled)) { | ||
| logger.warn("Guest OS requires vTPM by default, even if VM details doesn't have the setting: {}", vmSpec.getName()); |
There was a problem hiding this comment.
There is a grammatical error in this log message. "doesn't" should be "don't" because "VM details" is plural (referring to the details map/collection). The correct phrasing should be: "Guest OS requires vTPM by default, even if VM details don't have the setting"
| logger.warn("Guest OS requires vTPM by default, even if VM details doesn't have the setting: {}", vmSpec.getName()); | |
| logger.warn("Guest OS requires vTPM by default, even if VM details don't have the setting: {}", vmSpec.getName()); |
| String bootType = (bootMode == null) ? ApiConstants.BootType.BIOS.toString() : ApiConstants.BootType.UEFI.toString(); | ||
|
|
||
| if (!ApiConstants.BootType.UEFI.toString().equals(bootType)) { |
There was a problem hiding this comment.
The logic for determining boot type is incorrect. If bootMode is null, bootType is set to BIOS, but if bootMode has any non-empty value, bootType is unconditionally set to UEFI. This means any non-null value in the UEFI key will be treated as UEFI mode, regardless of the actual value. The logic should check the value of bootMode to determine if it's truly UEFI or not. Consider checking if the bootMode value equals "UEFI" or similar logic to properly determine the boot type.
| String bootType = (bootMode == null) ? ApiConstants.BootType.BIOS.toString() : ApiConstants.BootType.UEFI.toString(); | |
| if (!ApiConstants.BootType.UEFI.toString().equals(bootType)) { | |
| if (!ApiConstants.BootType.UEFI.toString().equalsIgnoreCase(bootMode)) { |
|
[SF] Trillian Build Failed (tid-15299) |
|
[SF] Trillian test result (tid-15298)
|
|
@Pearl1594 can you please check the comments from Copilot? Some seem to make sense |
8 participants
Description
This PR adds support for vTPM for XenServer 8.4/ XCP-ng 8.3
Where,
VM: i-2-9-VM : Windows 11 - which automatically comes up with vTPM
VM: i-2-19-VM: Ubuntu 24.04 - template was set with virtual.tpm.enabled = true setting. And vTPM was successfully created for the guest VM.
Note:
Tried using VTPM.create api in the java code like:
but failed with:
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
In Progress
How did you try to break this feature and the system with this change?