Skip to content
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.

Sign up for GitHub

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

Fix guest-agent not starting with HyperV Enlightenments enabled #3592

Merged
merged 1 commit into from
Sep 25, 2024
Merged

Fix guest-agent not starting with HyperV Enlightenments enabled #3592

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions ...-external/patches/systemd/0004-detect-virt-detect-hyperv-enlightened-qemu-as-qemu-n.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
From f42a5b49e95a8deed0b8e6f1bea6679af7e908e4 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <[email protected]>
Date: Fri, 19 Apr 2024 13:25:55 +0200
Subject: [PATCH] detect-virt: detect hyperv-enlightened qemu as qemu, not as
hyperv

CPUID reporting hyperv should be taken with a grain of salt, and we
should prefer other mechanisms then.
Comment on lines +7 to +8
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Correct the grammatical error in the commit message

In the commit message, the word "then" should be "instead" to improve clarity.

Apply this diff to fix the typo:

-CPUID reporting hyperv should be taken with a grain of salt, and we should prefer other mechanisms then.
+CPUID reporting hyperv should be taken with a grain of salt, and we should prefer other mechanisms instead.
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CPUID reporting hyperv should be taken with a grain of salt, and we
should prefer other mechanisms then.
CPUID reporting hyperv should be taken with a grain of salt, and we
should prefer other mechanisms instead.


Fixes: #28001
---
src/basic/virt.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/basic/virt.c b/src/basic/virt.c
index 88357a9..89abb53 100644
--- a/src/basic/virt.c
+++ b/src/basic/virt.c
@@ -446,7 +446,7 @@ static Virtualization detect_vm_zvm(void) {
/* Returns a short identifier for the various VM implementations */
Virtualization detect_vm(void) {
static thread_local Virtualization cached_found = _VIRTUALIZATION_INVALID;
- bool other = false;
+ bool other = false, hyperv = false;
int xen_dom0 = 0;
Virtualization v, dmi;

@@ -503,7 +503,12 @@ Virtualization detect_vm(void) {
v = detect_vm_cpuid();
if (v < 0)
return v;
- if (v == VIRTUALIZATION_VM_OTHER)
+ if (v == VIRTUALIZATION_MICROSOFT)
+ /* QEMU sets the CPUID string to hyperv's, in case it provides hyperv enlightenments. Let's
+ * hence not return Microsoft here but just use the other mechanisms first to make a better
+ * decision. */
+ hyperv = true;
+ else if (v == VIRTUALIZATION_VM_OTHER)
other = true;
else if (v != VIRTUALIZATION_NONE)
goto finish;
@@ -544,8 +549,15 @@ Virtualization detect_vm(void) {
return v;

finish:
- if (v == VIRTUALIZATION_NONE && other)
- v = VIRTUALIZATION_VM_OTHER;
+ /* None of the checks above gave us a clear answer, hence let's now use fallback logic: if hyperv
+ * enlightenments are available but the VMM wasn't recognized as anything yet, it's probably
+ * Microsoft. */
+ if (v == VIRTUALIZATION_NONE) {
+ if (hyperv)
+ v = VIRTUALIZATION_MICROSOFT;
+ else if (other)
+ v = VIRTUALIZATION_VM_OTHER;
+ }
Comment on lines +45 to +56
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Issues Found in Virtualization Detection Logic

  • VIRTUALIZATION_MICROSOFT is assigned outside the fallback logic in buildroot-external/patches/systemd/0004-detect-virt-detect-hyperv-enlightened-qemu-as-qemu-n.patch at lines 45 and 56.
Analysis chain

Confirm the correctness of the updated fallback logic

The updated fallback logic appears to correctly handle scenarios where no virtualization type is conclusively detected. If hyperv is true, it returns VIRTUALIZATION_MICROSOFT, otherwise, it checks for other.

Run the following script to verify that the detection behaves as expected in different virtualization environments:

Please ensure to perform tests in the actual environments to confirm the detection logic works as intended.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify virtualization detection in various environments.

# Test: Check the virtualization type reported by systemd-detect-virt.
# Expected Results: Correct virtualization type is reported for each environment.

# Test environments (simulate or document the environments you want to test):
environments=("QEMU with Hyper-V enlightenments" "Native Hyper-V" "Other VMs" "No virtualization")

# Placeholder for loop to execute in each environment (pseudo-code):
for env in "${environments[@]}"; do
    echo "Testing in environment: $env"
    # Command to run systemd-detect-virt in the specified environment
    # Since we cannot simulate environments here, document the expected behavior
    # e.g., systemd-detect-virt
    # Ensure the output matches the expected virtualization type
done

Length of output: 616

Script:

#!/bin/bash
# Description: Verify the updated fallback logic in virtualization detection.

# Search for assignments to VIRTUALIZATION_MICROSOFT to ensure they are correctly tied to the hyperv condition.
echo "Checking assignments to VIRTUALIZATION_MICROSOFT..."
ast-grep --lang patch --pattern $'if ($_ == "VIRTUALIZATION_NONE") { $$$ if ($_) { $$$ VIRTUALIZATION_MICROSOFT; $$$ } $$$ else if ($_) { $$$ VIRTUALIZATION_VM_OTHER; $$$ } $$$ }' > verification_result.txt

# Display the results
cat verification_result.txt

# Check for any unintended assignments to VIRTUALIZATION_MICROSOFT outside the fallback logic
echo "Ensuring VIRTUALIZATION_MICROSOFT is only assigned within the fallback logic..."
rg 'VIRTUALIZATION_MICROSOFT' | grep -v 'if (v == VIRTUALIZATION_NONE)' > unintended_assignments.txt

# Display unintended assignments if any
cat unintended_assignments.txt

Length of output: 1005


cached_found = v;
log_debug("Found VM virtualization %s", virtualization_to_string(v));