[linux-cifs-client] [PATCH] cifs: consolidate reconnect logic in smb_init routines

[Steve French]

On Sat, Aug 15, 2009 at 11:06 AM, Jeff Layton<jlayton at redhat.com> wrote:
> There's a large cut and paste chunk of code in smb_init and
> small_smb_init to handle reconnects. Break it out into a separate
> function, clean it up and have both routines call it.

As can be seen from the excerpt below, your patch does change behavior
(not just do cleanup) - not sure if that is what you intended.

In the old code, for handle based operations, when we have to
reconnect the socket in this path, after coming back from tcon
whether or not there was an error - the rc was converted
to EAGAIN - to signal the caller that he has to reopen the file (if
another reconnect is needed due to tcon failure that
will happen during the reopen).  The new code returns the tcon
error back to the caller for handle based operations (only returns
EAGAIN on handle based operations if tcon succeeds).

This will change behavior on paths when the session drops
and tcon reconnection fails - which may be ok - but needs to be
looked at.

> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 1866bc2..4bdca05 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
...
> +
> +       mark_open_files_invalid(tcon);
> +       rc = CIFSTCon(0, ses, tcon->treeName, tcon, nls_codepage);
> +       up(&ses->sesSem);
> +       cFYI(1, ("reconnect tcon rc = %d", rc));
> +
> +       if (rc)
> +               goto out;
....
> -                               mark_open_files_invalid(tcon);
> -                               rc = CIFSTCon(0, tcon->ses, tcon->treeName,
> -                                             tcon, nls_codepage);
> -                               up(&tcon->ses->sesSem);
> -                               /* BB FIXME add code to check if wsize needs
> -                               update due to negotiated smb buffer size
> -                               shrinking */
> -                               if (rc == 0) {
> -                                       atomic_inc(&tconInfoReconnectCount);
> -                                       /* tell server Unix caps we support */
> -                                       if (tcon->ses->capabilities & CAP_UNIX)
> -                                               reset_cifs_unix_caps(
> -                                               0 /* no xid */,
> -                                               tcon,
> -                                               NULL /* do not know sb */,
> -                                               NULL /* no vol info */);
> -                               }
> -
> -                               cFYI(1, ("reconnect tcon rc = %d", rc));
> -                               /* Removed call to reopen open files here.
> -                                  It is safer (and faster) to reopen files
> -                                  one at a time as needed in read and write */
> -
> -                               /* Check if handle based operation so we
> -                                  know whether we can continue or not without
> -                                  returning to caller to reset file handle */
> -                               switch (smb_command) {
> -                                       case SMB_COM_READ_ANDX:
> -                                       case SMB_COM_WRITE_ANDX:
> -                                       case SMB_COM_CLOSE:
> -                                       case SMB_COM_FIND_CLOSE2:
> -                                       case SMB_COM_LOCKING_ANDX: {
> -                                               unload_nls(nls_codepage);
> -                                               return -EAGAIN;
> -                                       }


-- 
Thanks,

Steve