I will commit the patch to the cifs git development tree but wanted to
consider things:
1) your patch changes from open with FILE_WRITE_ATTRIBUTES to open
with GENERIC_WRITE by calling cifs_set_file_info
2) we should probably do the Open then RenameOpenFile whether or not
the cifs_set_file_info fails
3) to remove the extern for a static in the c file, the function
cifs_set_file_info should be moved forward in the file
On Tue, Sep 16, 2008 at 6:35 PM, Steve French <smfrench@xxxxxxxxx> wrote:
>
> I will commit the patch to the cifs git development tree but wanted to consider things:
>
> 1) your patch changes from open with FILE_WRITE_ATTRIBUTES to open with GENERIC_WRITE by calling cifs_set_file_info
> 2) we should probably do the Open then RenameOpenFile whether or not the cifs_set_file_info fails
> 3) to remove the extern for a static in the c file, the function cifs_set_file_info should be moved forward in the file
>
> On Tue, Sep 16, 2008 at 1:05 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>>
>> We already have a cifs_set_file_info function that can flip DOS
>> attribute bits. Have cifs_unlink call it to handle turning ATTR_HIDDEN
>> on and ATTR_READONLY off when an unlink attempt returns -EACCES.
>>
>> This also removes a level of indentation from cifs_unlink.
>>
>> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
>> ---
>> fs/cifs/inode.c | 118 +++++++++++++++++++++---------------------------------
>> 1 files changed, 46 insertions(+), 72 deletions(-)
>>
>> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
>> index 2f75714..783f4ad 100644
>> --- a/fs/cifs/inode.c
>> +++ b/fs/cifs/inode.c
>> @@ -29,6 +29,8 @@
>> #include "cifs_debug.h"
>> #include "cifs_fs_sb.h"
>>
>> +static int cifs_set_file_info(struct inode *inode, struct iattr *attrs,
>> + int xid, char *full_path, __u32 dosattr);
>>
>> static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
>> {
>> @@ -675,7 +677,8 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
>> struct super_block *sb = dir->i_sb;
>> struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
>> struct cifsTconInfo *tcon = cifs_sb->tcon;
>> - FILE_BASIC_INFO *pinfo_buf;
>> + struct iattr *attrs;
>> + __u32 dosattr;
>>
>> cFYI(1, ("cifs_unlink, dir=0x%p, dentry=0x%p", dir, dentry));
>>
>> @@ -728,84 +731,55 @@ psx_del_no_retry:
>> }
>> } else if (rc == -EACCES) {
>> /* try only if r/o attribute set in local lookup data? */
>> - pinfo_buf = kzalloc(sizeof(FILE_BASIC_INFO), GFP_KERNEL);
>> - if (pinfo_buf) {
>> - /* ATTRS set to normal clears r/o bit */
>> - pinfo_buf->Attributes = cpu_to_le32(ATTR_NORMAL);
>> - if (!(tcon->ses->flags & CIFS_SES_NT4))
>> - rc = CIFSSMBSetPathInfo(xid, tcon, full_path,
>> - pinfo_buf,
>> - cifs_sb->local_nls,
>> - cifs_sb->mnt_cifs_flags &
>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>> - else
>> - rc = -EOPNOTSUPP;
>> -
>> - if (rc == -EOPNOTSUPP) {
>> - int oplock = 0;
>> - __u16 netfid;
>> - /* rc = CIFSSMBSetAttrLegacy(xid, tcon,
>> - full_path,
>> - (__u16)ATTR_NORMAL,
>> - cifs_sb->local_nls);
>> - For some strange reason it seems that NT4 eats the
>> - old setattr call without actually setting the
>> - attributes so on to the third attempted workaround
>> - */
>> -
>> - /* BB could scan to see if we already have it open
>> - and pass in pid of opener to function */
>> - rc = CIFSSMBOpen(xid, tcon, full_path,
>> - FILE_OPEN, SYNCHRONIZE |
>> - FILE_WRITE_ATTRIBUTES, 0,
>> - &netfid, &oplock, NULL,
>> - cifs_sb->local_nls,
>> - cifs_sb->mnt_cifs_flags &
>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>> - if (rc == 0) {
>> - rc = CIFSSMBSetFileInfo(xid, tcon,
>> - pinfo_buf,
>> - netfid,
>> - current->tgid);
>> - CIFSSMBClose(xid, tcon, netfid);
>> - }
>> - }
>> - kfree(pinfo_buf);
>> + attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
>> + if (attrs == NULL) {
>> + rc = -ENOMEM;
>> + goto out_reval;
>> }
>> +
>> + /* try to reset dos attributes */
>> + cifsInode = CIFS_I(inode);
>> + dosattr = cifsInode->cifsAttrs & ~ATTR_READONLY;
>> + if (dosattr == 0)
>> + dosattr |= ATTR_NORMAL;
>> + dosattr |= ATTR_HIDDEN;
>> +
>> + rc = cifs_set_file_info(inode, attrs, xid, full_path, dosattr);
>> + kfree(attrs);
>> + if (rc != 0)
>> + goto out_reval;
>> + rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls,
>> + cifs_sb->mnt_cifs_flags &
>> + CIFS_MOUNT_MAP_SPECIAL_CHR);
>> if (rc == 0) {
>> - rc = CIFSSMBDelFile(xid, tcon, full_path,
>> - cifs_sb->local_nls,
>> - cifs_sb->mnt_cifs_flags &
>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>> - if (!rc) {
>> + if (inode)
>> + drop_nlink(inode);
>> + } else if (rc == -ETXTBSY) {
>> + int oplock = 0;
>> + __u16 netfid;
>> +
>> + rc = CIFSSMBOpen(xid, tcon, full_path,
>> + FILE_OPEN, DELETE,
>> + CREATE_NOT_DIR |
>> + CREATE_DELETE_ON_CLOSE,
>> + &netfid, &oplock, NULL,
>> + cifs_sb->local_nls,
>> + cifs_sb->mnt_cifs_flags &
>> + CIFS_MOUNT_MAP_SPECIAL_CHR);
>> + if (rc == 0) {
>> + CIFSSMBRenameOpenFile(xid, tcon,
>> + netfid, NULL,
>> + cifs_sb->local_nls,
>> + cifs_sb->mnt_cifs_flags &
>> + CIFS_MOUNT_MAP_SPECIAL_CHR);
>> + CIFSSMBClose(xid, tcon, netfid);
>> if (inode)
>> drop_nlink(inode);
>> - } else if (rc == -ETXTBSY) {
>> - int oplock = 0;
>> - __u16 netfid;
>> -
>> - rc = CIFSSMBOpen(xid, tcon, full_path,
>> - FILE_OPEN, DELETE,
>> - CREATE_NOT_DIR |
>> - CREATE_DELETE_ON_CLOSE,
>> - &netfid, &oplock, NULL,
>> - cifs_sb->local_nls,
>> - cifs_sb->mnt_cifs_flags &
>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>> - if (rc == 0) {
>> - CIFSSMBRenameOpenFile(xid, tcon,
>> - netfid, NULL,
>> - cifs_sb->local_nls,
>> - cifs_sb->mnt_cifs_flags &
>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>> - CIFSSMBClose(xid, tcon, netfid);
>> - if (inode)
>> - drop_nlink(inode);
>> - }
>> - /* BB if rc = -ETXTBUSY goto the rename logic BB */
>> }
>> + /* BB if rc = -ETXTBUSY goto the rename logic BB */
>> }
>> }
>> +out_reval:
>> if (inode) {
>> cifsInode = CIFS_I(inode);
>> cifsInode->time = 0; /* will force revalidate to get info
>> --
>> 1.5.5.1
>>
>
>
>
> --
> Thanks,
>
> Steve
--
Thanks,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
