Return-Path: Received: from exprod5og103.obsmtp.com ([64.18.0.145]:52824 "HELO exprod5og103.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751943Ab0JGSlf (ORCPT ); Thu, 7 Oct 2010 14:41:35 -0400 Message-ID: <4CAE145F.5070809@panasas.com> Date: Thu, 07 Oct 2010 14:41:35 -0400 From: Benny Halevy To: andros@netapp.com CC: linux-nfs@vger.kernel.org, Fred Isaman Subject: Re: [PATCH 3/3] pnfs_submit: enforce requested DS only pNFS role References: <1286480230-9418-1-git-send-email-andros@netapp.com> <1286480230-9418-2-git-send-email-andros@netapp.com> <1286480230-9418-3-git-send-email-andros@netapp.com> <4CADFE2E.7050401@panasas.com> <4CAE0C8B.5060204@panasas.com> In-Reply-To: <4CAE0C8B.5060204@panasas.com> Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-nfs-owner@vger.kernel.org List-ID: MIME-Version: 1.0 On 2010-10-07 14:08, Benny Halevy wrote: > On 2010-10-07 13:06, Benny Halevy wrote: >> On 2010-10-07 15:37, andros@netapp.com wrote: >>> From: Andy Adamson >>> >>> Signed-off-by: Andy Adamson >>> --- >>> fs/nfs/nfs4filelayoutdev.c | 5 ----- >>> fs/nfs/nfs4state.c | 5 +++++ >>> 2 files changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c >>> index e0edf93..1f0ab62 100644 >>> --- a/fs/nfs/nfs4filelayoutdev.c >>> +++ b/fs/nfs/nfs4filelayoutdev.c >>> @@ -183,11 +183,6 @@ nfs4_pnfs_ds_create(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds) >>> goto out_put; >>> } >>> /* >>> - * Mask the (possibly) returned EXCHGID4_FLAG_USE_PNFS_MDS pNFS role >>> - * The is_ds_only_session depends on this. >>> - */ >>> - clp->cl_exchange_flags &= ~EXCHGID4_FLAG_USE_PNFS_MDS; >>> - /* >>> * Set DS lease equal to the MDS lease, renewal is scheduled in >>> * create_session >>> */ >>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c >>> index 91584ad..e2fc175 100644 >>> --- a/fs/nfs/nfs4state.c >>> +++ b/fs/nfs/nfs4state.c >>> @@ -188,6 +188,7 @@ static int nfs4_begin_drain_session(struct nfs_client *clp) >>> int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred) >>> { >>> int status; >>> + u32 req_exchange_flags = clp->cl_exchange_flags; >>> >>> nfs4_begin_drain_session(clp); >>> status = nfs4_proc_exchange_id(clp, cred); >>> @@ -196,6 +197,10 @@ int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred) >>> status = nfs4_proc_create_session(clp); >>> if (status != 0) >>> goto out; >>> + if (is_ds_only_session(req_exchange_flags)) >>> + /* Mask the (possibly) returned MDS and non-pNFS roles */ >> >> This comment does not really add anything substantial that the code doesn't tell you :) >> >>> + clp->cl_exchange_flags &= >>> + ~(EXCHGID4_FLAG_USE_PNFS_MDS | EXCHGID4_FLAG_USE_NON_PNFS); >> >> I'm not why you want to mask out EXCHGID4_FLAG_USE_NON_PNFS. >> If the server is not a DS why not just return an error? > > So Andy convinced me and the spec. says that USE_PNFS_DS | USE_NON_PNFS > is a valid response. > However, if USE_PNFS_DS is unset in the response in this case > we need not create the client and better return an error. > I'll send a patch that does that. So this is what I have in mind: diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index e2fc175..4723c41 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -197,10 +197,14 @@ int nfs41_init_clientid(struct nfs_client *clp, struct rpc_cred *cred) status = nfs4_proc_create_session(clp); if (status != 0) goto out; - if (is_ds_only_session(req_exchange_flags)) - /* Mask the (possibly) returned MDS and non-pNFS roles */ + if (is_ds_only_session(req_exchange_flags)) { clp->cl_exchange_flags &= ~(EXCHGID4_FLAG_USE_PNFS_MDS | EXCHGID4_FLAG_USE_NON_PNFS); + if (!is_ds_only_session(clp->cl_exchange_flags)) { + nfs4_proc_destroy_session(clp); + status = -ENOTSUPP; + } + } nfs41_setup_state_renewal(clp); nfs_mark_client_ready(clp, NFS_CS_READY); out: > > Benny > >> >> Benny >> >>> nfs41_setup_state_renewal(clp); >>> nfs_mark_client_ready(clp, NFS_CS_READY); >>> out: >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Benny Halevy Software Architect Panasas, Inc. bhalevy@panasas.com Tel/Fax: +972-3-647-8340 Mobile: +972-54-802-8340 Panasas: The Leader in Parallel Storage www.panasas.com