On 10/21/2025 3:39 AM, Michal Prívozník wrote:
On 10/20/25 16:55, Daniel P. Berrangé wrote:
On Mon, Oct 20, 2025 at 03:07:34PM +0200, Michal Prívozník via Devel wrote:
On 10/17/25 18:54, Praveen K Paladugu wrote:
A domain that runs with TCG emulation does not need kvm device, so drop
it from default device ACL.
So if this function is called multiple times then "/dev/kvm" will appear
multiple times on the list. Worse, if the first VM is of KVM type, then
/dev/kvm is added onto the list and the accelerator of the second one is
irrelevant as /dev/kvm will be allowed in its devices CGroup controller.
What you want to do is:
1) remove "/dev/kvm" from the defaultDeviceACL array,
2) audit and fix all users of defaultDeviceACL and let them act
accordingly if domain is of KVM type. For instance,
qemuSetupDevicesCgroup() will need to have something like:
qemuSetupDevicesCgroup(vm)
{
...
qemuCgroupAllowDevicesPaths(vm, deviceACL, VIR_CGROUP_DEVICE_RW, false);
if (vm->def->virtType == VIR_DOMAIN_VIRT_KVM) {
qemuCgroupAllowDevicePath(vm, "/dev/kvm", VIR_CGROUP_DEVICE_RW, false);
}
These 3 lines of code should simply be inside qemuCgroupAllowDevicesPaths,
either before/after the for() loop.
I don't think this is a good idea. I mean, qemuCgroupAllowDevicesPaths()
is basically the same as qemuCgroupAllowDevicePath(), i.e. qemu
specific, but still generic wrapper for allowing a single path in a
CGroup controller, or in this case an array of paths. It's called
whenever two or more devices need to be allowed: see
qemuSetupMemoryDevicesCgroup().
Since allowing /dev/kvm depends on the domain configuration, just like
other devices (say disks, hostdevs, etc.) I see qemuSetupDevicesCgroup()
more suitable.
Make sense Michal. I addressed this in v3 of this patch, which I just sent.
Michal
--
Regards,
Praveen K Paladugu
