Subject: checking for rename of hardlinked files on windows - msg#00085

Previous Message by Date: click to view message preview

Re: SetFileDisposition before or after rename

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.) 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@xxxxxxxxxx> wrote: > On Tue, 23 Sep 2008 12:36:41 -0500 > "Steve French" <smfrench@xxxxxxxxx> 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@xxxxxxxxxx> > -- Thanks, Steve

Next Message by Date: click to view message preview

Re: checking for rename of hardlinked files on windows

On Tue, 23 Sep 2008 13:19:48 -0500 "Steve French" <smfrench@xxxxxxxxx> wrote: > in cifs_rename we check: > > if (rc == -EEXIST) { > if (pTcon->unix_ext) { > /* > * Are src and dst hardlinks of same inode? We can > * only tell with unix extensions enabled > > we could also add the same check for Windows (the UniqueId field is > supported by many servers) > Yep. I thought of that too, but figured I was doing enough changes right now :) Certainly would be reasonable in a follow-on patch though... -- Jeff Layton <jlayton@xxxxxxxxxx>

Previous Message by Thread: click to view message preview

Re: jeff patches

comments on patch series follow. Remainder are merged now. On Tue, Sep 23, 2008 at 12:50 PM, Steve French <smfrench@xxxxxxxxx> wrote: 1) cifs: make cifs_rename handle -EACCES errors looks fine but has dependencies 2) cifs: update DOS attributes in cifsInode if we successf ... looks fine but has dependencies 3) cifs: clean up error handling in cifs_unlink 4) cifs: undo changes in cifs_rename_pending_delete if ... I think this isn't right. In any case we should set delete pending even if we can't rename here: rc = CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); - if (rc != 0) - goto out_close; + if (rc != 0) { + rc = -ETXTBSY; + goto undo_setattr; + } /* set DELETE_ON_CLOSE */ rc = CIFSSMBSetFileDisposition(xid, tcon, true, netfid, current->tgid); + if (rc != 0) + goto undo_rename; 5) cifs: add function to set file disposition merged 6) cifs: move rename and delete-on-close logic into helper ... merged 7) cifs: fix busy-file renames and refactor cifs_rename ... Question to resolve still on why forbid cross directory renames? 8) cifs: remove NULL termination from rename target in ... looks fine but waiting for answer from ms 9) cifs: add constants for string lengths of keynames ... fixed up and merged 10) cifs: clean up upcall handling for dns_resolver keys clarify or find similar usage of generic fields being overloaded by your patch x.[0] x.[1] (are they described somewhere?) 11) cifs: no need to use rcu_assign_pointer on immutable ... seems like it doesn't matter 12) cifs: explicitly revoke SPNEGO key after session setup the revoke vs. put - who cleans up question needs to be doublechecked at first glance it does not look like revoke is a superset of put 13) cifs: have find_writeable_file prefer filehandles opene ... merged -- Thanks, Steve

Next Message by Thread: click to view message preview

Re: checking for rename of hardlinked files on windows

On Tue, 23 Sep 2008 13:19:48 -0500 "Steve French" <smfrench@xxxxxxxxx> wrote: > in cifs_rename we check: > > if (rc == -EEXIST) { > if (pTcon->unix_ext) { > /* > * Are src and dst hardlinks of same inode? We can > * only tell with unix extensions enabled > > we could also add the same check for Windows (the UniqueId field is > supported by many servers) > Yep. I thought of that too, but figured I was doing enough changes right now :) Certainly would be reasonable in a follow-on patch though... -- Jeff Layton <jlayton@xxxxxxxxxx>