[linux-cifs-client] [PATCH 2/2] cifs: fix busy-file renames and refactor cifs_rename logic

Jeff Layton jlayton at redhat.com
Fri Sep 5 14:41:36 GMT 2008


On Fri, 5 Sep 2008 15:17:05 +0200
Wilhelm Meier <wilhelm.meier at fh-kl.de> wrote:

> Hi Jeff,
> 
> Thanks for your effort!
> Dumb question: where can I get the code to make some further testing 
> e.g. for kernel 2.6.22 or against what versions apply the patches?
> 
> -
> Wilhelm
> 
> Am Freitag, 5. September 2008 schrieb Jeff Layton:
> > On Thu, 4 Sep 2008 23:02:04 -0400
> >
> > Christoph Hellwig <hch at infradead.org> wrote:
> > > On Thu, Sep 04, 2008 at 10:45:26PM -0400, Jeff Layton wrote:
> > > > Break out the code that does the actual renaming into a
> > > > separate function and have cifs_rename call that. That function
> > > > will attempt a path based rename first and then do a filehandle
> > > > based one if it looks like the source is busy.
> > > >
> > > > The existing logic tried a path based rename first, but if we
> > > > needed to remove the destination then it only attempted a
> > > > filehandle based rename afterward. Not all servers support
> > > > renaming by filehandle, so we need to always attempt path
> > > > rename first and fall back to filehandle rename if it doesn't
> > > > work.
> > > >
> > > > The renaming logic also wasn't quite right in some cases. If
> > > > the source and target are hardlinked then it's not really a
> > > > no-op (we still need to unlink the source).
> > >
> > > Actually per posix rename _is_ a no-op if old and new are
> > > hardlinks to same inode.  But this case is completely handled in
> > > the VFS, and filesystems don't need to consider it at all.
> >
> > I guess you mean this quote from the posix spec:
> >
> > "If the old argument and the new argument resolve to the same
> > existing file, rename() shall return successfully and perform no
> > other action."
> >
> > I didn't bother to check the POSIX spec when I was doing this. I
> > just checked what happened on ext3 when you rename one hardlink to
> > an inode on top of another. Oops! If that's the case, I can just
> > make this a no-op after all. I'll plan to respin this patch and
> > repost.
> >
> > Does the rest of the logic here look correct?
> >
> > Thanks,
> 
> 
> 

The patches are against Steve's git tree, but I don't think this code
has changed substantially in a while. It'll probably apply to any
recent kernel...

-- 
Jeff Layton <jlayton at redhat.com>


More information about the linux-cifs-client mailing list