Re: [PATCH v3 5/6] KVM: Actually create debugfs in kvm_create_vm()

Sean Christopherson
2022-08-05T19:16:15Z

On Wed, Jul 20, 2022, Oliver Upton wrote:
> From: Oliver Upton <oupton@google.com>
>
> Doing debugfs creation after vm creation leaves things in a
> quasi-initialized state for a while. This is further complicated by the
> fact that we tear down debugfs from kvm_destroy_vm(). Align debugfs and
> stats init/destroy with the vm init/destroy pattern to avoid any
> headaches.
>
> Note the fix for a benign mistake in error handling for calls to
> kvm_arch_create_vm_debugfs() rolled in. Since all implementations of
> the function return 0 unconditionally it isn't actually a bug at
> the moment.

Heh, now I feel like you're being intentionally mean. Usually I discourage
play-by-play descriptions, but in this case being explicit saves readers a lot
of staring.

And I would argue it's still a bug, it's just that the bug can't be hit and so
can't cause issues. E.g.

Opportunistically fix a benign bug where KVM would return "i" instead
of "ret" if kvm_arch_create_vm_debugfs() failed. The bug is benign as
all implementations of the function return 0 unconditionally.

> Lastly, tear down debugfs/stats data in the kvm_create_vm_debugfs()
> error path. Previously it was safe to assume that kvm_destroy_vm() would
> take out the garbage, that is no longer the case.
>
> Signed-off-by: Oliver Upton <oupton@google.com>
> ---

Reviewed-by: Sean Christopherson <seanjc@google.com>