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@xxxxxxxxxx wrote:
>>> From: Andy Adamson <andros@xxxxxxxxxx>
>>>
>>> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxxxxxxx
>> 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Benny Halevy
Software Architect
Panasas, Inc.
bhalevy@xxxxxxxxxxx
Tel/Fax: +972-3-647-8340
Mobile: +972-54-802-8340
Panasas: The Leader in Parallel Storage
www.panasas.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
