On Tue, Nov 13, 2018 at 9:48 AM David Hildenbrand <david@xxxxxxxxxx> wrote:
>
> On 02.11.18 11:10, Okash Khawaja wrote:
> > On Wed, Oct 31, 2018 at 1:17 AM Okash Khawaja <okash.khawaja@xxxxxxxxx> wrote:
> >>
> >> Hi,
> >>
> >> Apologies if this has already been discussed but hasn't crossed my
> >> eyes. It seems that KVM_CREATE_VM ioctl will fail with -ENOMEM if the
> >> call to kvm_create_vm_debugfs() returns negative value. Is debugfs
> >> essential for KVM operation?
> >>
> >> I came across this when testing with a bpf-next kernel I manually
> >> compiled, from around v4.17. Creating a VM from QEMU threw following
> >> error:
> >>
> >> ioctl(KVM_CREATE_VM) failed: 12 Cannot allocate memory
> >>
> >> Running 4.19.0 kernel fixed the problem. Please note that
> >> kvm_create_vm_debugfs() would not fail if debugfs hasn't been
> >> registered at all.
> >>
> >> Thanks,
> >> Okash
> >
> > `kvm_create_vm_debugfs()` currently fails only if it fails to allocate
> > memory so in a way it does make sense that KVM_CREATE_VM ioctl fails
> > with -ENOMEM. However, failing to create VM because debugfs failed
> > doesn't seem optimal to me. For example, let's say we change
> > `kvm_create_vm_debugfs()` to add more error exit conditions which
> > aren't memory allocation related, then still the ioctl will fail with
> > -ENOMEM, unless we change the calling code to check return value of
> > `kvm_create_vm_debugfs()`. Also, if the memory pressure is transient
> > and `kvm_create_vm_debugfs()` fails because of that, the whole VM
> > creation shouldn't fail. But most importantly, since VM debugfs stat
> > isn't necessary for VM to function, VM creation shouldn't fail.
>
> It's okay to ignore corner cases if it makes life easier (e.g. handling
> debugfs created or not when tearing down a VM).
Thank you for getting back :) Actually this is when we are creating a
VM. Basically VM creation fails with -ENOMEM when debugfs creation
fails:
if (kvm_create_vm_debugfs(kvm, r) < 0) {
put_unused_fd(r);
fput(file);
return -ENOMEM;
}
>
> >
> > Please correct me if I'm wrong with my reasoning above. Ultimately, we
> > should check errors in `kvm_create_vm_debugfs()` and clean up files
> > and directories if needed and not fail KVM_CREATE_VM ioctl. If this
> > makes sense, I can put together a patch :)
> >
>
> Is this really worth investing time and effort?
>
> Why I am asking: Your kernel cannot even allocate memory for debugfs.
> How are you supposed to do anything with that VM? Maybe you cannot even
> create a VCPU. Maybe the OOM killer will zap your process. Is there any
> reasonable setup where this would be expected to work?
>
> "memory pressure is transient" is not a convincing argument. This sounds
> like a broken setup to me.
>
> --
>
> Thanks,
>
> David / dhildenb
