lkml.org 
[lkml]   [2012]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [ 131/149] NFSv4.1: Fix layoutcommit error handling
Date

On Apr 1, 2012, at 2:23 PM, Ben Hutchings wrote:

> On Fri, 2012-03-30 at 12:50 -0700, Greg KH wrote:
>> 3.2-stable review patch. If anyone has any objections, please let me know.
>>
>> ------------------
>>
>> From: Trond Myklebust <Trond.Myklebust@netapp.com>
>>
>> commit e59d27e05a6435f8c04d5ad843f37fa795f2eaaa upstream.
>>
>> Firstly, task->tk_status will always return negative error values,
>> so the current tests for 'NFS4ERR_DELEG_REVOKED' etc. are all being
>> ignored.
>> Secondly, clean up the code so that we only need to test
>> task->tk_status once!
>>
>> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>
>> ---
>> fs/nfs/nfs4proc.c | 25 +++++++++++++------------
>> 1 file changed, 13 insertions(+), 12 deletions(-)
>>
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -5983,21 +5983,22 @@ nfs4_layoutcommit_done(struct rpc_task *
>> return;
>>
>> switch (task->tk_status) { /* Just ignore these failures */
>> - case NFS4ERR_DELEG_REVOKED: /* layout was recalled */
>> - case NFS4ERR_BADIOMODE: /* no IOMODE_RW layout for range */
>> - case NFS4ERR_BADLAYOUT: /* no layout */
>> - case NFS4ERR_GRACE: /* loca_recalim always false */
>> + case -NFS4ERR_DELEG_REVOKED: /* layout was recalled */
>> + case -NFS4ERR_BADIOMODE: /* no IOMODE_RW layout for range */
>> + case -NFS4ERR_BADLAYOUT: /* no layout */
>> + case -NFS4ERR_GRACE: /* loca_recalim always false */
>> task->tk_status = 0;
>> - }
>> -
>> - if (nfs4_async_handle_error(task, server, NULL) == -EAGAIN) {
>> - rpc_restart_call_prepare(task);
>> - return;
>> - }
>> -
>> - if (task->tk_status == 0)
>> + break;
>
> It loooks like the previous intent was that for all those specific error
> codes we would end up calling nfs_post_op_update_inode_force_wcc().
> That didn't happen because of the incorrectly signed error codes. But
> it still won't happen now, because of this break. Do we really want to
> break here or fall through past 'case 0'?
>

The 'break' there is deliberate. If the LAYOUTCOMMIT gets an error, then the server will never get to process the post-op GETATTR.

Trond

> Ben.
>
>> + case 0:
>> nfs_post_op_update_inode_force_wcc(data->args.inode,
>> data->res.fattr);
>> + break;
>> + default:
>> + if (nfs4_async_handle_error(task, server, NULL) == -EAGAIN) {
>> + rpc_restart_call_prepare(task);
>> + return;
>> + }
>> + }
>> }
>>
>> static void nfs4_layoutcommit_release(void *calldata)
>
> --
> Ben Hutchings
> I'm always amazed by the number of people who take up solipsism because
> they heard someone else explain it. - E*Borg on alt.fan.pratchett



\
 
 \ /
  Last update: 2012-04-02 02:03    [from the cache]
©2003-2011 Jasper Spaans