Re: [PATCH 3.12-rc7] KVM: Fix modprobe failure for kvm_intel/kvm_amd
From: Greg KH
Date: Wed Oct 30 2013 - 10:22:07 EST
On Wed, Oct 30, 2013 at 07:31:05PM +0530, Raghavendra K T wrote:
> On 10/30/2013 01:03 AM, Linus Torvalds wrote:
> > On Tue, Oct 29, 2013 at 12:27 PM, Raghavendra K T
> > <raghavendra.kt@xxxxxxxxxxxxxxxxxx> wrote:
> >> Could one solution be cascading actual error
> >> that is lost in fs/debugfs/inode.c:__create_file(), so that we could
> >> take correct action in case of failure of debugfs_create_dir()?
> >> (ugly side is we increase total number of params for __create_file to
> >> 6). or I hope there could be some better solution.
> > The solution to this would be to simply return an error-pointer. See
> > <linux/err.h>. That's what we do for most complex subsystems that
> > return a pointer to a struct: rather than returning "NULL" as an
> > error, return the actual error number encoded in the pointer itself.
> Thank you Linus. Yes, this would have been ideal.
> > But that would require every user of debugfs_create_dir() to be
> > updated to check errors using IS_ERR() instead of checking against
> > NULL, and there's quite a few of them.
> > So I think just making the error be EEXIST is a simpler solution right now.
> I had below patch, and just before sending as formal mail I saw
> Paolo's pull request which already took care of it.
> >From ac5d9f038c273f27bea7a54aab6af79b57f56317 Mon Sep 17 00:00:00 2001
> From: Raghavendra K T <raghavendra.kt@xxxxxxxxxxxxxxxxxx>
> Date: Wed, 30 Oct 2013 18:59:46 +0530
> Subject: [PATCH] Return EEXIST on debugfs_create_dir failure in kvm
> As quoted by Linus, EFAULT means "user passed in an invalid
> virtual address pointer", which is why the error string is Bad address.
> But when a debugfs directory creation fails, the above error is not valid.
> Signed-off-by: Raghavendra K T <raghavendra.kt@xxxxxxxxxxxxxxxxxx>
> I understand that Tim's patch that renames directory to something like
> kvm-pv would solve kvm-amd/kvm-intel modules insertion problem.
> This patch is to address error code change complained by Linus.
> arch/x86/kernel/kvm.c | 2 +-
> virt/kvm/kvm_main.c | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index a0e2a8a..e475fdb 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -622,7 +622,7 @@ static int __init kvm_spinlock_debugfs(void)
> d_kvm = kvm_init_debugfs();
> if (d_kvm == NULL)
> - return -ENOMEM;
> + return -EEXIST;
Why even error out at all here? You should never care about debugfs
file creation return values, so why pass any error back up the stack?
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/