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.

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

Add preflight check for hardware-assisted virtualization #644

Merged

Conversation

tserong
Copy link
Contributor

@tserong tserong commented Jan 31, 2024

Problem:
We need to make sure the host we're installing on supports hardware-assisted virtualization.

Solution:
This is pretty quick and dirty, but I hope should not be unreasonable. https://help.ubuntu.com/community/KVM/Installation tells us that we need to run egrep -c '(vmx|svm)' /proc/cpuinfo to check for hardware-assisted virtualization on x86_64 systems, but I think we can short-circuit that.

I reviewed the kvm-ok script (https://bazaar.launchpad.net/~cpu-checker-dev/cpu-checker/trunk/view/head:/kvm-ok) and it first checks for those CPU flags, and then next checks for the existence of /dev/kvm. If both succeed, we're good. But I don't think we actually need the CPU flag check, at least on SUSE distros. I did some testing, and AFAICT if the CPU supports hardware virtualization, the appropriate kvm kernel module (e.g. kvm_intel) is loaded automatically, so /dev/kvm will exist. If not, the module doesn't get loaded and /dev/kvm doesn't exist.

More recent changes to kvm-ok in Debian-based distros (see https://sources.debian.org/patches/cpu-checker/0.7-1.3/) add support for other architectures (aarch64, ppc*, s390x, riscv64), but in all those cases, the only thing that happens is a check for the existence of /dev/kvm.

Related Issue:
harvester/harvester#1154

Test plan:
N/A (should be covered by unit tests in this PR)

@tserong tserong added the enhancement New feature or request label Jan 31, 2024
This is pretty quick and dirty, but I hope should not be unreasonable.
https://help.ubuntu.com/community/KVM/Installation tells us that we need
to run `egrep -c '(vmx|svm)' /proc/cpuinfo` to check for hardware-assisted
virtualization on x86_64 systems, but I think we can short-circuit that.

I reviewed the kvm-ok script (https://bazaar.launchpad.net/~cpu-checker-dev/cpu-checker/trunk/view/head:/kvm-ok)
and it first checks for those CPU flags, and then next checks for the
existence of /dev/kvm.  If both succeed, we're good.  But I don't think
we actually need the CPU flag check, at least on SUSE distros.  I did some
testing, and AFAICT if the CPU supports hardware virtualization, the
appropriate kvm kernel module (e.g. kvm_intel) is loaded automatically,
so /dev/kvm will exist.  If not, the module doesn't get loaded and
/dev/kvm doesn't exist.

More recent changes to kvm-ok in Debian-based distros (see
https://sources.debian.org/patches/cpu-checker/0.7-1.3/) add support
for other architectures (aarch64, ppc*, s390x, riscv64), but in all those
cases, the only thing that happens is a check for the existence of /dev/kvm.

Signed-off-by: Tim Serong <[email protected]>
@tserong tserong force-pushed the wip-hardware-preflight-check-vm-host branch from f94d245 to 740b109 Compare January 31, 2024 07:06
Copy link
Member

@bk201 bk201 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Contributor

@Vicente-Cheng Vicente-Cheng left a comment

Choose a reason for hiding this comment

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

Thanks for the enhancement!

Copy link
Contributor

@ibrokethecloud ibrokethecloud left a comment

Choose a reason for hiding this comment

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

lgtm. thanks.

@tserong tserong merged commit 5e98567 into harvester:master Feb 1, 2024
5 checks passed
@tserong tserong deleted the wip-hardware-preflight-check-vm-host branch February 1, 2024 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants