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.
Michal
