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

[Steve French]

---------- Forwarded message ----------
From: Steve French <smfrench at gmail.com>
Date: Tue, Sep 23, 2008 at 1:48 PM
Subject: Re: [linux-cifs-client] Re: SetFileDisposition before or after rename
To: Jeff Layton <jlayton at redhat.com>


On Tue, Sep 23, 2008 at 1:38 PM, Jeff Layton <jlayton at redhat.com> wrote:
> 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).

This could happen anyway (a race: another client, client 2, getting
between the delete and new create by client 1, and creating the file
first)

> 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>
>



--
Thanks,

Steve



-- 
Thanks,

Steve