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    [W:0.027 / U:0.436 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site