Fedora kernel list - Apr 2008 - Re: [PATCH] NFS Client mounts hang when exported directory do not exist

Steve Dickson wrote:
> This patch fixes a regression that was introduced by the string based
mounts.
> 
> nfs_mount() statically returns -EACCES for every error returned
> by the remote mounted. This is incorrect because -EACCES is
> an non-fatal error to the mount.nfs command. This error causes
> mount.nfs to retry the mount even in the case when the exported
> directory does not exist. 
I don''t doubt this is the case, but I''d like to see a few real
world
examples (maybe even add them as documentary comments in mount.nfs or in 
the kernel).
> This patch maps the errors returned by the remote mountd into
> valid errno values, exactly how it was done pre-string based 
> mounts. By returning the correct errno enables mount.nfs 
> to do the right thing.  
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
> 
> diff -up linux/fs/nfs/mount_clnt.c.orig linux/fs/nfs/mount_clnt.c
> --- linux/fs/nfs/mount_clnt.c.orig	2008-04-09 08:32:43.000000000 -0400
> +++ linux/fs/nfs/mount_clnt.c	2008-04-11 11:01:39.000000000 -0400
> @@ -21,6 +21,49 @@
>  
>  static struct rpc_program	mnt_program;
>  
> +static struct {
> +	enum nfs_stat stat;
> +	int errnum;
> +} mnt_errtbl[] = {
> +	{ NFS_OK,		0		},
> +	{ NFSERR_PERM,		EPERM		},
> +	{ NFSERR_NOENT,		ENOENT		},
> +	{ NFSERR_IO,		EIO		},
> +	{ NFSERR_NXIO,		ENXIO		},
> +	{ NFSERR_ACCES,		EACCES		},
> +	{ NFSERR_EXIST,		EEXIST		},
> +	{ NFSERR_NODEV,		ENODEV		},
> +	{ NFSERR_NOTDIR,	ENOTDIR		},
> +	{ NFSERR_ISDIR,		EISDIR		},
> +#ifdef NFSERR_INVAL
> +	{ NFSERR_INVAL,		EINVAL		},	/* that Sun forgot */
> +#endif
> +	{ NFSERR_FBIG,		EFBIG		},
> +	{ NFSERR_NOSPC,		ENOSPC		},
> +	{ NFSERR_ROFS,		EROFS		},
> +	{ NFSERR_NAMETOOLONG,	ENAMETOOLONG	},
> +	{ NFSERR_NOTEMPTY,	ENOTEMPTY	},
> +	{ NFSERR_DQUOT,		EDQUOT		},
> +	{ NFSERR_STALE,		ESTALE		},
> +#ifdef EWFLUSH
> +	{ NFSERR_WFLUSH,	EWFLUSH		},
> +#endif
> +	/* Throw in some NFSv3 values for even more fun (HP returns these) */
> +	{ 71,			EREMOTE		},
> +};
> +static int mnt_errtbl_sz = sizeof(mnt_errtbl)/sizeof(mnt_errtbl[0]);
> +
> +static inline int mnt_err_map(int stat)
> +{
> +	int i;
> +
> +	for (i = 0; i < mnt_errtbl_sz; i++) {
> +		if (mnt_errtbl[i].stat == stat)
> +			return -mnt_errtbl[i].errnum;
> +	}
> +	return -EACCES;
> +}
> +
This probably isn''t necessary.  It looks like nfs_stat_to_errno() 
already does everything you want.
>  struct mnt_fhstatus {
>  	u32 status;
>  	struct nfs_fh *fh;
> @@ -98,7 +141,7 @@ out_call_err:
>  
>  out_mnt_err:
>  	dprintk("NFS: MNT server returned result %d\n", result.status);
> -	status = -EACCES;
> +	status = mnt_err_map(result.status);
The question here is whether nfs_mount''s other caller (NFSROOT) can 
handle error codes other than EACCES.
>  	goto out;
>  }

Chuck Lever wrote:
> Steve Dickson wrote:
>> This patch fixes a regression that was introduced by the string based
>> mounts.
>> nfs_mount() statically returns -EACCES for every error returned
>> by the remote mounted. This is incorrect because -EACCES is
>> an non-fatal error to the mount.nfs command. This error causes
>> mount.nfs to retry the mount even in the case when the exported
>> directory does not exist. 
> 
> I don''t doubt this is the case, but I''d like to see a few
real world
> examples (maybe even add them as documentary comments in mount.nfs or in
> the kernel).
Thats easy.. Do a Fedora install over NFS and mistype the mount
point. Instead of returning immediately like it should, one has to
wait 2mins for the error.. 

ENOENT during a mount has always been a fatal error as it should be
and this patch is just restoring that fact.

steved.