kubevirt
set scsi as default disk bus on Arm64
What this PR does / why we need it: As SATA is not supported by qemu-kvm in virt-launcher of Arm64, set scsi as default bus for disk. Fix vmi-mutator_test errors
Release note:
none
@rmohr Currently, there are three place to do CPU arch specific modification,
- in the
virt-api webhooks, set DefaultCPUModel, DefaultBootloader and DefaultDisksBus - in the
virt-config/configuration.go, set DefaultOVMFPath, DefaultEmulatedMachines and DefaultMachineType - in the
virt-launcher/virtwrap/converter/converter.go, setAutoattachGraphicsDevice,OS.SMBiosanddomain.Spec.Devices.Controllers
Is it better to manage them in one place? What your opinion?
Currently, there are three place to do CPU arch specific modification,
- in the
virt-api webhooks, set DefaultCPUModel, DefaultBootloader and DefaultDisksBus- in the
virt-config/configuration.go, set DefaultOVMFPath, DefaultEmulatedMachines and DefaultMachineType- in the
virt-launcher/virtwrap/converter/converter.go, setAutoattachGraphicsDevice,OS.SMBiosanddomain.Spec.Devices.ControllersIs it better to manage them in one place? What your opinion?
Maybe. I would have to check the code more closely. It depends on the context I would say :D
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: rmohr
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [rmohr]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
/lgtm cancel
@zhlhahaha looks like the PR needs a rebase. The code does not compile anymore. Should just be a nit.
Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
/lifecycle stale
/remove-lifecycle stale
@zhlhahaha I think we were pretty close with this PR. Still interested in following up?
/remove-lifecycle stale
@zhlhahaha I think we were pretty close with this PR. Still interested in following up?
Hi @rmohr , I will follow up. As qemu-kvm binary on Arm64 eliminates many devices, this leads to lots of specific changes on Arm64, I am not sure if this is right. How's your idea?
Hi @rmohr , I will follow up. As qemu-kvm binary on Arm64 eliminates many devices, this leads to lots of specific changes on Arm64, I am not sure if this is right. How's your idea?
Hm, I think it simply does not make any sense to set defaults which don't work for arm. I think the only thing missing was, that we overlooked that the mutating webhook set some defaults in a too generic fashion and that it needs to be reworked to set defaults per arch.
Maybe I misunderstand you, but letting each user find out what values to set to successfully start a VM on a specific arch does not sound like a good user experience. I would say that alone justifies the additional complexity in our code base.
These devices do support Arm64, but the qemu-kvm binary in virt-launcher get rid of them when compiling. The qemu-kvm rpm package is from http://mirror.centos.org/centos/8-stream/AppStream/aarch64/os/Packages/qemu-kvm-core-6.2.0-5.module_el8.6.0+1087+b42c8331.aarch64. Is it possible to tell them not remove devices when compiling the qemu-kvm binary?
These devices do support Arm64, but the qemu-kvm binary in virt-launcher get rid of them when compiling. The qemu-kvm rpm package is from
http://mirror.centos.org/centos/8-stream/AppStream/aarch64/os/Packages/qemu-kvm-core-6.2.0-5.module_el8.6.0+1087+b42c8331.aarch64. Is it possible to tell them not remove devices when compiling the qemu-kvm binary?
Oh, so even scsi is just disabled?
These devices do support Arm64, but the qemu-kvm binary in virt-launcher get rid of them when compiling. The qemu-kvm rpm package is from
http://mirror.centos.org/centos/8-stream/AppStream/aarch64/os/Packages/qemu-kvm-core-6.2.0-5.module_el8.6.0+1087+b42c8331.aarch64. Is it possible to tell them not remove devices when compiling the qemu-kvm binary?
@alicefr @andreabolognani FYI
@zhlhahaha can you please provide us which flags you are using to configure the QEMU builds? We could ask to the centos stream team if they can enable them. It will be useful to have a complete list of devices that are disabled and you need in KubeVirt
These devices do support Arm64, but the qemu-kvm binary in virt-launcher get rid of them when compiling. The qemu-kvm rpm package is from
http://mirror.centos.org/centos/8-stream/AppStream/aarch64/os/Packages/qemu-kvm-core-6.2.0-5.module_el8.6.0+1087+b42c8331.aarch64. Is it possible to tell them not remove devices when compiling the qemu-kvm binary?
Are you saying that virtio-scsi is disabled in the aarch64 build? I'd be very surprised if that were the case, and would probably consider it a bug that needs to be addressed.
OTOH, why virtio-scsi instead of virtio-blk? The latter seems to be the more commonly used option for some reason. I'm not sure which one is considered to be the preferred option these days.
If we're changing the default bus type for aarch64, is there any chance we could roll out this change to x86_64 as well and make the default bus type architecture-independent? Both virtio-blk and virtio-scsi should work well across all architectures, including those that KubeVirt doesn't currently support but might in the future (ppc64le).
Maybe it'd be too big a change? Windows in particular would probably not take kindly to it :)
These devices do support Arm64, but the qemu-kvm binary in virt-launcher get rid of them when compiling. The qemu-kvm rpm package is from
http://mirror.centos.org/centos/8-stream/AppStream/aarch64/os/Packages/qemu-kvm-core-6.2.0-5.module_el8.6.0+1087+b42c8331.aarch64. Is it possible to tell them not remove devices when compiling the qemu-kvm binary?Oh, so even scsi is just disabled?
scsi storage device is enabled but usb or sata related devices are disabled.
Here is the list of all devices that are supported by qemu-kvm on Arm64.
/usr/libexec/qemu-kvm -M virt -device help
Controller/Bridge/Hub devices:
name "pci-bridge", bus PCI, desc "Standard PCI Bridge"
name "pci-bridge-seat", bus PCI, desc "Standard PCI Bridge (multiseat)"
name "pcie-pci-bridge", bus PCI
name "pcie-root-port", bus PCI, desc "PCI Express Root Port"
name "usb-host", bus usb-bus
name "usb-hub", bus usb-bus
name "x3130-upstream", bus PCI, desc "TI X3130 Upstream Port of PCI Express Switch"
name "xio3130-downstream", bus PCI, desc "TI X3130 Downstream Port of PCI Express Switch"
USB devices:
name "qemu-xhci", bus PCI
Storage devices:
name "scsi-block", bus SCSI, desc "SCSI block device passthrough"
name "scsi-cd", bus SCSI, desc "virtual SCSI CD-ROM"
name "scsi-disk", bus SCSI, desc "virtual SCSI disk or CD-ROM (legacy)"
name "scsi-generic", bus SCSI, desc "pass through generic scsi device (/dev/sg*)"
name "scsi-hd", bus SCSI, desc "virtual SCSI disk"
name "vhost-user-fs-device", bus virtio-bus
name "vhost-user-fs-pci", bus PCI
name "virtio-blk-device", bus virtio-bus
name "virtio-blk-pci", bus PCI, alias "virtio-blk"
name "virtio-blk-pci-non-transitional", bus PCI
name "virtio-blk-pci-transitional", bus PCI
name "virtio-scsi-device", bus virtio-bus
name "virtio-scsi-pci", bus PCI, alias "virtio-scsi"
name "virtio-scsi-pci-non-transitional", bus PCI
name "virtio-scsi-pci-transitional", bus PCI
Network devices:
name "virtio-net-device", bus virtio-bus
name "virtio-net-pci", bus PCI, alias "virtio-net"
name "virtio-net-pci-non-transitional", bus PCI
name "virtio-net-pci-transitional", bus PCI
Input devices:
name "usb-kbd", bus usb-bus
name "usb-mouse", bus usb-bus
name "usb-tablet", bus usb-bus
name "virtconsole", bus virtio-serial-bus
name "virtio-input-host-device", bus virtio-bus
name "virtio-input-host-pci", bus PCI, alias "virtio-input-host"
name "virtio-keyboard-device", bus virtio-bus
name "virtio-keyboard-pci", bus PCI, alias "virtio-keyboard"
name "virtio-mouse-device", bus virtio-bus
name "virtio-mouse-pci", bus PCI, alias "virtio-mouse"
name "virtio-serial-device", bus virtio-bus
name "virtio-serial-pci", bus PCI, alias "virtio-serial"
name "virtio-serial-pci-non-transitional", bus PCI
name "virtio-serial-pci-transitional", bus PCI
name "virtio-tablet-device", bus virtio-bus
name "virtio-tablet-pci", bus PCI, alias "virtio-tablet"
name "virtserialport", bus virtio-serial-bus
Display devices:
name "ramfb", bus System, desc "ram framebuffer standalone device"
name "virtio-gpu-device", bus virtio-bus
name "virtio-gpu-pci", bus PCI, alias "virtio-gpu"
Misc devices:
name "pci-testdev", bus PCI, desc "PCI Test Device"
name "vfio-pci", bus PCI, desc "VFIO-based PCI device assignment"
name "vfio-pci-nohotplug", bus PCI, desc "VFIO-based PCI device assignment"
name "vhost-user-vsock-device", bus virtio-bus
name "vhost-user-vsock-pci", bus PCI
name "vhost-user-vsock-pci-non-transitional", bus PCI
name "vhost-vsock-device", bus virtio-bus
name "vhost-vsock-pci", bus PCI
name "vhost-vsock-pci-non-transitional", bus PCI
name "virtio-balloon-device", bus virtio-bus
name "virtio-balloon-pci", bus PCI, alias "virtio-balloon"
name "virtio-balloon-pci-non-transitional", bus PCI
name "virtio-balloon-pci-transitional", bus PCI
name "virtio-rng-device", bus virtio-bus
name "virtio-rng-pci", bus PCI, alias "virtio-rng"
name "virtio-rng-pci-non-transitional", bus PCI
name "virtio-rng-pci-transitional", bus PCI
name "vmcoreinfo"
Uncategorized devices:
name "nvdimm", desc "DIMM memory module"
name "pc-dimm", desc "DIMM memory module"
name "tpm-tis-device", bus System
@zhlhahaha can you please provide us which flags you are using to configure the QEMU builds? We could ask to the centos stream team if they can enable them. It will be useful to have a complete list of devices that are disabled and you need in KubeVirt
I have not verified all devices that supported by kubevirt, but for now, we see following devices is missing. name "ide-hd", bus IDE, desc "virtual IDE disk" name "usb-storage", bus usb-bus name "VGA", bus PCI WatchDog devices
Are you saying that virtio-scsi is disabled in the aarch64 build?
virtio-scsi is enabled in the build, but sata related storage device is disabled.
OTOH, why virtio-scsi instead of virtio-blk?
No specific reason for choosing virtio-scsi. I think they both works well.
If we're changing the default bus type for aarch64, is there any chance we could roll out this change to x86_64 as well and make the default bus type architecture-independent?
Good point.
Hi @rmohr @alicefr @andreabolognani Thanks for your reply.
As you can see the supported devices by qemu-kvm on Arm64 in the https://github.com/kubevirt/kubevirt/pull/7007#issuecomment-1208981433 , compare with the build on x86_64, many devices are missing (like sound devices.) I tried to install the qemu on Arm64 server via rpm from fedore official repository https://mirrors.fedoraproject.org/, the qemu-kvm do support more devices.
Are there any reasons why the centos stream team disable them in the aarch64 build? We may need send request to the team as long as kubevirt enable new devices or we find some other missing devices on Arm64. It may not a good idea.
As you can see the supported devices by qemu-kvm on Arm64 in the #7007 (comment) , compare with the build on x86_64, many devices are missing (like sound devices.) I tried to install the qemu on Arm64 server via rpm from fedore official repository
https://mirrors.fedoraproject.org/, the qemu-kvm do support more devices.
Fedora enables all devices that upstream does, possibly even more, which could lead to undesired effects - see RHBZ#2076224 for an example. CentOS builds are more deliberate in what devices and features are enabled.
Comparing x86_64 and aarch64 is a good starting point for identifying what the CentOS build is lacking, but the fact that a device is enabled on x86_64 does not automatically mean that the same device should be enabled on aarch64 as well.
SATA for example, is commonly used on x86_64 due to its wide availability and its out-of-the-box Windows support, but it has drawbacks in terms of emulation overhead and if you're installing Linux, which on aarch64 you most likely are going to, you will do much better with virtio-blk or virtio-scsi instead. Thinking of a future where ppc64le support is introduced, SATA's not available at all on that architecture as far as I'm aware.
So in the end I think it's going to be unavoidable that, on different architectures, VMs will use similar but not identical hardware.
Are there any reasons why the centos stream team disable them in the aarch64 build? We may need send request to the team as long as kubevirt enable new devices or we find some other missing devices on Arm64. It may not a good idea.
I think it'd be fine to ask for more devices to be enabled in the aarch64 build. We just need to come up with an actual list, and each addition will have to be adequately justified.
As a starting point, however, it should be possible to get basic aarch64 VMs running using the current binary, right? I'm thinking "headless HTTP server" rather than "full-blown GNOME desktop" :)
Thanks for your detailed reply @andreabolognani
Fedora enables all devices that upstream does, possibly even more, which could lead to undesired effects - see RHBZ#2076224 for an example. CentOS builds are more deliberate in what devices and features are enabled.
I see and I think it is make sense.
SATA for example, is commonly used on x86_64 due to its wide availability and its out-of-the-box Windows support, but it has drawbacks in terms of emulation overhead and if you're installing Linux, which on aarch64 you most likely are going to, you will do much better with virtio-blk or virtio-scsi instead. Thinking of a future where ppc64le support is introduced, SATA's not available at all on that architecture as far as I'm aware.
Is it means using virtio-blk or virtio-scsi as default bus would be OK on Arm64? And not necessary to enable SATA?
So in the end I think it's going to be unavoidable that, on different architectures, VMs will use similar but not identical hardware. I think it'd be fine to ask for more devices to be enabled in the aarch64 build. We just need to come up with an actual list, and each addition will have to be adequately justified.
Ok, I may take some time to verify which devices are supposed to be enabled on aarch64 build.
As a starting point, however, it should be possible to get basic aarch64 VMs running using the current binary, right?
Right
Is it means using virtio-blk or virtio-scsi as default bus would be OK on Arm64? And not necessary to enable SATA?
Correct.
I looked at the files in examples/ and most seem to look like
disks:
- disk:
bus: virtio
name: containerdisk
- disk:
bus: virtio
name: cloudinitdisk
So they explicitly choose virtio-blk for disk devices, which makes sense because if you're going to use Linux or one of the BSDs that's going to result in a much better experience. SATA is still better for Windows, unless of course you are starting from a custom golden image in which you have already installed the virtio drivers.
The interesting part is that the network interface uses virtio-net by default, so the Windows configurations need
interfaces:
- model: e1000
for the same driver-related reasons.
Personally I think it would make sense to use virtio-blk as the default for disks, matching the virtio-net default for network interfaces, on all architectures, guaranteeing a great out-of-the-box experience for Linux VMs with minimal YAML, and requiring Windows VMs to explicitly select SATA for disks like they already do with e1000 for network interfaces. But I'll admit that I'm biased O:-)
@rmohr, do you think there's any chance such a change in default behavior could be made at this point in KubeVirt's development? Or has that ship already sailed for good, and we're stuck having to specify a better device type for one of the categories both for Linux and Windows VMs?
Regardless of the answer to that question, I see no reason not to use virtio-blk as default at least on non-x86 architectures.
I don't feel like I can do a proper review, but this all look roughly right to me :)
@rmohr about the idea of making virtio-blk the default across architectures, do you think there's a chance such a change could ever be accepted? Or are we stuck with SATA on x86_64?
I don't feel like I can do a proper review, but this all look roughly right to me :)
@rmohr about the idea of making virtio-blk the default across architectures, do you think there's a chance such a change could ever be accepted? Or are we stuck with SATA on x86_64?
I think that if we adopt the deprecation policy of k8s (likely and currently discussed), then we would need a v2 of our core api to change defaults. So yes, unlikely to change unless we want to go to v2 where we can change defaults.
I think that if we adopt the deprecation policy of k8s (likely and currently discussed), then we would need a
v2of our core api to change defaults. So yes, unlikely to change unless we want to go tov2where we can change defaults.
From working on libvirt, I am all too familiar with the conundrum :)
Is a list of changes that could be considered for a v2 API maintained anywhere? Something to look at before KubeVirt hits 1.0, perhaps?
@zhlhahaha: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| pull-kubevirt-e2e-k8s-1.21-sig-performance | 21cb2b25bd7dc2bd12d9c89819f0c5adc4220d7a | link | false | /test pull-kubevirt-e2e-k8s-1.21-sig-performance |
| pull-kubevirt-fossa | 64c85191f23b23b90502baea10db72a80e6b0ac4 | link | false | /test pull-kubevirt-fossa |
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.
/retest-required
This bot automatically retries required jobs that failed/flaked on approved PRs.
Silence the bot with an /lgtm cancel or /hold comment for consistent failures.
Is a list of changes that could be considered for a v2 API maintained anywhere? Something to look at before KubeVirt hits 1.0, perhaps?
Now there is one: https://github.com/kubevirt/kubevirt/issues/8360