Commit 6050247d authored by Steve French's avatar Steve French

[CIFS] clean up error handling in cifs_unlink

Currently, if a standard delete fails and we end up getting -EACCES
we try to clear ATTR_READONLY and try the delete again. If that
then fails with -ETXTBSY then we try a rename_pending_delete. We
aren't handling other errors appropriately though.

Another client could have deleted the file in the meantime and
we get back -ENOENT, for instance. In that case we wouldn't do a
d_drop. Instead of retrying in a separate call, simply goto the
original call and use the error handling from that.

Also, we weren't properly undoing any attribute changes that
were done before returning an error back to the caller.

CC: Jeff Layton <jlayton@redhat.com>
Signed-off-by: default avatarSteve French <sfrench@us.ibm.com>
parent 6b37faa1
...@@ -837,12 +837,12 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) ...@@ -837,12 +837,12 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
int xid; int xid;
char *full_path = NULL; char *full_path = NULL;
struct inode *inode = dentry->d_inode; struct inode *inode = dentry->d_inode;
struct cifsInodeInfo *cifsInode; struct cifsInodeInfo *cifsInode = CIFS_I(inode);
struct super_block *sb = dir->i_sb; struct super_block *sb = dir->i_sb;
struct cifs_sb_info *cifs_sb = CIFS_SB(sb); struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
struct cifsTconInfo *tcon = cifs_sb->tcon; struct cifsTconInfo *tcon = cifs_sb->tcon;
struct iattr *attrs; struct iattr *attrs = NULL;
__u32 dosattr; __u32 dosattr = 0, origattr = 0;
cFYI(1, ("cifs_unlink, dir=0x%p, dentry=0x%p", dir, dentry)); cFYI(1, ("cifs_unlink, dir=0x%p, dentry=0x%p", dir, dentry));
...@@ -867,8 +867,10 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) ...@@ -867,8 +867,10 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
goto psx_del_no_retry; goto psx_del_no_retry;
} }
retry_std_delete:
rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls, rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
psx_del_no_retry: psx_del_no_retry:
if (!rc) { if (!rc) {
if (inode) if (inode)
...@@ -879,8 +881,7 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) ...@@ -879,8 +881,7 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
rc = cifs_rename_pending_delete(full_path, inode, xid); rc = cifs_rename_pending_delete(full_path, inode, xid);
if (rc == 0) if (rc == 0)
drop_nlink(inode); drop_nlink(inode);
} else if (rc == -EACCES) { } else if (rc == -EACCES && dosattr == 0) {
/* try only if r/o attribute set in local lookup data? */
attrs = kzalloc(sizeof(*attrs), GFP_KERNEL); attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
if (attrs == NULL) { if (attrs == NULL) {
rc = -ENOMEM; rc = -ENOMEM;
...@@ -888,28 +889,25 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) ...@@ -888,28 +889,25 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
} }
/* try to reset dos attributes */ /* try to reset dos attributes */
cifsInode = CIFS_I(inode); origattr = cifsInode->cifsAttrs;
dosattr = cifsInode->cifsAttrs & ~ATTR_READONLY; if (origattr == 0)
origattr |= ATTR_NORMAL;
dosattr = origattr & ~ATTR_READONLY;
if (dosattr == 0) if (dosattr == 0)
dosattr |= ATTR_NORMAL; dosattr |= ATTR_NORMAL;
dosattr |= ATTR_HIDDEN; dosattr |= ATTR_HIDDEN;
rc = cifs_set_file_info(inode, attrs, xid, full_path, dosattr); rc = cifs_set_file_info(inode, attrs, xid, full_path, dosattr);
kfree(attrs);
if (rc != 0) if (rc != 0)
goto out_reval; goto out_reval;
rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags & goto retry_std_delete;
CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc == 0) {
if (inode)
drop_nlink(inode);
} else if (rc == -ETXTBSY) {
rc = cifs_rename_pending_delete(full_path, inode, xid);
if (rc == 0)
drop_nlink(inode);
}
} }
/* undo the setattr if we errored out and it's needed */
if (rc != 0 && dosattr != 0)
cifs_set_file_info(inode, attrs, xid, full_path, origattr);
out_reval: out_reval:
if (inode) { if (inode) {
cifsInode = CIFS_I(inode); cifsInode = CIFS_I(inode);
...@@ -919,9 +917,10 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) ...@@ -919,9 +917,10 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
} }
dir->i_ctime = dir->i_mtime = current_fs_time(sb); dir->i_ctime = dir->i_mtime = current_fs_time(sb);
cifsInode = CIFS_I(dir); cifsInode = CIFS_I(dir);
cifsInode->time = 0; /* force revalidate of dir as well */ CIFS_I(dir)->time = 0; /* force revalidate of dir as well */
kfree(full_path); kfree(full_path);
kfree(attrs);
FreeXid(xid); FreeXid(xid);
return rc; return rc;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment