[linux-cifs-client] Re: SetFileDisposition before or after rename

[Jeff Layton]

On Tue, 23 Sep 2008 13:17:28 -0500
"Steve French" <smfrench at gmail.com> wrote:

> Ok - then I made the change to unconditionally do set delete on close
> even if the rename fails (the target fail may already exist e.g.)
> 

I don't see this delta in your tree yet, but I'm not sure this is a
good idea. Consider:

We go to unlink a file that's open (maybe we intend to create a
directory with the same name). We try to rename the file to cifsXXXXX
and set delete on close. The rename fails, but we set delete on close
anyway.

Now, suppose we return success on the unlink call. We try to create a
directory with the same name and that fails (file didn't get renamed).

Suppose we return failure? Once the file is closed for the last time
then it will eventually be unlinked. This may be what we want, but I
don't think we can make that assumption. The app may want to do
something with the file if it can't be deleted.

The right course of action would seem to be to try to put everything
back like it was and return error, and let the application deal with
it.

Of course, this doesn't take into account Sam's point about races
between clients, but that's virtually impossible to prevent...

 
> Also made a few other changes to that patch e.g. you added a cFYI of
> the return code but FreeXid already does that
> 
> On Tue, Sep 23, 2008 at 1:02 PM, Jeff Layton <jlayton at redhat.com> wrote:
> > On Tue, 23 Sep 2008 12:36:41 -0500
> > "Steve French" <smfrench at gmail.com> wrote:
> >
> >> In http://git.samba.org/?p=jlayton/cifs.git;a=commitdiff;h=6bd5977a4df58b233512b52242d33c3540a3d8f9
> >> you do the SetFileDisposition after the rename - if the
> >> setfiledisposition failed do we even want to do the rename at all?
> >>
> >> Is it worth changing this line:
> >>
> >>         rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
> >> -                        DELETE|FILE_WRITE_ATTRIBUTES,
> >> -                        CREATE_NOT_DIR|CREATE_DELETE_ON_CLOSE,
> >> +                        DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR,
> >>                          &netfid, &oplock, NULL, cifs_sb->local_nls,
> >>                          cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
> >>
> >> We still don't know if setting DELETE_ON_CLOSE on open works in some
> >> cases (and it can't hurt).
> >>
> >
> > JRA seemed pretty convinced that Windows servers always ignore that bit
> > in an open call. In fact, I think he mentioned that he had to fix samba
> > to do the same (maybe some windows clients set it unintentionally?).
> >
> > As far as the ordering of the calls. I tried that first. Once you do a
> > SetFileDisposition, open-file renames no longer work. You have to set
> > the delete on close after renaming...
> >
> > --
> > Jeff Layton <jlayton at redhat.com>
> >
> 
> 
> 


-- 
Jeff Layton <jlayton at redhat.com>