Re: [pnfs] [PATCH 3/5] pnfsd: DLM file layout only support read iomode layouts

[Boaz Harrosh <bharrosh@panasas.com>]

On 02/05/2010 07:10 PM, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>
> 
> In a DLM cluster, writing to a node other than the node where the open call
> occurred and where meta data is cached will have performance implications when
> the write causes meta data changes that need to be propagated to the open call
> node.
> 
> DlM clusters support only LAYOUTIOMODE4_READ layouts. Writes will go through
> the MDS.
> 
> Return NFS4ERR_BADIOMODE for LAYOUTGET requests with LAYOUTIOMODE4_RW iomode.
> 
> Signed-off-by: Andy Adamson <andros@netapp.com>
> ---
>  fs/nfsd/nfs4pnfsd.c   |    2 ++
>  fs/nfsd/nfs4pnfsdlm.c |    5 +++++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
> index 3951e02..e1ecf81 100644
> --- a/fs/nfsd/nfs4pnfsd.c
> +++ b/fs/nfsd/nfs4pnfsd.c
> @@ -881,6 +881,8 @@ nfs4_pnfs_get_layout(struct nfsd4_pnfs_layoutget *lgp,
>  		case -ENOENT:
>  			status = nfserr_stale;
>  			break;
> +		case nfserr_badiomode:
> +			break;

2 questions:

- Why In this patch? - The first patch already fixes up what is allowed
  and what is not. It is more clear to establish protocol in one place
  so we can review.

- Why only this one? Why not all the nfserr_xxx allowed/make-sense, in
  a layoutget operation?

In my opinion we should only allow nfserr_xxx constants and be done with it
It should not be hard to fix all users.

Boaz
>  		default:
>  			status = nfserr_layoutunavailable;
>  		}
> diff --git a/fs/nfsd/nfs4pnfsdlm.c b/fs/nfsd/nfs4pnfsdlm.c
> index 293bf95..8cc7c84 100644
> --- a/fs/nfsd/nfs4pnfsdlm.c
> +++ b/fs/nfsd/nfs4pnfsdlm.c
> @@ -28,6 +28,7 @@
>  #include <linux/nfsd/nfs4layoutxdr.h>
>  
>  #include "nfsfh.h"
> +#include "nfsd.h"
>  
>  #define NFSDDBG_FACILITY                NFSDDBG_PROC
>  
> @@ -331,6 +332,10 @@ static int nfsd4_pnfs_dlm_layoutget(struct inode *inode,
>  
>  	dprintk("%s: LAYOUT_GET\n", __func__);
>  
> +	/* DLM exported file systems only support layouts for READ */
> +	if (res->lg_seg.iomode == IOMODE_RW)
> +		return nfserr_badiomode;
> +
>  	index = dlm_ino_hash(inode);
>  	dprintk("%s first stripe index %d i_ino %lu\n", __func__, index,
>  		inode->i_ino);