cifs: don't attempt busy-file rename unless it's in same-directory

Busy-file renames don't actually work across directories, so we need
to limit this code to renames within the same dir.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/inode.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

Are we certain of this for all server types (NetApp, EMC, Samba etc.)?

On Thu, May 20, 2010 at 12:05 PM, Jeff Layton <jlayton@redhat.com> wrote:
> Busy-file renames don't actually work across directories, so we need
> to limit this code to renames within the same dir.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/inode.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 62b324f..6f0683c 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -1401,6 +1401,10 @@ cifs_do_rename(int xid, struct dentry *from_dentry, const char *fromPath,
>        if (rc == 0 || rc != -ETXTBSY)
>                return rc;
>
> +       /* open-file renames don't work across directories */
> +       if (to_dentry->d_parent != from_dentry->d_parent)
> +               return rc;
> +
>        /* open the file to be renamed -- we need DELETE perms */
>        rc = CIFSSMBOpen(xid, pTcon, fromPath, FILE_OPEN, DELETE,
>                         CREATE_NOT_DIR, &srcfid, &oplock, NULL,
> --
> 1.6.6.1
>
>

On Thu, 20 May 2010 12:15:37 -0500
Steve French <smfrench@gmail.com> wrote:

> Are we certain of this for all server types (NetApp, EMC, Samba etc.)?
> 

This is definitely the case with windows. The open-file rename call
only accepts simple filenames (no paths).

> On Thu, May 20, 2010 at 12:05 PM, Jeff Layton <jlayton@redhat.com> wrote:
> > Busy-file renames don't actually work across directories, so we need
> > to limit this code to renames within the same dir.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/cifs/inode.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> > index 62b324f..6f0683c 100644
> > --- a/fs/cifs/inode.c
> > +++ b/fs/cifs/inode.c
> > @@ -1401,6 +1401,10 @@ cifs_do_rename(int xid, struct dentry *from_dentry, const char *fromPath,
> >        if (rc == 0 || rc != -ETXTBSY)
> >                return rc;
> >
> > +       /* open-file renames don't work across directories */
> > +       if (to_dentry->d_parent != from_dentry->d_parent)
> > +               return rc;
> > +
> >        /* open the file to be renamed -- we need DELETE perms */
> >        rc = CIFSSMBOpen(xid, pTcon, fromPath, FILE_OPEN, DELETE,
> >                         CREATE_NOT_DIR, &srcfid, &oplock, NULL,
> > --
> > 1.6.6.1
> >
> >
> 
> 
> 
> -- 
> Thanks,
> 
> Steve

On Thu, May 20, 2010 at 12:23 PM, Jeff Layton <jlayton@redhat.com> wrote:
> On Thu, 20 May 2010 12:15:37 -0500
> Steve French <smfrench@gmail.com> wrote:
>
>> Are we certain of this for all server types (NetApp, EMC, Samba etc.)?
>>
>
> This is definitely the case with windows. The open-file rename call
> only accepts simple filenames (no paths).

The restriction on rename always seemed to me just a Windows
limitation not a "should" or "must" - we should check what MS-CIFS says

On Fri, 21 May 2010 11:37:25 -0500
Steve French <smfrench@gmail.com> wrote:

> On Thu, May 20, 2010 at 12:23 PM, Jeff Layton <jlayton@redhat.com> wrote:
> > On Thu, 20 May 2010 12:15:37 -0500
> > Steve French <smfrench@gmail.com> wrote:
> >
> >> Are we certain of this for all server types (NetApp, EMC, Samba etc.)?
> >>
> >
> > This is definitely the case with windows. The open-file rename call
> > only accepts simple filenames (no paths).
> 
> The restriction on rename always seemed to me just a Windows
> limitation not a "should" or "must" - we should check what MS-CIFS says
> 
> 
> 

MS-CIFS doesn't cover this. This is a passthrough infolevel. MS-FSCC says this:

---------------[snip]--------------------
RootDirectory (4 bytes): A 32-bit unsigned integer that contains the file handle for the root
   directory. For network operations, this value MUST always be zero.
FileNameLength (4 bytes): A 32-bit unsigned integer that specifies the length, in bytes, of the
   file name contained within the FileName member.
FileName (variable): A sequence of Unicode characters containing the file name. When
   working with this field, use FileNameLength to determine the length of the file name rather
   than assuming the presence of a trailing null delimiter. If the RootDirectory member is 0,
   this member MUST specify a full pathname to be assigned to the file. For network operations,
   this pathname is relative to the root of the share. If the RootDirectory member is not 0, this
   member MUST specify a pathname, relative to RootDirectory, for the new name of the file.
---------------[snip]--------------------

...which makes it look like renaming an open file across directories
ought to work. However...experimentation has shown that this isn't
actually correct. Anything other than a simple filename is rejected by
all of the servers where this seems to matter (win2k3 is where I mainly
tested this, but earlier NT-based servers worked the same way).

This is the reason that the call in cifs_do_rename does this:

                rc = CIFSSMBRenameOpenFile(xid, pTcon, srcfid,
                                (const char *) to_dentry->d_name.name,
                                cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
                                        CIFS_MOUNT_MAP_SPECIAL_CHR);

...note that it uses 'd_name.name' instead of toName. Earlier efforts
to use full paths for the target proved problematic.

Now, we can debate what the spec says but the fact of the matter is
that the proposed patch is quite correct for the code as it exists
today and fixes an actual bug:

    https://bugzilla.redhat.com/show_bug.cgi?id=591938

I suggest that we take the patch as-is to fix that problem. If you want
to do the legwork to try to expand this such that cross-directory
busy-file renames work, then I'll be happy to help review it.